[eme] Convert lifetime tests to promise_tests

[eme] Convert lifetime tests to promise_tests

Now that there is support to count the number of active MediaKey
and MediaKeySession objects, update the tests to check the count
directly (there should be no such objects at the start of the test,
and if there are then something else is running and the counts
will be wrong anyway). By converting these tests to promise tests,
we get better logging if the test fails.

BUG=445324
TEST=updated tests pass

Review-Url: https://codereview.chromium.org/2618603002
Cr-Commit-Position: refs/heads/master@{#443113}
Committed: https://chromium.googlesource.com/chromium/src/+/284ba384594565e8bdfdc25aad3cecf4f55ba16b

[jrummell, 2017-01-05 00:40:15]

The CQ bit was checked by jrummell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 00:40:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jrummell, 2017-01-05 00:43:43]

jrummell@chromium.org changed reviewers:
+ xhwang@chromium.org

[jrummell, 2017-01-05 00:43:44]

PTAL.

[commit-bot: I haz the power, 2017-01-05 02:52:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-05 02:52:46]

Dry run: This issue passed the CQ dry run.

[xhwang, 2017-01-05 20:05:06]

lg, just a few nits

https://codereview.chromium.org/2618603002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html
(right):

https://codereview.chromium.org/2618603002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html:26:
return createGCPromise().then(function() {
Could you please add a comment about why we need to run GC every time at the
start of the test?

https://codereview.chromium.org/2618603002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeysession-reference.html
(right):

https://codereview.chromium.org/2618603002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeysession-reference.html:39:
assert_equals(window.internals.mediaKeySessionCount(), 0, 'After final gc()');
There two are used in multiple places in multiple tests. Does it make sense to
add two helper functions in encrypted-media-utils.js to make it shorter?

e.g. 

function mediaKeysCount()
{
  return window.internals.mediaKeysCount();
}

[jrummell, 2017-01-06 22:10:18]

Updated.

https://codereview.chromium.org/2618603002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html
(right):

https://codereview.chromium.org/2618603002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeys-with-session.html:26:
return createGCPromise().then(function() {
On 2017/01/05 20:05:06, xhwang wrote:
> Could you please add a comment about why we need to run GC every time at the
> start of the test?

Done.

https://codereview.chromium.org/2618603002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeysession-reference.html
(right):

https://codereview.chromium.org/2618603002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeysession-reference.html:39:
assert_equals(window.internals.mediaKeySessionCount(), 0, 'After final gc()');
On 2017/01/05 20:05:06, xhwang wrote:
> There two are used in multiple places in multiple tests. Does it make sense to
> add two helper functions in encrypted-media-utils.js to make it shorter?
> 
> e.g. 
> 
> function mediaKeysCount()
> {
>   return window.internals.mediaKeysCount();
> }

Not sure that a shorter name helps that much. However, I moved the asserts into
a separate function. (Note that the assert output doesn't contain line numbers,
so |description| is important to determine which assert failed. Now output looks
like "FAIL MediaKeys lifetime with session assert_equals: After gc(),
MediaKeySession: expected 2 but got 1").

[xhwang, 2017-01-09 17:57:17]

https://codereview.chromium.org/2618603002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeysession-reference.html
(right):

https://codereview.chromium.org/2618603002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-lifetime-mediakeysession-reference.html:39:
assert_equals(window.internals.mediaKeySessionCount(), 0, 'After final gc()');
On 2017/01/06 22:10:18, jrummell wrote:
> On 2017/01/05 20:05:06, xhwang wrote:
> > There two are used in multiple places in multiple tests. Does it make sense
to
> > add two helper functions in encrypted-media-utils.js to make it shorter?
> > 
> > e.g. 
> > 
> > function mediaKeysCount()
> > {
> >   return window.internals.mediaKeysCount();
> > }
> 
> Not sure that a shorter name helps that much. However, I moved the asserts
into
> a separate function. (Note that the assert output doesn't contain line
numbers,
> so |description| is important to determine which assert failed. Now output
looks
> like "FAIL MediaKeys lifetime with session assert_equals: After gc(),
> MediaKeySession: expected 2 but got 1").

This looks much cleaner!

Does it make sense to move this helper function to encrypted-media-utils.js to
further reduce duplication?

[jrummell, 2017-01-09 18:45:49]

On 2017/01/09 17:57:17, xhwang wrote:

> Does it make sense to move this helper function to encrypted-media-utils.js to
> further reduce duplication?

I tried that. assert() comes from testharness.js, so it's not available in
utils.

[xhwang, 2017-01-10 00:36:24]

On 2017/01/09 18:45:49, jrummell wrote:
> On 2017/01/09 17:57:17, xhwang wrote:
> 
> > Does it make sense to move this helper function to encrypted-media-utils.js
to
> > further reduce duplication?
> 
> I tried that. assert() comes from testharness.js, so it's not available in
> utils.

I see. But it seems we already use assert_*() in encrypted-media-utils.js:

https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/media/enc...
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/media/enc...

Do we need to add testharness.js in encrypted-media-utils.js?

[jrummell, 2017-01-11 21:26:08]

The CQ bit was checked by jrummell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 21:27:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jrummell, 2017-01-11 21:28:54]

> I see. But it seems we already use assert_*() in encrypted-media-utils.js:
> 
>
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/media/enc...
>
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/media/enc...
> 
> Do we need to add testharness.js in encrypted-media-utils.js?

I guess JavaScript doesn't care until call time. So moved the function to
encrypted-media-utils.js. Not sure how to include testharness.js, but given it's
OK for the other calls to assert, it should be fine for this too.

[xhwang, 2017-01-11 21:53:27]

Thanks! LGTM

[commit-bot: I haz the power, 2017-01-11 22:46:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-11 22:46:33]

Dry run: This issue passed the CQ dry run.

[jrummell, 2017-01-12 01:28:53]

The CQ bit was checked by jrummell@chromium.org

[commit-bot: I haz the power, 2017-01-12 01:29:33]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-12 01:37:26]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484184533918460,
"parent_rev": "57c86c65d60a0e5da917fa3cb6c6b374821b7984", "commit_rev":
"284ba384594565e8bdfdc25aad3cecf4f55ba16b"}

[commit-bot: I haz the power, 2017-01-12 01:37:57]

Description was changed from

==========
[eme] Convert lifetime tests to promise_tests

Now that there is support to count the number of active MediaKey
and MediaKeySession objects, update the tests to check the count
directly (there should be no such objects at the start of the test,
and if there are then something else is running and the counts
will be wrong anyway). By converting these tests to promise tests,
we get better logging if the test fails.

BUG=445324
TEST=updated tests pass
==========

to

==========
[eme] Convert lifetime tests to promise_tests

Now that there is support to count the number of active MediaKey
and MediaKeySession objects, update the tests to check the count
directly (there should be no such objects at the start of the test,
and if there are then something else is running and the counts
will be wrong anyway). By converting these tests to promise tests,
we get better logging if the test fails.

BUG=445324
TEST=updated tests pass

Review-Url: https://codereview.chromium.org/2618603002
Cr-Commit-Position: refs/heads/master@{#443113}
Committed:
https://chromium.googlesource.com/chromium/src/+/284ba384594565e8bdfdc25aad3c...
==========

[commit-bot: I haz the power, 2017-01-12 01:37:58]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/284ba384594565e8bdfdc25aad3c...