1107545 - [EME] Test case for off-main-thread GMPStorage APIs

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[JW Wang [:jwwang] [:jw_wang]]

Bug 1102607 makes GMPStorage APIs callable from all threads. Per bug 1102607 comment 14, we should have test cases for exercising the APIs in non-main threads.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1107545_part1_TestManager-v1.patch

Introduce a test manager to make storage tests scalable and easier to add more tests.

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1107545_part2_add_off_main_thread_tests-v1.patch

Add off-main-thread tests for GMPStorage APIs.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8532092 [details] [diff] [review]
1107545_part2_add_off_main_thread_tests-v1.patch

Review of attachment 8532092 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with nits fixed.

::: dom/media/gmp-plugin/gmp-test-decryptor.cpp
@@ +363,5 @@
> +  // Off-main-thread tests.
> +  if (g_platform_api->createthread(&thread) == GMPNoErr) {
> +    thread->Post(new TestStorageTask("thread1-", testManager));
> +    thread->Join();
> +  }

I think we should fail the test of the createthread function fails.

@@ +367,5 @@
> +  }
> +
> +  if (g_platform_api->createthread(&thread) == GMPNoErr) {
> +    thread->Post(new TestStorageTask("thread2-", testManager));
> +    thread->Join();

I think we should run both these cases concurrently. So we should have thread1 and thread2 variables, and post the tasks to both, and then join the threads after the tasks have been posted.

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8532092 [details] [diff] [review]
1107545_part2_add_off_main_thread_tests-v1.patch

Review of attachment 8532092 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/media/gmp-plugin/gmp-test-decryptor.cpp
@@ +363,5 @@
> +  // Off-main-thread tests.
> +  if (g_platform_api->createthread(&thread) == GMPNoErr) {
> +    thread->Post(new TestStorageTask("thread1-", testManager));
> +    thread->Join();
> +  }

By calling FakeDecryptor::Message("FAIL xxx") here?

@@ +367,5 @@
> +  }
> +
> +  if (g_platform_api->createthread(&thread) == GMPNoErr) {
> +    thread->Post(new TestStorageTask("thread2-", testManager));
> +    thread->Join();

Will do.

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to JW Wang [:jwwang] from comment #4)
> Comment on attachment 8532092 [details] [diff] [review]
> 1107545_part2_add_off_main_thread_tests-v1.patch
> 
> Review of attachment 8532092 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/media/gmp-plugin/gmp-test-decryptor.cpp
> @@ +363,5 @@
> > +  // Off-main-thread tests.
> > +  if (g_platform_api->createthread(&thread) == GMPNoErr) {
> > +    thread->Post(new TestStorageTask("thread1-", testManager));
> > +    thread->Join();
> > +  }
> 
> By calling FakeDecryptor::Message("FAIL xxx") here?

Yes.

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1107545_part2_add_off_main_thread_tests-v2.patch

Address nits in comment 3.

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Try: https://tbpl.mozilla.org/?tree=Try&rev=49e9268fc622

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1107545_part1_TestManager-v2.patch

rebase.

Comment 9

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1107545_part2_add_off_main_thread_tests-v3.patch

rebase.

try for gtests: https://tbpl.mozilla.org/?tree=Try&rev=cfccbd820ac0

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1185ff1ada05
https://hg.mozilla.org/integration/mozilla-inbound/rev/769d12f35376

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1185ff1ada05
https://hg.mozilla.org/mozilla-central/rev/769d12f35376