LayoutTests/http/tests/serviceworker/postmessage-cross-process.html - Issue 730543005: Add a test for cross process service worker messaging.

Side by Side Diff: LayoutTests/http/tests/serviceworker/postmessage-cross-process.html

Issue 730543005: Add a test for cross process service worker messaging. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master

Patch Set: Created 6 years, 1 month ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « no previous file | LayoutTests/http/tests/serviceworker/resources/postmessage-cross-process-helper.html » ('j') | LayoutTests/http/tests/serviceworker/resources/postmessage-cross-process-helper.html » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <title>Service Worker: postMessage across processes</title>

	3 <script src="../resources/testharness.js"></script>

	4 <script src="../resources/testharnessreport.js"></script>

	5 <script src="resources/test-helpers.js"></script>

	6 <script>

	7 if (window.testRunner) {
	jsbell 2014/11/18 00:24:14 Since this is validating chrome-specific behavior Since this is validating chrome-specific behavior (cross-process communication) and references content_shell helpers (testRunner), the test should probably be in the chromium/ subdir Marijn Kruisselbrink 2014/11/18 01:16:13 Yeah, I guess this is kind of on the edge between Show quoted text On 2014/11/18 00:24:14, jsbell wrote: > Since this is validating chrome-specific behavior (cross-process communication) > and references content_shell helpers (testRunner), the test should probably be > in the chromium/ subdir Yeah, I guess this is kind of on the edge between generic and chrome specific behavior. Nothing it tests is really chrome specific. Being able to send messages to a port that is transferred to a service worker is a perfectly generic thing, and the fact that it does some trickery to run this code in a separate process in chrome (and the need for testRunner.setCanOpenWindows() because of that) shouldn't effect how well this runs in any other browser. It's unfortunate that chrome's testing infrastructure requires chrome specific code to do non chrome specific things like opening windows. The waitUntilDone in the helper html file can be easily eliminated by instead including testharnessreport.js, so the window thing is really the only part where chrome specific code has to be called. So on the one hand it would be nice to have a non-chrome-specific test that makes sure messages can be sent to ports sent to service workers, but I agree that the magic that is necessary to hit this particular bug in chrome might mean that this test should not be that non chrome specific test. Marijn Kruisselbrink 2014/11/20 20:41:12 And I've convinced myself that even though the tes And I've convinced myself that even though the test doesn't test anything chrome specific, it probably makes sense to at least for now just move the test to the chromium directory, since it seems some things would need to be cleaned up to be a proper w3c test (even though the test as is would run fine on any browser).
	8 testRunner.setCanOpenWindows();

	9 }

	10

	11 async_test(function(t) {

	12 var scope = 'resources/simple.html';

	13 var registration;

	14 service_worker_unregister_and_register(

	15 t, 'resources/postmessage-cross-process-worker.js', scope)

	16 .then(function(r) {

	17 registration = r;

	18 return wait_for_activated(t, registration);

	19 })

	20 .then(function() {

	21 return get_newest_worker(registration);

	22 })

	23 .then(function(worker) {

	24 var a = document.createElement('a');
	jsbell 2014/11/18 00:24:14 Maybe factor this out into a helper function? Maybe factor this out into a helper function? Marijn Kruisselbrink 2014/11/18 01:16:13 Done. Show quoted text On 2014/11/18 00:24:14, jsbell wrote: > Maybe factor this out into a helper function? Done.
	25 a.href = 'resources/postmessage-cross-process-helper.html';

	26 a.setAttribute('rel', 'noreferrer');
	jsbell 2014/11/18 00:24:14 I assume this is the magic that effectively forces I assume this is the magic that effectively forces a new process? Marijn Kruisselbrink 2014/11/18 01:16:13 Yes. Show quoted text On 2014/11/18 00:24:14, jsbell wrote: > I assume this is the magic that effectively forces a new process? Yes.
	27 a.target = '_blank';

	28 a.click();

	29

	30 var messageChannel = new MessageChannel();

	31 messageChannel.port1.onmessage = t.step_func(onMessage);

	32 worker.postMessage({resultport: messageChannel.port2}, [messageChannel .port2]);
	jsbell 2014/11/18 00:24:14 wrap at 80 columns wrap at 80 columns Marijn Kruisselbrink 2014/11/18 01:16:13 Done. Show quoted text On 2014/11/18 00:24:14, jsbell wrote: > wrap at 80 columns Done.
	33 })

	34 .catch(unreached_rejection(t));

	35

	36 var expected = [

	37 'Acking value: 1',

	38 'Acking value: 2',

	39 ];

	40

	41 function onMessage(e) {

	42 var result = e.data;

	43 assert_array_equals(result, expected,

	44 'Worker should post back expected values.');
	jsbell 2014/11/18 00:24:14 Can you explain more than the assertion already do Can you explain more than the assertion already does, e.g. "Worker should ack values posted by new window in order" ? Marijn Kruisselbrink 2014/11/18 01:16:13 Done. Show quoted text On 2014/11/18 00:24:14, jsbell wrote: > Can you explain more than the assertion already does, e.g. "Worker should ack > values posted by new window in order" ? Done.
	45 t.done();

	46 }

	47 }, 'postMessage MessagePorts from client to SW in a different process');

	48 </script>

OLD	NEW