Shows the error message while fetching the SW script in the DevTools console.

Shows the error message while fetching the SW script in the DevTools console.

This cl introduces EmbeddedWorkerMsg_AddMessageToConsole IPC message.
The browser process sends this message to the renderer to add message to the devtools console.
This behavior is similar to FrameMsg_AddMessageToConsole.

BUG=605817
Committed: https://crrev.com/7498e14b76230504cc034eb6ee268eea5d91906d
Cr-Commit-Position: refs/heads/master@{#389415}

[horo, 2016-04-22 08:06:46]

Description was changed from

==========
Shows the error message while fetching the SW script in the DevTools console.

BUG=605817
==========

to

==========
Shows the error message while fetching the SW script in the DevTools console.

This cl introduces EmbeddedWorkerMsg_AddMessageToConsole IPC message.
The browser process sends this message to the renderer to add message to the
devtools console.
This behavior is similar to FrameMsg_AddMessageToConsole.

BUG=605817
==========

[horo, 2016-04-22 08:07:42]

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

[horo, 2016-04-22 08:07:43]

falken@
Could you please review this?

[falken, 2016-04-22 08:34:18]

Looks good. What happens when you do register() and the script response is bad?
I suspect you'll get two error messages... one from the rejected Promise and one
from the addToConsole code. I guess that would be OK, let's see what it looks
like though.

https://codereview.chromium.org/1908313003/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_write_to_cache_job.cc
(right):

https://codereview.chromium.org/1908313003/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_write_to_cache_job.cc:451:
status_message);
I think this should move to NotifyFinishedCaching. There are some error paths
like when ReadRawData() fails, that don't end up in NotifyStartErrorHelper, but
all paths eventually lead to NotifyFinishedCaching.

[horo, 2016-04-22 09:22:31]

We will see three messages when register() failed.

1. "The script has an unsupported MIME type ('application/foobar')."
   This if added by ServiceWorkerWriteToCacheJob.
2. "GET http://127.0.0.1:8000/serviceworker/demo/simpleblob/sw.js
net::ERR_INSECURE_RESPONSE"
   This if added by FrameConsole::didFailLoading().
3. "Uncaught (in promise) DOMException: Failed to register a ServiceWorker: The
script has an unsupported MIME type ('application/foobar')."
   This if added by RejectedPromises.

https://codereview.chromium.org/1908313003/diff/1/content/browser/service_wor...
File content/browser/service_worker/service_worker_write_to_cache_job.cc
(right):

https://codereview.chromium.org/1908313003/diff/1/content/browser/service_wor...
content/browser/service_worker/service_worker_write_to_cache_job.cc:451:
status_message);
On 2016/04/22 08:34:18, falken wrote:
> I think this should move to NotifyFinishedCaching. There are some error paths
> like when ReadRawData() fails, that don't end up in NotifyStartErrorHelper,
but
> all paths eventually lead to NotifyFinishedCaching.

Done.

[horo, 2016-04-22 09:22:57]

We will see three messages when register() failed.

1. "The script has an unsupported MIME type ('application/foobar')."
   This is added by ServiceWorkerWriteToCacheJob.
2. "GET http://127.0.0.1:8000/serviceworker/demo/simpleblob/sw.js
net::ERR_INSECURE_RESPONSE"
   This is added by FrameConsole::didFailLoading().
3. "Uncaught (in promise) DOMException: Failed to register a ServiceWorker: The
script has an unsupported MIME type ('application/foobar')."
   This is added by RejectedPromises.

[falken, 2016-04-22 09:39:53]

lgtm

https://codereview.chromium.org/1908313003/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_write_to_cache_job.cc
(right):

https://codereview.chromium.org/1908313003/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_write_to_cache_job.cc:451: //
worker stops soon after recieving the error response.
This comment is outdated now that NotifyStartError may not be called in this
codepath. I guess we can just delete it, otherwise it should be more general,
something like: "AddMessageToConsole must be called before this job notifies
that an error occurred because the worker stops soon after receiving the error
response."

[horo, 2016-04-22 10:55:22]

Thank you.

https://codereview.chromium.org/1908313003/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_write_to_cache_job.cc
(right):

