1343437 - Demuxer InitPromise success value can include a warning, to be forwarded to the media element

Status: RESOLVED FIXED

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

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

The resolve-value of MediaDataDemuxer::InitPromise is currently an nsresult that is always set to true, and is never actually used.

It could be turned into a more useful MediaResult, which could carry a warning, and this warning could be forwarded to the appropriate HTMLMediaElement::DecodeWarning().

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1343437 - MediaDataDemuxer::InitPromise resolve-value is now a MediaResult -

In case of successful initialization, there could still be some warning about
a recoverable error. To carry more information about this potential warning,
the resolve-value type of the promise is changed from nsresult to MediaResult.

Existing code doesn't need to be changed:
- In Resolve() calls, the stored MediaResult can implicitly be constructed from
an nsresult -- always NS_OK at the moment anyway.
- In following Then(), the promise's MediaResult can implicitly be converted to
an nsresult.

Future patches will modify some of the Resolve's and Then's, to forward warning
details to some Decoder Doctor object...

Review commit: https://reviewboard.mozilla.org/r/116524/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116524/

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1343437 - MP4Demuxer::Init resolves with a MediaResult -

If MP4Demuxer::Init detects some recoverable error (e.g., invalid tracks when
others may still be usable), it will eventually Resolve the promise with the
first warning.

Later on, errors/warnings from the MP4Metadata parser will also be handled, to
provide even better diagnostics.

Review commit: https://reviewboard.mozilla.org/r/116526/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116526/

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1343437 - MP4Demuxer rejects InitPromise is there is no valid track -

An easy check that the MP4Demuxer::Init can do, is to ensure that there is at
least one valid track.

Note that the caller may itself do similar checks, but they are currently only
looking at the first track of each type, so this check here adds a bit more
information ("*no* valid tracks" vs "no valid *first* tracks").

Review commit: https://reviewboard.mozilla.org/r/116528/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116528/

Comment 4

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1343437 - MFR::OnDemuxerInitDone forwards non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

The MediaFormatReader now takes the MediaResult from the Demuxer::Init promise
resolution, and if there are no other errors but the MediaResult is not NS_OK
it will forward that warning to the decoder owner (i.e., the associated
HTMLMediaElement).

Review commit: https://reviewboard.mozilla.org/r/116530/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116530/

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1343437 - TBM::OnDemuxer{Init,Reset}Done forward non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

Similarly to the MediaFormatReader, TrackBuffersManager can forward warnings
from the demuxer initialization to the associated HTMLMediaElement.

Note that errors (sent to OnDemuxerInitFailed) are currently *not* forwarded
to the HTMLMediaElement by design. In the future, we may want to add this
feature so that mediasource errors can also be reported to webcompat.

Review commit: https://reviewboard.mozilla.org/r/116532/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116532/

Comment 6

[Jean-Yves Avenard [:jya]]

Comment on attachment 8842757 [details]
Bug 1343437 - MediaDataDemuxer::InitPromise resolve-value is now a MediaResult -

https://reviewboard.mozilla.org/r/116524/#review118142

I am dubious to the need to report errors but that aren't errors when an init promise resolved.
Either it works, or it doesn't.

What information could be of any use to the user (or even developer) for a file we accepted to open.

Providing details is good, but it has to be of some use.
Reporting on the why we refused to open a file is one thing. Reporting that we opened a file but really we may not have is useless IMHO.

So r+ if you really want this, but I don't think it should go in: information overload.

Comment 7

[Jean-Yves Avenard [:jya]]

Comment on attachment 8842758 [details]
Bug 1343437 - MP4Demuxer::Init resolves with a MediaResult -

https://reviewboard.mozilla.org/r/116526/#review118144

samre reserve as earlier

Comment 8

[Jean-Yves Avenard [:jya]]

Comment on attachment 8842759 [details]
Bug 1343437 - MP4Demuxer rejects InitPromise is there is no valid track -

https://reviewboard.mozilla.org/r/116528/#review118146

This serves no purpose, and is redundant. The MediaFormatReader already reject the MetadataPromise if there's no valid track

Comment 9

[Jean-Yves Avenard [:jya]]

Comment on attachment 8842760 [details]
Bug 1343437 - MFR::OnDemuxerInitDone forwards non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

https://reviewboard.mozilla.org/r/116530/#review118150

Comment 10

[Jean-Yves Avenard [:jya]]

Comment on attachment 8842761 [details]
Bug 1343437 - TBM::OnDemuxer{Init,Reset}Done forward non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

https://reviewboard.mozilla.org/r/116532/#review118152

