StateMirroring/StateWatching calling the mirror callback out of order

RESOLVED FIXED in Firefox 45

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

Trunk
mozilla45
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox45 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
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.
(Assignee)

Comment 1

3 years ago
Created attachment 8694071 [details] [diff] [review]
Use tail dispatch to notify the mirror of new value. r=bholley

This ensures that tasks are run in the proper order.
(Assignee)

Updated

3 years ago
Assignee: nobody → jyavenard
(Assignee)

Updated

3 years ago
Attachment #8694071 - Flags: review?(bobbyholley)
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!
Attachment #8694071 - Flags: review?(bobbyholley) → review+

Comment 4

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f6e234ee862b
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox45: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
(Assignee)

Comment 5

3 years ago
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
Flags: needinfo?(jwwang)
Sure. Uplift is preferred.
Flags: needinfo?(jwwang)
You need to log in before you can comment on or make changes to this bug.