Closed Bug 1273310 Opened 5 years ago Closed 5 years ago

Simplify DecoderDoctor PDM startup latches

Categories

(Core :: Audio/Video: Playback, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: rillian, Assigned: rillian)

Details

Attachments

(1 file)

We really want the latest result, not whether it has ever failed.
This is a bit easier to read. Currently there's no difference
since Startup is only called once, but it's probably better
future-proofing too, since the diagnostics should care more
about the current state than whether there's every been a
failure.

Review commit: https://reviewboard.mozilla.org/r/53004/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/53004/
Attachment #8753105 - Flags: review?(gsquelart)
Assignee: nobody → giles
Comment on attachment 8753105 [details]
MozReview Request: Bug 1273310 - Simplify DecoderDoctor failure flags. r=gerald

https://reviewboard.mozilla.org/r/53004/#review49836

r+ with the following changes:

I wouldn't call these variables 'latches' in your commit description, as they *could* theoretically go back to 'false' if CreatePDMs were to be run again.
How about 'failure states'?

::: dom/media/platforms/PDMFactory.cpp:256
(Diff revision 1)
>    if (MediaPrefs::PDMWMFEnabled()) {
>      m = new WMFDecoderModule();
> -    if (!StartupPDM(m)) {
> +    mWMFFailedToLoad = !StartupPDM(m);
> -      mWMFFailedToLoad = true;
> -    }
>    }

In case the pref is off, we want to reset the 'failed' variables to False, i.e.:
if (MediaPrefs::PDMWMFEnabled() {
  ...
} else {
  mWMFFailedToLoad = false;
}

Same thing with the other ones below...
Attachment #8753105 - Flags: review?(gsquelart) → review+
Comment on attachment 8753105 [details]
MozReview Request: Bug 1273310 - Simplify DecoderDoctor failure flags. r=gerald

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53004/diff/1-2/
Attachment #8753105 - Attachment description: MozReview Request: Bug 1273310 - Simplify DecoderDoctor latches. r?gerald → MozReview Request: Bug 1273310 - Simplify DecoderDoctor failure flags. r=gerald
https://reviewboard.mozilla.org/r/53004/#review49852

Applied. Carrying forward r=gerald.
Thank you for making this code more correcter.
https://hg.mozilla.org/mozilla-central/rev/e80ac440424e
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.