Add LayoutTests to test Blob URL transfer to {Shared,Service}Workers

Add LayoutTests to test Blob URL transfer to {Shared,Service}Workers

In follow-up to: https://codereview.chromium.org/2422793002/

BUG=n/a
TEST=fast/workers/shared-worker-blob-url.html
TEST=http/tests/serviceworker/postmessage-blob-url.html

Review-Url: https://codereview.chromium.org/2695093003
Cr-Commit-Position: refs/heads/master@{#450889}
Committed: https://chromium.googlesource.com/chromium/src/+/707342cb16f9258ec335b5f04d44a5ddc6bd1032

[kinuko, 2017-02-15 01:52:28]

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

[kinuko, 2017-02-15 01:52:41]

Description was changed from

==========
Add LayoutTests to test Blob URL transfer to {Shared,Service}Workers

In follow-up to: https://codereview.chromium.org/2422793002/

BUG=n/a
TEST=fast/workers/shared-worker-blob-url.html
TEST=http/tests/serviceworker/postmessage-blob-url.html
==========

to

==========
Add LayoutTests to test Blob URL transfer to {Shared,Service}Workers

In follow-up to: https://codereview.chromium.org/2422793002/

BUG=n/a
TEST=fast/workers/shared-worker-blob-url.html
TEST=http/tests/serviceworker/postmessage-blob-url.html
==========

[kinuko, 2017-02-15 01:52:42]

kinuko@chromium.org changed reviewers:
+ falken@chromium.org, mek@chromium.org

[commit-bot: I haz the power, 2017-02-15 01:53:45]

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

[kinuko, 2017-02-15 01:57:05]

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

[kinuko, 2017-02-15 01:57:10]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-02-15 01:57:42]

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

[kinuko, 2017-02-15 01:59:46]

Hey, could you review so that we won't forget these use cases? (I wasn't been
able to find similar tests but let me know if I'm wrong-- thanks!)

[commit-bot: I haz the power, 2017-02-15 03:43:54]

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

[commit-bot: I haz the power, 2017-02-15 03:43:55]

Dry run: This issue passed the CQ dry run.

[falken, 2017-02-15 04:46:06]

Could we commit these tests to external/wpt directly?

[blink-dev] PSA: Commit your WPT changes directly to LayoutTests/external/wpt

https://codereview.chromium.org/2695093003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-blob-url.html
(right):

https://codereview.chromium.org/2695093003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-blob-url.html:30:
});
would it be simpler to just do worker.postMessage() and have the worker do
event.source.postMessage() back, instead of the MessageChannel? "messageChannel"
has to be repeated like 5 times so I try to avoid it.

also if using MessageChannel, I don't think you need to pass "port" as part of
the message data. In onmessage the worker can do e.ports[0].postMessage.

https://codereview.chromium.org/2695093003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-blob-url.html:33:
assert_equals(e.data, 'Connected');
Do we need the "Connected" step? Can't we send blobUrl as the first postMessage?

[kinuko, 2017-02-15 05:43:45]

On 2017/02/15 04:46:06, falken wrote:
> Could we commit these tests to external/wpt directly?

Oh yeah... thanks, we should.

> [blink-dev] PSA: Commit your WPT changes directly to LayoutTests/external/wpt
> 
>
https://codereview.chromium.org/2695093003/diff/20001/third_party/WebKit/Layo...
> File
>
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-blob-url.html
> (right):
> 
>
https://codereview.chromium.org/2695093003/diff/20001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-blob-url.html:30:
> });
> would it be simpler to just do worker.postMessage() and have the worker do
> event.source.postMessage() back, instead of the MessageChannel?
"messageChannel"
> has to be repeated like 5 times so I try to avoid it.
> 
> also if using MessageChannel, I don't think you need to pass "port" as part of
> the message data. In onmessage the worker can do e.ports[0].postMessage.

Yeah... the patch that was in discussion touches MessageChannel code so I
thought we might want to explicitly test it.

>
https://codereview.chromium.org/2695093003/diff/20001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-blob-url.html:33:
> assert_equals(e.data, 'Connected');
> Do we need the "Connected" step? Can't we send blobUrl as the first
postMessage?

We can remove the step, but I expect message delivery (R -> B -> R) and blob URL
creation (R -> B and then another R reads it) become more racy this way (which
is the motivation to have this test).

[kinuko, 2017-02-15 06:28:57]

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

[commit-bot: I haz the power, 2017-02-15 06:29:24]

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

[kinuko, 2017-02-15 07:03:31]

Patchset #2 (id:40001) has been deleted

[kinuko, 2017-02-15 07:07:57]

Patchset #2 (id:60001) has been deleted

[kinuko, 2017-02-15 07:09:49]

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

[commit-bot: I haz the power, 2017-02-15 07:10:00]

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

[kinuko, 2017-02-15 07:10:16]

Thanks, updated.

https://codereview.chromium.org/2695093003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-blob-url.html
(right):

