1229299 - StateMirroring/StateWatching calling the mirror callback out of order

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Jean-Yves Avenard [:jya]]

While investigating bug 1229256, I noticed that the WatchManager was calling MediaDecoder::UpdateReadyState with incorrect value.

The value I saw was first:
mNextFrameStatus = NEXT_FRAME_UNAVAILABLE
followed by another call with:
mNextFrameStatus = NEXT_FRAME_UNINITIALIZED

The first call being of the updated value of the mNextFrameStatus MDSM canonical, and the 2nd being the value set in the MDSM canonical constructor.

Upon investigation, the issue is that the first time the canonical's value is modified it dispatch a task using tail dispatch.
However, in the construction a standard task is dispatched.

As the tail dispatch is run first, we see the new updated value before the original value.

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Use tail dispatch to notify the mirror of new value. r=bholley

This ensures that tasks are run in the proper order.

Comment 2

[Bobby Holley (:bholley)]

Comment on attachment 8694071 [details] [diff] [review]
Use tail dispatch to notify the mirror of new value. r=bholley

Review of attachment 8694071 [details] [diff] [review]:
-----------------------------------------------------------------

Nice find!

Comment 3

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f6e234ee862b

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f6e234ee862b

Comment 5

[Jean-Yves Avenard [:jya]]

JW, do you think this should be uplifted?

AFAICT, purely looking at MediaDecoder, it would cause to not set up readyState to HAVE_FUTURE_DATA until playback starts.
there may be others as we're pretty reliant now on the StateMirror machinery to work

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Sure. Uplift is preferred.