https://codereview.chromium.org/1908313003/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_write_to_cache_job.cc:451: //
worker stops soon after recieving the error response.
On 2016/04/22 09:39:52, falken wrote:
> This comment is outdated now that NotifyStartError may not be called in this
> codepath. I guess we can just delete it, otherwise it should be more general,
> something like: "AddMessageToConsole must be called before this job notifies
> that an error occurred because the worker stops soon after receiving the error
> response."

Done.

[horo, 2016-04-22 10:56:40]

horo@chromium.org changed reviewers:
+ pfeldman@chromium.org

[horo, 2016-04-22 10:56:41]

pfeldman@
Could you please review third_party/WebKit/*?

[pfeldman, 2016-04-22 14:32:07]

It is unclear in which console you intend to show the message. Do you want to
route it to the page console for the case when devtools are opened post-factum?
Or are you only routing it into the worker console to surface it while devtools
are opened?

[pfeldman, 2016-04-22 14:33:46]

lgtm, but i wonder if something should be done for the post-mortem case.

[horo, 2016-04-25 02:25:24]

On 2016/04/22 14:33:46, pfeldman wrote:
> lgtm, but i wonder if something should be done for the post-mortem case.

Thank you.

We can see this message only when DevTools was opened before the Service Worker
soft update which is executed 1 second after the page navigation.

It may be better to route the message to the page's ConsoleMessageStorage.
But it will make the code more complex.

[horo, 2016-04-25 02:26:13]

horo@chromium.org changed reviewers:
+ wfh@chromium.org

[horo, 2016-04-25 02:26:14]

wfh@
Could you please review
content/common/service_worker/embedded_worker_messages.h?

[Will Harris, 2016-04-25 04:08:57]

On 2016/04/25 02:26:14, horo wrote:
> wfh@
> Could you please review
> content/common/service_worker/embedded_worker_messages.h?

lgtm for embedded_worker_messages.h

[horo, 2016-04-25 04:27:42]

The CQ bit was checked by horo@chromium.org

[horo, 2016-04-25 04:27:42]

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/1908313003/#ps40001
(title: "incorporated falken's comment")

[horo, 2016-04-25 04:27:43]

The CQ bit was unchecked by horo@chromium.org

[horo, 2016-04-25 04:27:47]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2016-04-25 04:27:56]

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

[commit-bot: I haz the power, 2016-04-25 05:52:52]

Description was changed from

==========
Shows the error message while fetching the SW script in the DevTools console.

This cl introduces EmbeddedWorkerMsg_AddMessageToConsole IPC message.
The browser process sends this message to the renderer to add message to the
devtools console.
This behavior is similar to FrameMsg_AddMessageToConsole.

BUG=605817
==========

to

==========
Shows the error message while fetching the SW script in the DevTools console.

This cl introduces EmbeddedWorkerMsg_AddMessageToConsole IPC message.
The browser process sends this message to the renderer to add message to the
devtools console.
This behavior is similar to FrameMsg_AddMessageToConsole.

BUG=605817
==========

[commit-bot: I haz the power, 2016-04-25 05:52:53]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-04-25 05:53:54]

Description was changed from

==========
Shows the error message while fetching the SW script in the DevTools console.

This cl introduces EmbeddedWorkerMsg_AddMessageToConsole IPC message.
The browser process sends this message to the renderer to add message to the
devtools console.
This behavior is similar to FrameMsg_AddMessageToConsole.

BUG=605817
==========

to

==========
Shows the error message while fetching the SW script in the DevTools console.

This cl introduces EmbeddedWorkerMsg_AddMessageToConsole IPC message.
The browser process sends this message to the renderer to add message to the
devtools console.
This behavior is similar to FrameMsg_AddMessageToConsole.

BUG=605817

Committed: https://crrev.com/7498e14b76230504cc034eb6ee268eea5d91906d
Cr-Commit-Position: refs/heads/master@{#389415}
==========

[commit-bot: I haz the power, 2016-04-25 05:53:55]

Patchset 3 (id:??) landed as
https://crrev.com/7498e14b76230504cc034eb6ee268eea5d91906d
Cr-Commit-Position: refs/heads/master@{#389415}