https://codereview.chromium.org/2695093003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-blob-url.html:30:
});
On 2017/02/15 04:46:06, falken wrote:
> would it be simpler to just do worker.postMessage() and have the worker do
> event.source.postMessage() back, instead of the MessageChannel?
"messageChannel"
> has to be repeated like 5 times so I try to avoid it.
> 
> also if using MessageChannel, I don't think you need to pass "port" as part of
> the message data. In onmessage the worker can do e.ports[0].postMessage.

Done.

https://codereview.chromium.org/2695093003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/serviceworker/postmessage-blob-url.html:33:
assert_equals(e.data, 'Connected');
On 2017/02/15 04:46:06, falken wrote:
> Do we need the "Connected" step? Can't we send blobUrl as the first
postMessage?

Well, I ended up removing this step.

[commit-bot: I haz the power, 2017-02-15 07:12:11]

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

[commit-bot: I haz the power, 2017-02-15 07:12:12]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[falken, 2017-02-15 07:19:09]

ah sorry didn't know if you were waiting for a reply.

lgtm. I'm not familiar with MANIFEST.json but I assume it's an automated change.

[kinuko, 2017-02-15 07:31:49]

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

[commit-bot: I haz the power, 2017-02-15 07:31:59]

Dry run: 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-02-15 08:51:20]

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

[commit-bot: I haz the power, 2017-02-15 08:51:20]

Dry run: This issue passed the CQ dry run.

[Marijn Kruisselbrink, 2017-02-15 18:18:55]

lgtm

https://codereview.chromium.org/2695093003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/postmessage-blob-url.https.html
(right):

https://codereview.chromium.org/2695093003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/postmessage-blob-url.https.html:30:
return registration.unregister(scope);
nit: no scope parameter needed for ServiceWorkerRegistration.unregister

https://codereview.chromium.org/2695093003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/workers/SharedWorker_blobUrl.html
(right):

https://codereview.chromium.org/2695093003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/workers/SharedWorker_blobUrl.html:17:
worker.port.start();
nit: You shouldn't need to call start(), assigning to onmessage below
automatically starts the port.

[kinuko, 2017-02-16 03:05:21]

Thanks!

https://codereview.chromium.org/2695093003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/postmessage-blob-url.https.html
(right):

https://codereview.chromium.org/2695093003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/postmessage-blob-url.https.html:30:
return registration.unregister(scope);
On 2017/02/15 18:18:55, Marijn Kruisselbrink wrote:
> nit: no scope parameter needed for ServiceWorkerRegistration.unregister

Done.

https://codereview.chromium.org/2695093003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/external/wpt/workers/SharedWorker_blobUrl.html
(right):

https://codereview.chromium.org/2695093003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/external/wpt/workers/SharedWorker_blobUrl.html:17:
worker.port.start();
On 2017/02/15 18:18:55, Marijn Kruisselbrink wrote:
> nit: You shouldn't need to call start(), assigning to onmessage below
> automatically starts the port.

Done.

[kinuko, 2017-02-16 03:05:54]

The CQ bit was checked by kinuko@chromium.org

[kinuko, 2017-02-16 03:05:55]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org,
mek@chromium.org
Link to the patchset: https://codereview.chromium.org/2695093003/#ps140001
(title: ".")

[commit-bot: I haz the power, 2017-02-16 03:07:17]

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-02-16 03:58:40]

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

[commit-bot: I haz the power, 2017-02-16 03:58:42]

Try jobs failed on following builders:
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)

[kinuko, 2017-02-16 04:01:38]

The CQ bit was checked by kinuko@chromium.org

[commit-bot: I haz the power, 2017-02-16 04:03:40]

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-02-16 05:09:54]

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

[commit-bot: I haz the power, 2017-02-16 05:09:56]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)

[kinuko, 2017-02-16 06:53:51]

The CQ bit was checked by kinuko@chromium.org

[commit-bot: I haz the power, 2017-02-16 06:54:53]

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-02-16 07:44:19]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1487228031458990,
"parent_rev": "e5269741ff2995db282752d3b15d59e456818d06", "commit_rev":
"707342cb16f9258ec335b5f04d44a5ddc6bd1032"}

[commit-bot: I haz the power, 2017-02-16 07:45:29]

Description was changed from

==========
Add LayoutTests to test Blob URL transfer to {Shared,Service}Workers

In follow-up to: https://codereview.chromium.org/2422793002/

BUG=n/a
TEST=fast/workers/shared-worker-blob-url.html
TEST=http/tests/serviceworker/postmessage-blob-url.html
==========

to

==========
Add LayoutTests to test Blob URL transfer to {Shared,Service}Workers

In follow-up to: https://codereview.chromium.org/2422793002/

BUG=n/a
TEST=fast/workers/shared-worker-blob-url.html
TEST=http/tests/serviceworker/postmessage-blob-url.html

Review-Url: https://codereview.chromium.org/2695093003
Cr-Commit-Position: refs/heads/master@{#450889}
Committed:
https://chromium.googlesource.com/chromium/src/+/707342cb16f9258ec335b5f04d44...
==========

[commit-bot: I haz the power, 2017-02-16 07:45:31]

Committed patchset #5 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/707342cb16f9258ec335b5f04d44...