We're going to need to do this at some point if we want to use the state-mirroring machinery to mirror canonical main-thread values like mPlayState. I've also determined that TailDispatchers would nicely solve the races I'm seeing in bug 1144486 (given a few modifications).

This test assumes that the decoding machinery will have completed its work one event-loop-round-trip after the document loads (see the executeSoon call), which is a totally bogus assumption.

This is a followup from bug 1154805 comment 13.

Comment on attachment 8593593 [details] [diff] [review] Part 4 - Implement tail dispatch for the main thread. v1 Review of attachment 8593593 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/AbstractThread.cpp @@ +67,5 @@ > MOZ_ASSERT(in == (GetCurrent() == this)); > return in; > } > > + void FireTailDispatcher() { MOZ_ASSERT(mTailDispatcher.isSome()); mTailDispatcher.reset(); } I am afraid this is not consistent with |appShell->RunInStableState|. We should run the runnables contained in TaskGroupRunnable immediately here instead of dispatch it because FireTailDispatcher() is already executed in the synchronous section.

Comment on attachment 8593593 [details] [diff] [review] Part 4 - Implement tail dispatch for the main thread. v1 Review of attachment 8593593 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/media/AbstractThread.cpp @@ +67,5 @@ > MOZ_ASSERT(in == (GetCurrent() == this)); > return in; > } > > + void FireTailDispatcher() { MOZ_ASSERT(mTailDispatcher.isSome()); mTailDispatcher.reset(); } Thinking about it again, I don't think we need to make TailDispatcher interchangeable with |appShell->RunInStableState|.

(In reply to JW Wang [:jwwang] from comment #9) > Thinking about it again, I don't think we need to make TailDispatcher > interchangeable with |appShell->RunInStableState|. Yeah - we use RunInStableState to implement TailDispatcher, but their semantics are fundamentally different. Would you mind stamping the test change (part 1) as well? Then I can land it when inbound reopens. :-)

Comment on attachment 8593590 [details] [diff] [review] Part 1 - Fix racey test_error_in_video_document.html. v1 Review of attachment 8593590 [details] [diff] [review]: ----------------------------------------------------------------- This is indeed bug 608634. The delay-load-flag of the media element will prevent 'load' event of the page from being fired. Therefore, the error event of the media element should always be fired before the page load event. However, my last attempt to fix bug 608634 didn't succeed and the bug was pending until now. In order not to block your patch, I suggest to put a SimpleTest.todo to note we have to fix it in the future.

Thanks for the reviews! remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/7b48c3a29d25 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/5005606be182 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/af419f5b5ef9 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/82b1e0300d4f

https://hg.mozilla.org/mozilla-central/rev/7b48c3a29d25 https://hg.mozilla.org/mozilla-central/rev/5005606be182 https://hg.mozilla.org/mozilla-central/rev/af419f5b5ef9 https://hg.mozilla.org/mozilla-central/rev/82b1e0300d4f