nsBuiltinDecoderStateMachine::UpdateReadyState gets called frequently from nsBuiltinDecoderStateMachine off the main thread. It calls nsBuiltinDecoderStateMachine::GetNextFrameStatus and based on the results, dispatches one of three nsBuiltinDecoder::NextFrame* runnables to the main thread, which call mElement->UpdateReadyStateForData with the right status. Meanwhile, on the main thread, nsBuiltinDecoder::UpdateReadyStateForData gets called quite often, which calls nsBuiltinDecoderStateMachine::GetNextFrameStatus and then forwards to mElement->UpdateReadyStateForData. The problem is, values of nsBuiltinDecoderStateMachine::GetNextFrameStatus are computed off the main thread and then effectively stored in the event queue. Meanwhile, while those events are queued, the status can change. Then, for a while, we can have calls to nsBuiltinDecoder::UpdateReadyStateForData interleaved with processing of NextFrame* runnables, which disagree about the the next-frame-status is, so we end up flip-flopping between two different states. The fix is easy and simplifies code: nsBuiltinDecoderStateMachine::UpdateReadyState should just fire off a runnable to run nsBuiltinDecoder::UpdateReadyStateForData on the main thread, which will get the latest GetNextFrameStatus when it runs. No more flip-flopping.

I'm not sure how much this actually hurts Youtube, but it definitely doesn't help, because it can trigger the firing of a lot of DOM events unnecessarily.

Comment on attachment 761286 [details] [diff] [review] fix Review of attachment 761286 [details] [diff] [review]: ----------------------------------------------------------------- Remember to patch on m-c too.

From a user perspective, what impact does this bug have on watching media content?

It'll be slowing down the Web site a bit, at least, and may in some cases cause the spinny "waiting" controls UI to appear and disappear unnecessarily. It's hard to say though, because it's hard to do a controlled experiment while there are other bugs.

Comment on attachment 761286 [details] [diff] [review] fix NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings. [Approval Request Comment] Bug caused by (feature/regressing bug #): none User impact if declined: slightly slower Web sites playing video, possibly unnecessary flipping of video control UI between waiting/not-waiting states Testing completed: local testing Risk to taking this patch (and alternatives if risky): pretty low risk. Just makes readyState handling more uniform instead of handling it differently depending on what triggered the change. String or UUID changes made by this patch: none The main reason I want this on b2g18 is to simplify testing. I'm trying to fix Youtube bugs on b2g18 and I want to be sure that bugs we see are not due to this (known) issue.

Per Alex we're apparently not doing approvals anymore, so I'll do the leo? nom instead to see if we want this on b2g18.

Backed out: https://hg.mozilla.org/integration/mozilla-inbound/rev/775c4219d32a The patch needs to be rebased on top of 822952 on trunk, which is nontrivial.

Push backed out for reftest failures: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/ff486a6eb5c9 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/1ad2d8b5e7c9 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/f8048b030fe6 https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f2d55d17a0d8