Bug 180430 – [EME] Add layout test for InitData and InitDataType in CENC encrypted event

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Yacine Bandou]

This layout test, tests the receipt of the encrypted event with a right initData and InitDataType fields in CENC protection scheme.

[Yacine Bandou]

Created attachment 328467 [details]
Patch

[EWS Watchlist]

Comment on attachment 328467 [details]
Patch

Attachment 328467 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5503704

New failing tests:
media/encrypted-media/clearKey/clearKey-encrypted-cenc-event.html

[EWS Watchlist]

Created attachment 328479 [details]
Archive of layout-test-results from ews103 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 328467 [details]
Patch

Attachment 328467 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5504275

New failing tests:
media/encrypted-media/clearKey/clearKey-encrypted-cenc-event.html

[EWS Watchlist]

Created attachment 328486 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 328467 [details]
Patch

Attachment 328467 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5506637

New failing tests:
media/encrypted-media/clearKey/clearKey-encrypted-cenc-event.html

[EWS Watchlist]

Created attachment 328510 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Yacine Bandou]

This layout test fails in mac-elcapitan platforms because we are not receiving the encrypted event from EME implementation.

Should I add in TestExpectations of these platforms that the test is expected to fail?

[Jer Noble]

(In reply to Yacine Bandou from comment #8)
> This layout test fails in mac-elcapitan platforms because we are not
> receiving the encrypted event from EME implementation.
> 
> Should I add in TestExpectations of these platforms that the test is
> expected to fail?

Yes, go ahead and add a TestExpectation for Sierra and El Capitan; there should be a few entries there already.

[Yacine Bandou]

Created attachment 328897 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 328897 [details]
Patch

As discussed privately I see two issues here. First is that we should use current MSE test architecture instead of creating a new one. Second is that for this case MSE is not needed so we can save time by using the regular player instead of the MSE one.

[Jer Noble]

(In reply to Xabier Rodríguez Calvar from comment #11)
> Comment on attachment 328897 [details]
> Patch
> 
> As discussed privately I see two issues here. First is that we should use
> current MSE test architecture instead of creating a new one. Second is that
> for this case MSE is not needed so we can save time by using the regular
> player instead of the MSE one.

Okay, the unfortunate part is that the Mac port will never be able to support EME through the standard flat-file, non-HLS, non-MSE path. So please just make a global platform/mac/TestExpectations entry (rather than an El Capitan one) for this test.

[Yacine Bandou]

Created attachment 329000 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 329000 [details]
Patch

(In reply to Jer Noble from comment #12)
> Okay, the unfortunate part is that the Mac port will never be able to
> support EME through the standard flat-file, non-HLS, non-MSE path. So please
> just make a global platform/mac/TestExpectations entry (rather than an El
> Capitan one) for this test.

Ok, there is no problem in running it though MSE if it's better for you too, I think we can go with the second option of patch with some small changes:
* place the file under media-source instead of encrypted-media and call it media-source-loader-simple.js
* Rename the objects to MediaSourceLoaderSimple and SourceBufferLoaderSimple.
* In the tests changelog remove the JS functions cause they are not interesting and be a bit more verbose about the files you create. It would be nice to know what medias-enc.js, the media itself and the simple loader includes.

[Jer Noble]

(In reply to Xabier Rodríguez Calvar from comment #14)
> Comment on attachment 329000 [details]
> Patch
> 
> (In reply to Jer Noble from comment #12)
> > Okay, the unfortunate part is that the Mac port will never be able to
> > support EME through the standard flat-file, non-HLS, non-MSE path. So please
> > just make a global platform/mac/TestExpectations entry (rather than an El
> > Capitan one) for this test.
> 
> Ok, there is no problem in running it though MSE if it's better for you too,
> I think we can go with the second option of patch with some small changes:
> * place the file under media-source instead of encrypted-media and call it
> media-source-loader-simple.js
> * Rename the objects to MediaSourceLoaderSimple and SourceBufferLoaderSimple.
> * In the tests changelog remove the JS functions cause they are not
> interesting and be a bit more verbose about the files you create. It would
> be nice to know what medias-enc.js, the media itself and the simple loader
> includes.

I don't see a problem with having two tests, one for the simple loading path, and one for the MSE one. We'd just have to skip/fail the simple loading path for Mac. But I'm sure a simple path test would have value for GTK+.

[Yacine Bandou]

Created attachment 329230 [details]
Patch

[Yacine Bandou]

This patch above adds two tests, one for normal playback and the other for MSE playback, as suggested by Jer.

[EWS Watchlist]

Comment on attachment 329230 [details]
Patch

Attachment 329230 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/5647412

New failing tests:
media/encrypted-media/clearKey/clearKey-encrypted-cenc-event-mse.html

[EWS Watchlist]

Created attachment 329236 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[EWS Watchlist]

Comment on attachment 329230 [details]
Patch

Attachment 329230 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/5647711

New failing tests:
media/encrypted-media/clearKey/clearKey-encrypted-cenc-event-mse.html

[EWS Watchlist]

Created attachment 329241 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Yacine Bandou]

Created attachment 329269 [details]
Patch

[Xabier Rodríguez Calvar]

Comment on attachment 329269 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=329269&action=review

> LayoutTests/platform/mac-elcapitan/TestExpectations:11
> +media/encrypted-media/clearKey/clearKey-encrypted-cenc-event-mse.html [ Skip ]

Please, make this [ Failure ] as they are supposed to work some day.

> LayoutTests/platform/mac-wk2/TestExpectations:520
> +media/encrypted-media/clearKey/clearKey-encrypted-cenc-event-mse.html [ Skip ]

Please, make this [ Failure ] as they are supposed to work some day.

[Yacine Bandou]

Created attachment 329337 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 329337 [details]
Patch

Clearing flags on attachment: 329337

Committed r225899: <https://trac.webkit.org/changeset/225899>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/36045367>

[Miguel Gomez]

Reopening to attach new patch.

[Miguel Gomez]

Created attachment 329345 [details]
Patch

[Miguel Gomez]

Comment on attachment 329345 [details]
Patch

Sorry, not sure how this patch ended in this bug!!

[Xabier Rodríguez Calvar]

(In reply to WebKit Commit Bot from comment #26)
> All reviewed patches have been landed.  Closing bug.

[Matt Lewis]

The test media/encrypted-media/clearKey/clearKey-encrypted-cenc-event-mse.html is timing out on multiple platforms and failing on others:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Fencrypted-media%2FclearKey%2FclearKey-encrypted-cenc-event-mse.html


Judging from the comments the expectations for this rather than putting the test in the  mac-elcapitan/TestExpectations, we should just mark specific OSs within the top level Mac expectations.

EX:

webkit.org/b/180071 [ ElCapitan ] media/encrypted-media/clearKey/clearKey-encrypted-cenc-event-mse.html [ Failure ]

Also it helps to have the bug for every expectation file in case they get changed for some reason
Due to the fact that the test is timing out on multiple Mac platforms, rather than marking it as failing, I'm marking it as skip:

https://trac.webkit.org/changeset/225942/webkit

[Michael Catanzaro]

This test is timing out on GTK since it was added in r225899, not failing. Additionally, it looks like Matt's comments above were not addressed. I'm going to roll this out. Please don't intentionally add timeouts: skip the test if need be.

[Michael Catanzaro]

Committed r226278: <https://trac.webkit.org/changeset/226278>

[Michael Catanzaro]

Reopening.

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #33)
> This test is timing out on GTK since it was added in r225899, not failing.
> Additionally, it looks like Matt's comments above were not addressed. I'm
> going to roll this out. Please don't intentionally add timeouts: skip the
> test if need be.

Should probably be skipped in the global TestExpectations if it's expected to be broken on so many platforms.

[Yacine Bandou]

(In reply to Michael Catanzaro from comment #36)
> (In reply to Michael Catanzaro from comment #33)
> > This test is timing out on GTK since it was added in r225899, not failing.
> > Additionally, it looks like Matt's comments above were not addressed. I'm
> > going to roll this out. Please don't intentionally add timeouts: skip the
> > test if need be.
> 
> Should probably be skipped in the global TestExpectations if it's expected
> to be broken on so many platforms.

This test is currently correctly passing in the WPE platform since r225642, see bug 180071.

I do not understand why you removed this test instead of update TestExpectations.

[Yacine Bandou]

Created attachment 330163 [details]
Patch

[Michael Catanzaro]

Hi, I had no clue what the expectations were supposed to be, except that what you had committed wasn't right. As a rule, if your commit introduces a regression or test failure, you can expect it to be rolled out.

Anyway, thanks for updating the patch. Skipping the new tests everywhere except for WPE seems sane.

[Yacine Bandou]

(In reply to Michael Catanzaro from comment #39)
> Hi, I had no clue what the expectations were supposed to be, except that
> what you had committed wasn't right. As a rule, if your commit introduces a
> regression or test failure, you can expect it to be rolled out.
> 
Understood, thanks.

[EWS Watchlist]

Comment on attachment 330163 [details]
Patch

Attachment 330163 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/5813695

New failing tests:
imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https.html

[EWS Watchlist]

Created attachment 330164 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6

[Yacine Bandou]

(In reply to Build Bot from comment #41)
> Comment on attachment 330163 [details]
> Patch
> 
> Attachment 330163 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.webkit.org/results/5813695
> 
> New failing tests:
> imported/w3c/web-platform-tests/service-workers/service-worker/register-same-
> scope-different-script-url.https.html

I think, this failure already exists, it is independent of my patch.

[Michael Catanzaro]

Comment on attachment 330163 [details]
Patch

Yes, the unrelated test is flaky. You can ignore it.

[Yacine Bandou]

Who can review and land this patch ?

[Xabier Rodríguez Calvar]

Comment on attachment 330163 [details]
Patch

(In reply to Michael Catanzaro from comment #39)
> Hi, I had no clue what the expectations were supposed to be, except that
> what you had committed wasn't right. As a rule, if your commit introduces a
> regression or test failure, you can expect it to be rolled out.
> 
> Anyway, thanks for updating the patch. Skipping the new tests everywhere
> except for WPE seems sane.

[WebKit Commit Bot]

Comment on attachment 330163 [details]
Patch

Clearing flags on attachment: 330163

Committed r226518: <https://trac.webkit.org/changeset/226518>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.