Comment 11

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842759 [details]
Bug 1343437 - MP4Demuxer rejects InitPromise is there is no valid track -

https://reviewboard.mozilla.org/r/116528/#review118146

As I explained in the commit description, the MFR rejects if there's no valid *first* tracks!
The new test here reports an error if there's not valid tracks *at all*, which I think would be a useful addition, it's not totally redundant.

But it's not that critical I suppose, so I'm fine if you really don't want this extra bit of information.

Comment 12

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1343437 - 'media.playback.warnings-as-errors' pref -

If 'media.playback.warnings-as-errors' is true, warnings coming from demuxer's
Init will be treated as errors, causing playback to fail.

Currently set to false by default.

This could be later changed to catch and diagnose more issues.

Review commit: https://reviewboard.mozilla.org/r/116952/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/116952/

Comment 13

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842757 [details]
Bug 1343437 - MediaDataDemuxer::InitPromise resolve-value is now a MediaResult -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116524/diff/1-2/

Comment 14

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842758 [details]
Bug 1343437 - MP4Demuxer::Init resolves with a MediaResult -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116526/diff/1-2/

Comment 15

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842760 [details]
Bug 1343437 - MFR::OnDemuxerInitDone forwards non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116530/diff/1-2/

Comment 16

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842761 [details]
Bug 1343437 - TBM::OnDemuxer{Init,Reset}Done forward non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116532/diff/1-2/

Comment 17

[Jean-Yves Avenard [:jya]]

Comment on attachment 8843185 [details]
Bug 1343437 - 'media.playback.warnings-as-errors' pref -

https://reviewboard.mozilla.org/r/116952/#review118964

