ServiceWorker: Add layout test for WindowClient.navigate().

ServiceWorker: Add layout test for WindowClient.navigate().

This is a test for http://crrev.com/1196193003 and http://crrev.com/1202453002.

BUG=500911
Committed: https://crrev.com/2aefccceea614c7876882a240e5b900d18f97e9b
Cr-Commit-Position: refs/heads/master@{#353910}

[zino, 2015-10-07 02:31:14]

jinho.bang@samsung.com changed reviewers:
+ nhiroki@chromium.org

[zino, 2015-10-07 02:31:23]

PTAL

[zino, 2015-10-08 02:43:39]

PTAL

[nhiroki, 2015-10-08 09:19:06]

Looks good. nitpicks only.

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/windowclient-navigate-worker.js
(right):

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/windowclient-navigate-worker.js:7:
}
You could remove this hack if the document sends urls via "port1.postMessage"
instead of "worker.postMessage":

<doc>                           <worker>
worker.postMessage(port2)  -->  self.onmessage(port2)
port1.onmessage(msg)       <--  port2.postMessage("ready")
port1.postMessage(url1)    -->  port2.onmessage(url1)
port1.onmessage(res1)      <--  port2.postMessage(res1)
port1.postMessage(url2)    -->  port2.onmessage(url2)
...


self.onmessage = function(e) {
  port = e.data.port;
  port.onmessage = onmessage;
  port.postMessage('ready');
}

function onmessage(e) {
  var url = e.data;
  ...
}

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/windowclient-navigate-worker.js:15:
client= c;
nit: space before '='

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/windowclient-navigate-worker.js:17:
client.navigate(url).then(function(c) {
nit: To unindent, you might want to return a promise returned by navigate(url)
and chain 'then' block like this:

self.clients.matchAll().then(function(cs) {
      // snip
      return client.navigate(url);
    })
  .then(function(c) {
      // snip
    })
  .catch(function(e) {
      // snip
    });

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/windowclient-navigate.html
(right):

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/windowclient-navigate.html:54:
return with_iframe(init_url + '#client');
"#client" might be no longer necessary?

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/windowclient-navigate.html:75:
with_iframe(init_url + '#client').then(function(f) {
ditto (#client)

[zino, 2015-10-08 13:35:36]

Thank you for review!

PTAL again

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/windowclient-navigate-worker.js
(right):

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/windowclient-navigate-worker.js:7:
}
On 2015/10/08 09:19:06, nhiroki wrote:
> You could remove this hack if the document sends urls via "port1.postMessage"
> instead of "worker.postMessage":
> 
> <doc>                           <worker>
> worker.postMessage(port2)  -->  self.onmessage(port2)
> port1.onmessage(msg)       <--  port2.postMessage("ready")
> port1.postMessage(url1)    -->  port2.onmessage(url1)
> port1.onmessage(res1)      <--  port2.postMessage(res1)
> port1.postMessage(url2)    -->  port2.onmessage(url2)
> ...
> 
> 
> self.onmessage = function(e) {
>   port = e.data.port;
>   port.onmessage = onmessage;
>   port.postMessage('ready');
> }
> 
> function onmessage(e) {
>   var url = e.data;
>   ...
> }

Done.

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/windowclient-navigate-worker.js:15:
client= c;
On 2015/10/08 09:19:06, nhiroki wrote:
> nit: space before '='

Done.

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/windowclient-navigate-worker.js:17:
client.navigate(url).then(function(c) {
On 2015/10/08 09:19:06, nhiroki wrote:
> nit: To unindent, you might want to return a promise returned by navigate(url)
> and chain 'then' block like this:
> 
> self.clients.matchAll().then(function(cs) {
>       // snip
>       return client.navigate(url);
>     })
>   .then(function(c) {
>       // snip
>     })
>   .catch(function(e) {
>       // snip
>     });

Done.

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/serviceworker/windowclient-navigate.html
(right):

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/windowclient-navigate.html:54:
return with_iframe(init_url + '#client');
On 2015/10/08 09:19:06, nhiroki wrote:
> "#client" might be no longer necessary?

Done.

https://codereview.chromium.org/1390993002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/serviceworker/windowclient-navigate.html:75:
with_iframe(init_url + '#client').then(function(f) {
On 2015/10/08 09:19:06, nhiroki wrote:
> ditto (#client)

Done.

[zino, 2015-10-12 03:21:16]

Ping nhiroki@

Could you please review this?

Thank you :)

[zino, 2015-10-13 07:40:13]

nhiroki@,

Sorry for repeated requests.

Could you review this again?

Thank you!

[nhiroki, 2015-10-13 09:52:02]

lgtm (sorry for the delayed reply)

[zino, 2015-10-13 12:57:34]

On 2015/10/13 09:52:02, nhiroki wrote:
> lgtm (sorry for the delayed reply)

No problem. Thank you!

[zino, 2015-10-13 22:57:38]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2015-10-13 23:04:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1390993002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1390993002/20001

[commit-bot: I haz the power, 2015-10-13 23:45:51]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-13 23:46:47]

Patchset 2 (id:??) landed as
https://crrev.com/2aefccceea614c7876882a240e5b900d18f97e9b
Cr-Commit-Position: refs/heads/master@{#353910}