This runnable showed up recently in our list of unlabeled runnables. I'm guessing it's Stylo-related. It's now the #4 most common unlabeled runnable. It looks like it's just doing some cleanup, so maybe we could label it with the SystemGroup? That would be valid if the effects of this runnable are not observable from web content. Is that true, Cameron?

Since you are seeing these runnables unlabeled, I guess that means we're in the branch here where we call NS_DispatchToMainThread? http://searchfox.org/mozilla-central/rev/30a47c4339bd397b937abdb2305f99d3bb537ba6/layout/style/nsStyleStruct.cpp#2019-2040 SystemGroup runnables can be run at any arbitrary time afterwards, is that right? The effects of the runnable aren't observable from web content obviously, although if it does run later, then it's possible a new image load request for the same image would not result in a new network request, since the ImageValue object that the nsStyleImageRequest is holding on to would still be alive. Maybe that's a good thing? :-) I think the work that the ImageValue destructor does should be safe to be called at any point later.

It's not expected that SystemGroup runnables should be delayed for more than a millisecond or two. It sounds like SystemGroup should be fine here.

It seems strange to me that so many of these are being dispatched when we don't know what document we're connected to. Are we firing off a bunch of requests that are never getting resolved? Could it be possible to fire off fewer requests? Anyway, we could do that as a followup.

ni? to heycam for comment 3.

FYI, I also tried the same fix locally last week before taking this bug, but I hit timeout consistently in test_compute_data_with_start_struct.html in the build with stylo enabled: https://treeherder.mozilla.org/logviewer.html#?job_id=122204179&repo=try&lineNumber=21598

(In reply to Bevis Tseng [:bevis][:btseng] from comment #5) > FYI, I also tried the same fix locally last week before taking this bug, but > I hit timeout consistently in test_compute_data_with_start_struct.html in > the build with stylo enabled: > https://treeherder.mozilla.org/logviewer. > html#?job_id=122204179&repo=try&lineNumber=21598 Indeed, I can see that same result! It's very peculiar...SystemGroup::Dispatch() isn't failing, so the runnable is getting into the event queue, but then it's getting...dropped? Furthermore, if we didn't have mDocGroup, that means we never called Resolve(). So we constructed this nsStyleImageRequest off the main thread, never resolved it, and then don't have anything to clean up. Unless the css::ImageValue stored in such a request still needs some sort of special handling? Or we constructed this nsStyleImageRequest on the main thread, and then...what? We have mRequestProxy, and it releases things as normal, or we don't, and we have a css::ImageValue (and *possibly* an ImageTracker), and those are somehow left hanging. Local debugging says that *all* of the nsStyleImageRequests that we are releasing off the main thread have nullptr everywhere for this particular test, so these tasks aren't even doing anything. Could we just...not create this task if we didn't have any work to do?

Reducing things to: MOZ_ASSERT(!mImageTracker); NS_ReleaseOnMainThreadSystemGroup("ImageValue Release", mImageValue.forget(), /* aAlwaysProxy= */ true); when we have a nullptr mDocGroup also triggers hangs. Still very unclear why that happens. Not sure what we're waiting for.

Comment on attachment 8896386 [details] [diff] [review] label StyleImageRequestCleanupTask, part 2 Review of attachment 8896386 [details] [diff] [review]: ----------------------------------------------------------------- Canceling review for now since it seems like there's more investigation needed. Should we needinfo Cameron about comment 6?

This runnable now is the top 1 unlabeled runnable according to the latest telemetry analysis: https://gist.github.com/bevis-tseng/d8a918cb53112a82357c872ce15ee790 We'll need this one to be labeled in priority.

The timeout problem in stylo build mentioned in comment 5 is gone recently. After further bisect, the modification of ImageFactory in Bug 1325080 somehow fixes/suppresses this timeout issue: https://hg.mozilla.org/mozilla-central/rev/072e904155c3

(In reply to Bevis Tseng[:bevis][:btseng] from comment #10) > The timeout problem in stylo build mentioned in comment 5 is gone recently. > After further bisect, the modification of ImageFactory in Bug 1325080 > somehow fixes/suppresses this timeout issue: > https://hg.mozilla.org/mozilla-central/rev/072e904155c3 Cool, thanks for looking at this! A try run confirms this, so I'm going to commit the patch.

Comment on attachment 8896386 [details] [diff] [review] label StyleImageRequestCleanupTask, part 2 Oh, actually, Bill hasn't reviewed the patch yet. That would be good to do first!

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/fa27d49fed59 label StyleImageRequestCleanupTask; r=billm