1691578 - Add test cases to test `DecoderDoctorDiagnostics` and `DecoderDoctorDocumentWatcher`

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, task, P2)

Description

[Alastor Wu [:alwu]]

Although we have browser_decoderDoctor.js, that test is only testing the front end part, which starts from sending decoder-doctor-notification notification.

We don't have any test to test the process of (1) setting the error flag on DecoderDoctorDiagnostics (2) processing different error flags (3) determining if the error should be reported to the notification banner or not.

Therefore, in this bug, I will add some test cases to ensure we cover those parts.

Comment 1

[Alastor Wu [:alwu]]

Attachment: Bug 1691578 - part1 : rename `MediaPlatformDecoderNotFound` to `MediaFFMpegNotFound`.

MediaPlatformDecoderNotFound sounds too general and seems indicating that this error is platform independent, but that is not true. This error is actually for not founding ffmpeg.

Comment 2

[Alastor Wu [:alwu]]

Attachment: Bug 1691578 - part2 : remove unnecessary operation of removing ffmpeg error flag.

This operation is actually meaningless, because at the time we call CreateRddPDMs(), the PDMFactory is just created. [PDMFactory()->CreatePDMs()->CreateRddPDMs()]

So that is impossible that we've failed to load ffmpeg before, so no need to subtract the FFmpegFailedToLoad flag.

Depends on D104471

Comment 3

[Alastor Wu [:alwu]]

Attachment: Bug 1691578 - part3 : Set different ffmpeg error in PDMFactory.

Two reasons to have this patch,

(1) we should let caller to be responsible to set the error, like what we do for WMF decoder module. DecoderDoctorDocumentWatcher should simply relys on the flag set in DecoderDoctorDiagnostics.

(2) separating these two different error helps us write a test for different error situation, without relying on the actual state of ffmpeglinker.

Depends on D104472

Comment 4

[Alastor Wu [:alwu]]

Attachment: Bug 1691578 - part4 : allow to set different error in all platforms, but still report them to corresponding platforms, except in testing.

If some attributes are only able to be set in certain platform, that increase the difficulty of wrting a test.

Therefore, we make them being able to set in all platforms, but doing the check in the fianl stage, which is reporting the result, to ensure that those error would still only be displayed on the right platform.

But if we're in testing, then we can test all error types in one platform.

Depends on D104473

Comment 5

[Alastor Wu [:alwu]]

Attachment: Bug 1691578 - part5 : add test cases to test the ability of reporting error for not supported media type.

Add test cases for DecoderDoctorDiagnostics::StoreFormatDiagnostics(), which is used when checking the ability of whether the decoder is supported for certain type.

Depends on D104474

Comment 6

[Alastor Wu [:alwu]]

I'm still working on this, and will add more test cases.

Comment 7

[Alastor Wu [:alwu]]

Attachment: Bug 1691578 - part6 : add test cases to test reporting decode error.

Depends on D104475

Comment 8

[Alastor Wu [:alwu]]

Attachment: Bug 1691578 - part7 : add test case for audio sink failed start-up.

Depends on D104570

Comment 9

[Alastor Wu [:alwu]]

Attachment: Bug 1691578 - part8 : allow showing platform-decoder-not-found notification.

Depends on D104571

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6aa662e8d06e
part1 : rename `MediaPlatformDecoderNotFound` to `MediaFFMpegNotFound`. r=bryce
https://hg.mozilla.org/integration/autoland/rev/71a17ff3322c
part2 : remove unnecessary operation of removing ffmpeg error flag. r=bryce
https://hg.mozilla.org/integration/autoland/rev/214ab3a2f00b
part3 : Set different ffmpeg error in PDMFactory. r=bryce
https://hg.mozilla.org/integration/autoland/rev/bfd3e4878545
part4 : allow to set different error in all platforms, but still report them to corresponding platforms, except in testing. r=bryce
https://hg.mozilla.org/integration/autoland/rev/23f6dcc3ceb5
part5 : add test cases to test the ability of reporting error for not supported media type. r=bryce,emilio
https://hg.mozilla.org/integration/autoland/rev/c181e29b13f2
part6 : add test cases to test reporting decode error. r=bryce,webidl,emilio
https://hg.mozilla.org/integration/autoland/rev/518eadda7826
part7 : add test case for audio sink failed start-up. r=bryce,emilio
https://hg.mozilla.org/integration/autoland/rev/c535dd429ada
part8 : allow showing platform-decoder-not-found notification. r=bryce

Comment 11

[Cosmin Sabou [:CosminS]]

Backed out for bc failures on browser_decoderDoctor.js.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=c535dd429adad1adcc3547c03b0ae26c9b9b5575&test_paths=dom%2Fmedia%2Fdoctor%2Ftest%2Fbrowser&selectedTaskRun=E1UylgrBSZGmweTf5NVFjg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=331221011&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/c0f907010345f4ef9c6593408e5226e625b260eb

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f471ed33aae
part1 : rename `MediaPlatformDecoderNotFound` to `MediaFFMpegNotFound`. r=bryce
https://hg.mozilla.org/integration/autoland/rev/cdc2e43b78b9
part2 : remove unnecessary operation of removing ffmpeg error flag. r=bryce
https://hg.mozilla.org/integration/autoland/rev/739bd8fadc35
part3 : Set different ffmpeg error in PDMFactory. r=bryce
https://hg.mozilla.org/integration/autoland/rev/f8a5b58b52cd
part4 : allow to set different error in all platforms, but still report them to corresponding platforms, except in testing. r=bryce
https://hg.mozilla.org/integration/autoland/rev/d43685e5850b
part5 : add test cases to test the ability of reporting error for not supported media type. r=bryce,emilio
https://hg.mozilla.org/integration/autoland/rev/a440c4bacde1
part6 : add test cases to test reporting decode error. r=bryce,webidl,emilio
https://hg.mozilla.org/integration/autoland/rev/c7afc5693e31
part7 : add test case for audio sink failed start-up. r=bryce,emilio
https://hg.mozilla.org/integration/autoland/rev/51161ef5f029
part8 : allow showing platform-decoder-not-found notification. r=bryce

Comment 13

[Cosmin Sabou [:CosminS]]

Relanded here ^
Failures were from Bug 1691499 https://hg.mozilla.org/integration/autoland/rev/36b76c196497ca9fa8ac4a95a651feffa5cad7ad
Sorry for the trouble.

Comment 14

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/6f471ed33aae
https://hg.mozilla.org/mozilla-central/rev/cdc2e43b78b9
https://hg.mozilla.org/mozilla-central/rev/739bd8fadc35
https://hg.mozilla.org/mozilla-central/rev/f8a5b58b52cd
https://hg.mozilla.org/mozilla-central/rev/d43685e5850b
https://hg.mozilla.org/mozilla-central/rev/a440c4bacde1
https://hg.mozilla.org/mozilla-central/rev/c7afc5693e31
https://hg.mozilla.org/mozilla-central/rev/51161ef5f029