ServiceWorker: Shows a warning when adding event handlers asynchronously

ServiceWorker: Shows a warning when adding event handlers asynchronously

As shown at 15 on [1], "The user agents are encouraged to show a warning that
the event listeners must be added on the very first evaluation of the worker
script.". This patch adds warning messages for the events.

[1]: https://w3c.github.io/ServiceWorker/spec/service_worker/index.html#run-service-worker-algorithm

BUG=594160
Committed: https://crrev.com/e14cea541b991c7907123395ea663f39eea7dcfa
Cr-Commit-Position: refs/heads/master@{#419967}

[shimazu, 2016-09-08 04:20:41]

shimazu@chromium.org changed reviewers:
+ falken@chromium.org

[shimazu, 2016-09-08 04:20:42]

ptal

[falken, 2016-09-09 07:49:16]

Sorry to get to this late.

Is there any reason not to just show this warning for all event types?

[shimazu, 2016-09-14 03:49:13]

On 2016/09/09 07:49:16, falken wrote:
> Sorry to get to this late.
> 
> Is there any reason not to just show this warning for all event types?

No, that's because just I didn't understand the meaning of 'the event listeners'
(I thought 'the' meant listeners related to the service workers)!
Updated.

[falken, 2016-09-14 03:56:24]

Thanks! Can you dupe bugs 594160 and 644617 together and update the CL
description?

Also I think we now understand more about how to test things that happen in
DevTools. Is it possible to write a test for this (probably in
http/inspector/service-workers)?

[shimazu, 2016-09-14 08:26:08]

Description was changed from

==========
Shows a warning when adding fetch handler after the first evaluation

As shown at 15 on [1], "The user agents are encouraged to show a warning that
the event listeners must be added on the very first evaluation of the worker
script.". This patch adds warning messages for fetch event and foreignfetch
event.

[1]:
https://w3c.github.io/ServiceWorker/spec/service_worker/index.html#run-servic...

BUG=644617
==========

to

==========
Shows a warning when adding event handlers after the first evaluation

As shown at 15 on [1], "The user agents are encouraged to show a warning that
the event listeners must be added on the very first evaluation of the worker
script.". This patch adds warning messages for the events.

[1]:
https://w3c.github.io/ServiceWorker/spec/service_worker/index.html#run-servic...

BUG=644617
==========

[shimazu, 2016-09-14 08:26:43]

Description was changed from

==========
Shows a warning when adding event handlers after the first evaluation

As shown at 15 on [1], "The user agents are encouraged to show a warning that
the event listeners must be added on the very first evaluation of the worker
script.". This patch adds warning messages for the events.

[1]:
https://w3c.github.io/ServiceWorker/spec/service_worker/index.html#run-servic...

BUG=644617
==========

to

==========
Shows a warning when adding event handlers after the first evaluation

As shown at 15 on [1], "The user agents are encouraged to show a warning that
the event listeners must be added on the very first evaluation of the worker
script.". This patch adds warning messages for the events.

[1]:
https://w3c.github.io/ServiceWorker/spec/service_worker/index.html#run-servic...

BUG=594160
==========

[shimazu, 2016-09-15 09:50:55]

On 2016/09/14 03:56:24, falken wrote:
> Thanks! Can you dupe bugs 594160 and 644617 together and update the CL
> description?
> 
> Also I think we now understand more about how to test things that happen in
> DevTools. Is it possible to write a test for this (probably in
> http/inspector/service-workers)?

Merged and added a test:)
ptal again

[falken, 2016-09-20 01:46:41]

Thanks for the test!

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/service-workers/lazy-addeventlisteners.html
(right):

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/service-workers/lazy-addeventlisteners.html:20:
<p>Warning should be shown at the console when calling addEventListener after
the first evaluation.</p>
"the first evaluation" is ambiguous.

"Tests that a warning is shown in the console if addEventListener is called
after initial evaluation of the service worker script."

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/service-workers/resources/service-worker-lazy-addeventlistener.js
(right):

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/service-workers/resources/service-worker-lazy-addeventlistener.js:6:
}, 10);
setTimeout is always unfortunate: it places a lower bound on the time required
for the test to complete. Please use setTimeout 0 or redesign the test to not
require it: e.g., call addEventListener() inside an install event handler.

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/service-workers/resources/service-worker-lazy-addeventlistener.js:7:
console.log('Here is in the first execution.');
I think you could simplify the test by removing these console.logs.

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp
(right):

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp:163:
String message = String::format("Event handler of '%s' event must be added on
the initial evaluation of worker script.", eventType.ascii().data());
Maybe I'm paranoid but I'd just utf8() instead of ascii(), performance isn't an
issue for this console warning.

[shimazu, 2016-09-20 06:07:52]

Updated.
PTAL again! :)

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/service-workers/lazy-addeventlisteners.html
(right):

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/service-workers/lazy-addeventlisteners.html:20:
<p>Warning should be shown at the console when calling addEventListener after
the first evaluation.</p>
On 2016/09/20 01:46:41, falken wrote:
> "the first evaluation" is ambiguous.
> 
> "Tests that a warning is shown in the console if addEventListener is called
> after initial evaluation of the service worker script."