::: dom/media/MediaFormatReader.cpp:1267
(Diff revision 1)
>  MediaFormatReader::OnDemuxerInitDone(const MediaResult& aResult)
>  {
>    MOZ_ASSERT(OnTaskQueue());
>    mDemuxerInitRequest.Complete();
>  
> +  if (aResult != NS_OK && MediaPrefs::MediaWarningsAsErrors()) {

NS_SUCCEEDED

as thats how all MediaResult are always used to check for success.

::: dom/media/mediasource/TrackBuffersManager.cpp:863
(Diff revision 1)
>  TrackBuffersManager::OnDemuxerResetDone(const MediaResult& aResult)
>  {
>    MOZ_ASSERT(OnTaskQueue());
>    mDemuxerInitRequest.Complete();
> +
> +  if (aResult != NS_OK && MediaPrefs::MediaWarningsAsErrors()) {

2 spaces indent

::: dom/media/mediasource/TrackBuffersManager.cpp:951
(Diff revision 1)
>    MOZ_DIAGNOSTIC_ASSERT(mInputDemuxer, "mInputDemuxer has been destroyed");
>  
>    mDemuxerInitRequest.Complete();
>  
> +  if (aResult != NS_OK && MediaPrefs::MediaWarningsAsErrors()) {
> +      RejectAppend(aResult, __func__);

2 spaces indent

::: modules/libpref/init/all.js:611
(Diff revision 1)
>  
>  // Log level for cubeb, the audio input/output system. Valid values are
>  // "verbose", "normal" and "" (log disabled).
>  pref("media.cubeb.log_level", "");
>  
> +// Set to true to force demux/decode warnings to be treated as errors.

The comment about decode errors doesnt match the use. 

its all about demuxing warnings right now.

or you can also make it for non fatal decode errors as well, and quite easily so. Modify the MFR so that the test checking if an error is fatal to always return true if the pref is set.

Comment 18

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8843185 [details]
Bug 1343437 - 'media.playback.warnings-as-errors' pref -

https://reviewboard.mozilla.org/r/116952/#review118964

> The comment about decode errors doesnt match the use. 
> 
> its all about demuxing warnings right now.
> 
> or you can also make it for non fatal decode errors as well, and quite easily so. Modify the MFR so that the test checking if an error is fatal to always return true if the pref is set.

I'm not sure I understand your comment.

To make sure we're talking about the same things:
What I call "warning" is an error code that comes through a promise *resolution* (as opposed to a real error that comes through a promise *rejection*).

If this pref "media.playback.warnings-as-errors" is set to true, warnings that come into promise resolution callbacks will be handled the same was as errors in rejection callbacks.
For example, in `MFR::OnDemuxerInitDone(MediaResult aResult)`, if NS_FAILED(aResult) and MediaWarningsAsErrors, we just reject the mMetadataPromise and return, which is what happens in MFR::OnDemuxerInitFailed.

So I think the comment matches the code. Don't you think? Or should I rephrase the comment to be more understandable?


About fatal errors in the MFR, are you talking about HasFatalError() allowing a certain number of NS_ERROR_DOM_MEDIA_DECODE_ERR's?
If yes, I could indeed use that pref to make them immediately fatal. -- Is that what you meant when saying the comment didn't match the use?
And looking at it, maybe I should treat these non-fatal NS_ERROR_DOM_MEDIA_DECODE_ERR errors as warnings otherwise. I.e., I could show a user notification if they happen.

Comment 19

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1343437 - MFR::NotifyError reports non-fatal MediaResult as warning -

Review commit: https://reviewboard.mozilla.org/r/117444/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/117444/

Comment 20

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842757 [details]
Bug 1343437 - MediaDataDemuxer::InitPromise resolve-value is now a MediaResult -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116524/diff/2-3/

Comment 21

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842758 [details]
Bug 1343437 - MP4Demuxer::Init resolves with a MediaResult -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116526/diff/2-3/

Comment 22

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842760 [details]
Bug 1343437 - MFR::OnDemuxerInitDone forwards non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116530/diff/2-3/

Comment 23

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842761 [details]
Bug 1343437 - TBM::OnDemuxer{Init,Reset}Done forward non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116532/diff/2-3/

Comment 24

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8843185 [details]
Bug 1343437 - 'media.playback.warnings-as-errors' pref -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116952/diff/1-2/

Comment 25

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8843185 [details]
Bug 1343437 - 'media.playback.warnings-as-errors' pref -

https://reviewboard.mozilla.org/r/116952/#review118964

> I'm not sure I understand your comment.
> 
> To make sure we're talking about the same things:
> What I call "warning" is an error code that comes through a promise *resolution* (as opposed to a real error that comes through a promise *rejection*).
> 
> If this pref "media.playback.warnings-as-errors" is set to true, warnings that come into promise resolution callbacks will be handled the same was as errors in rejection callbacks.
> For example, in `MFR::OnDemuxerInitDone(MediaResult aResult)`, if NS_FAILED(aResult) and MediaWarningsAsErrors, we just reject the mMetadataPromise and return, which is what happens in MFR::OnDemuxerInitFailed.
> 
> So I think the comment matches the code. Don't you think? Or should I rephrase the comment to be more understandable?
> 
> 
> About fatal errors in the MFR, are you talking about HasFatalError() allowing a certain number of NS_ERROR_DOM_MEDIA_DECODE_ERR's?
> If yes, I could indeed use that pref to make them immediately fatal. -- Is that what you meant when saying the comment didn't match the use?
> And looking at it, maybe I should treat these non-fatal NS_ERROR_DOM_MEDIA_DECODE_ERR errors as warnings otherwise. I.e., I could show a user notification if they happen.

Hopefully I've interpreted your review correctly:
- A new patch stores non-fatal errors as warnings.
- 'media.playback.warnings-as-errors' doesn't allow multiple decode errors before failing.
Please let me know if you were thinking of other things to do.

Comment 26

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=024c40d932d106e6329a36302554bfb87741f44f

It contains printf's to see what kind of errors/warnings would be reported; search for "::Decode" in non-e10s logs.

Comment 27

[Jean-Yves Avenard [:jya]]

Comment on attachment 8843842 [details]
Bug 1343437 - MFR::NotifyError reports non-fatal MediaResult as warning -

https://reviewboard.mozilla.org/r/117444/#review119074

you now have duplicated code in the MFR, this code should be placed where the handling of error is done in MFR::Update

::: dom/media/MediaFormatReader.cpp:1742
(Diff revision 1)
>  MediaFormatReader::NotifyError(TrackType aTrack, const MediaResult& aError)
>  {
>    MOZ_ASSERT(OnTaskQueue());
>    NS_WARNING(aError.Description().get());
>    LOGV("%s Decoding error", TrackTypeToStr(aTrack));
> -  auto& decoder = GetDecoderData(aTrack);
> +  auto& decoderData = GetDecoderData(aTrack);

throughout the code, the temporary is called decoder.
please leave as is, or change them all

Comment 28

[Jean-Yves Avenard [:jya]]

Comment on attachment 8843185 [details]
Bug 1343437 - 'media.playback.warnings-as-errors' pref -

https://reviewboard.mozilla.org/r/116952/#review118964

> Hopefully I've interpreted your review correctly:
> - A new patch stores non-fatal errors as warnings.
> - 'media.playback.warnings-as-errors' doesn't allow multiple decode errors before failing.
> Please let me know if you were thinking of other things to do.

the comment is "// Set to true to force demux/decode warnings to be treated as errors."

But only demux warning are treated as errors in the patch queue at the time you asked me for review.
There was nothing related to decoding error being treated as error.

All you had to do was change HasFatalError to test for the pref and return true when needed. A small change.

Comment 29

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842757 [details]
Bug 1343437 - MediaDataDemuxer::InitPromise resolve-value is now a MediaResult -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116524/diff/3-4/

Comment 30

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842758 [details]
Bug 1343437 - MP4Demuxer::Init resolves with a MediaResult -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116526/diff/3-4/

Comment 31

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842760 [details]
Bug 1343437 - MFR::OnDemuxerInitDone forwards non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116530/diff/3-4/

Comment 32

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842761 [details]
Bug 1343437 - TBM::OnDemuxer{Init,Reset}Done forward non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116532/diff/3-4/

Comment 33

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8843185 [details]
Bug 1343437 - 'media.playback.warnings-as-errors' pref -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116952/diff/2-3/

Comment 34

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1343437 - HTMLMediaElement::DecodeWarning() should store a warning instead of an error -

Review commit: https://reviewboard.mozilla.org/r/119676/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/119676/

Comment 35

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842757 [details]
Bug 1343437 - MediaDataDemuxer::InitPromise resolve-value is now a MediaResult -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116524/diff/4-5/

Comment 36

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842758 [details]
Bug 1343437 - MP4Demuxer::Init resolves with a MediaResult -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116526/diff/4-5/

Comment 37

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842760 [details]
Bug 1343437 - MFR::OnDemuxerInitDone forwards non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116530/diff/4-5/

Comment 38

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842761 [details]
Bug 1343437 - TBM::OnDemuxer{Init,Reset}Done forward non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116532/diff/4-5/

Comment 39

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8843185 [details]
Bug 1343437 - 'media.playback.warnings-as-errors' pref -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116952/diff/3-4/

Comment 40

[Jean-Yves Avenard [:jya]]

Comment on attachment 8846659 [details]
Bug 1343437 - HTMLMediaElement::DecodeWarning() should store a warning instead of an error -

https://reviewboard.mozilla.org/r/119676/#review121952

Comment 41

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8846659 [details]
Bug 1343437 - HTMLMediaElement::DecodeWarning() should store a warning instead of an error -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/119676/diff/1-2/

Comment 42

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842757 [details]
Bug 1343437 - MediaDataDemuxer::InitPromise resolve-value is now a MediaResult -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116524/diff/5-6/

Comment 43

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842758 [details]
Bug 1343437 - MP4Demuxer::Init resolves with a MediaResult -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116526/diff/5-6/

Comment 44

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842760 [details]
Bug 1343437 - MFR::OnDemuxerInitDone forwards non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116530/diff/5-6/

Comment 45

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8842761 [details]
Bug 1343437 - TBM::OnDemuxer{Init,Reset}Done forward non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116532/diff/5-6/

Comment 46

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8843185 [details]
Bug 1343437 - 'media.playback.warnings-as-errors' pref -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/116952/diff/4-5/

Comment 47

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07744b3914c7
HTMLMediaElement::DecodeWarning() should store a warning instead of an error - r=jya
https://hg.mozilla.org/integration/autoland/rev/fe99ecf674b8
MediaDataDemuxer::InitPromise resolve-value is now a MediaResult - r=jya
https://hg.mozilla.org/integration/autoland/rev/f90ddaaea394
MP4Demuxer::Init resolves with a MediaResult - r=jya
https://hg.mozilla.org/integration/autoland/rev/545bdc6c8ca3
MFR::OnDemuxerInitDone forwards non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning - r=jya
https://hg.mozilla.org/integration/autoland/rev/1c76c26c779c
TBM::OnDemuxer{Init,Reset}Done forward non-NS_OK MediaResult to HTMLMediaElement::DecodeWarning - r=jya
https://hg.mozilla.org/integration/autoland/rev/3b5b46e78a82
'media.playback.warnings-as-errors' pref - r=jya

Comment 48

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/07744b3914c7
https://hg.mozilla.org/mozilla-central/rev/fe99ecf674b8
https://hg.mozilla.org/mozilla-central/rev/f90ddaaea394
https://hg.mozilla.org/mozilla-central/rev/545bdc6c8ca3
https://hg.mozilla.org/mozilla-central/rev/1c76c26c779c
https://hg.mozilla.org/mozilla-central/rev/3b5b46e78a82