This runnable is in the top 25 or so unlabeled runnables according to telemetry.

This should be able to follow on directly from bug 1359833 as we add ProgressTracker::GetEventTarget which can be used to get the best-effort labelled guess. imgRequest::FinishPreparingForNewPart itself does not appear to have any label/document specific calls, so a simple dispatch on the right target is all we need.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=91e12a5c355d4637ae50b6d636e5e083bb5974c5

Comment on attachment 8888306 [details] [diff] [review] Part 1. imgRequest::FinishPreparingForNewPart should dispatch using ProgressTracker::GetEventTarget., v1 Why is calling FinishPreparingForNewPart on the current thread the right thing to do if we don't have an event target?

Comment on attachment 8888307 [details] [diff] [review] Part 2. imgRequest::Cancel should dispatch using ProgressTracker::GetEventTarget., v1 Here we don't nullcheck eventTarget, so it's always non-null?

(In reply to Timothy Nikkel (:tnikkel) from comment #5) > Comment on attachment 8888306 [details] [diff] [review] > Part 1. imgRequest::FinishPreparingForNewPart should dispatch using > ProgressTracker::GetEventTarget., v1 > > Why is calling FinishPreparingForNewPart on the current thread the right > thing to do if we don't have an event target? It is indirectly a way to check NS_IsMainThread() since eventTarget is only set if it isn't the main thread. I can add an assert for this inside the if block -- it is a bit counter-intuitive.

(In reply to Timothy Nikkel (:tnikkel) from comment #6) > Comment on attachment 8888307 [details] [diff] [review] > Part 2. imgRequest::Cancel should dispatch using > ProgressTracker::GetEventTarget., v1 > > Here we don't nullcheck eventTarget, so it's always non-null? Yes, ProgressTracker::GetEventTarget will always give a valid event target (mEventTarget is NotNull). I think I tried to make GetEventTarget return NotNull as well, but there were some callers which wanted *their* mEventTarget to be null sometimes, and there was hairy to put a NotNull<RefPtr> into a RefPtr without adjusting the refcount again, so I didn't make it return that.

(In reply to Andrew Osmond [:aosmond] from comment #7) > (In reply to Timothy Nikkel (:tnikkel) from comment #5) > > Comment on attachment 8888306 [details] [diff] [review] > > Part 1. imgRequest::FinishPreparingForNewPart should dispatch using > > ProgressTracker::GetEventTarget., v1 > > > > Why is calling FinishPreparingForNewPart on the current thread the right > > thing to do if we don't have an event target? > > It is indirectly a way to check NS_IsMainThread() since eventTarget is only > set if it isn't the main thread. I can add an assert for this inside the if > block -- it is a bit counter-intuitive. Okay, just put something there to make that clear, otherwise it seems weird.

Add comment and assert.

Pushed by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ed28a90d1edb Part 1. imgRequest::FinishPreparingForNewPart should dispatch using ProgressTracker::GetEventTarget. r=tnikkel https://hg.mozilla.org/integration/mozilla-inbound/rev/013c4af28c40 Part 2. imgRequest::Cancel should dispatch using ProgressTracker::GetEventTarget. r=tnikkel

https://hg.mozilla.org/mozilla-central/rev/ed28a90d1edb https://hg.mozilla.org/mozilla-central/rev/013c4af28c40