Done.

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/service-workers/resources/service-worker-lazy-addeventlistener.js
(right):

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/service-workers/resources/service-worker-lazy-addeventlistener.js:6:
}, 10);
On 2016/09/20 01:46:41, falken wrote:
> setTimeout is always unfortunate: it places a lower bound on the time required
> for the test to complete. Please use setTimeout 0 or redesign the test to not
> require it: e.g., call addEventListener() inside an install event handler.

Done.

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/service-workers/resources/service-worker-lazy-addeventlistener.js:7:
console.log('Here is in the first execution.');
On 2016/09/20 01:46:41, falken wrote:
> I think you could simplify the test by removing these console.logs.

Done.

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp
(right):

https://codereview.chromium.org/2319563003/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp:163:
String message = String::format("Event handler of '%s' event must be added on
the initial evaluation of worker script.", eventType.ascii().data());
On 2016/09/20 01:46:41, falken wrote:
> Maybe I'm paranoid but I'd just utf8() instead of ascii(), performance isn't
an
> issue for this console warning.

Done.

[falken, 2016-09-20 06:10:53]

lgtm, thank you.

CL description nit: add something on the first line to say this commit is about
service workers.

[shimazu, 2016-09-20 06:19:25]

Description was changed from

==========
Shows a warning when adding event handlers after the first evaluation

As shown at 15 on [1], "The user agents are encouraged to show a warning that
the event listeners must be added on the very first evaluation of the worker
script.". This patch adds warning messages for the events.

[1]:
https://w3c.github.io/ServiceWorker/spec/service_worker/index.html#run-servic...

BUG=594160
==========

to

==========
ServiceWorker: Shows a warning when adding event handlers asynchronously

As shown at 15 on [1], "The user agents are encouraged to show a warning that
the event listeners must be added on the very first evaluation of the worker
script.". This patch adds warning messages for the events.

[1]:
https://w3c.github.io/ServiceWorker/spec/service_worker/index.html#run-servic...

BUG=594160
==========

[shimazu, 2016-09-20 06:20:15]

The CQ bit was checked by shimazu@chromium.org

[commit-bot: I haz the power, 2016-09-20 06:20:26]

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, 2016-09-20 07:34:41]

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

[commit-bot: I haz the power, 2016-09-20 07:34:42]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[shimazu, 2016-09-21 02:27:27]

The CQ bit was checked by shimazu@chromium.org

[shimazu, 2016-09-21 02:27:27]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org
Link to the patchset: https://codereview.chromium.org/2319563003/#ps120001
(title: "Rebased and updated the expectation")

[commit-bot: I haz the power, 2016-09-21 02:27:46]

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, 2016-09-21 03:47:15]

Description was changed from

==========
ServiceWorker: Shows a warning when adding event handlers asynchronously

As shown at 15 on [1], "The user agents are encouraged to show a warning that
the event listeners must be added on the very first evaluation of the worker
script.". This patch adds warning messages for the events.

[1]:
https://w3c.github.io/ServiceWorker/spec/service_worker/index.html#run-servic...

BUG=594160
==========

to

==========
ServiceWorker: Shows a warning when adding event handlers asynchronously

As shown at 15 on [1], "The user agents are encouraged to show a warning that
the event listeners must be added on the very first evaluation of the worker
script.". This patch adds warning messages for the events.

[1]:
https://w3c.github.io/ServiceWorker/spec/service_worker/index.html#run-servic...

BUG=594160
==========

[commit-bot: I haz the power, 2016-09-21 03:47:17]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-09-21 03:48:58]

Description was changed from

==========
ServiceWorker: Shows a warning when adding event handlers asynchronously

As shown at 15 on [1], "The user agents are encouraged to show a warning that
the event listeners must be added on the very first evaluation of the worker
script.". This patch adds warning messages for the events.

[1]:
https://w3c.github.io/ServiceWorker/spec/service_worker/index.html#run-servic...

BUG=594160
==========

to

==========
ServiceWorker: Shows a warning when adding event handlers asynchronously

As shown at 15 on [1], "The user agents are encouraged to show a warning that
the event listeners must be added on the very first evaluation of the worker
script.". This patch adds warning messages for the events.

[1]:
https://w3c.github.io/ServiceWorker/spec/service_worker/index.html#run-servic...

BUG=594160

Committed: https://crrev.com/e14cea541b991c7907123395ea663f39eea7dcfa
Cr-Commit-Position: refs/heads/master@{#419967}
==========

[commit-bot: I haz the power, 2016-09-21 03:48:59]

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