Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

Before this CL, script loading and evaluation are directly notified to
WorkerGlobalScope. After this CL, they are notified via WorkerReportingProxy
that is a proper route.

BUG=519111, 640843
Committed: https://crrev.com/ac6098db746f5939263b51863df6f061f0ebdb76
Cr-Commit-Position: refs/heads/master@{#418230}

[nhiroki, 2016-09-13 06:21:44]

Description was changed from

==========
Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

BUG=640843
==========

to

==========
Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

Before this CL, script loading and evaluation are directly notified to
WorkerGlobalScope. After this CL, they are notified via WorkerReportingProxy
that is a proper route for the events.

BUG=640843
==========

[nhiroki, 2016-09-13 06:30:01]

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

[commit-bot: I haz the power, 2016-09-13 06:30:13]

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

[nhiroki, 2016-09-13 06:32:48]

Description was changed from

==========
Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

Before this CL, script loading and evaluation are directly notified to
WorkerGlobalScope. After this CL, they are notified via WorkerReportingProxy
that is a proper route for the events.

BUG=640843
==========

to

==========
Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

Before this CL, script loading and evaluation are directly notified to
WorkerGlobalScope. After this CL, they are notified via WorkerReportingProxy
that is a proper route.

BUG=640843
==========

[nhiroki, 2016-09-13 06:35:30]

Description was changed from

==========
Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

Before this CL, script loading and evaluation are directly notified to
WorkerGlobalScope. After this CL, they are notified via WorkerReportingProxy
that is a proper route.

BUG=640843
==========

to

==========
Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

Before this CL, script loading and evaluation are directly notified to
WorkerGlobalScope. After this CL, they are notified via WorkerReportingProxy
that is a proper route.

BUG=519111, 640843
==========

[nhiroki, 2016-09-13 06:35:48]

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

[nhiroki, 2016-09-13 06:35:49]

PTAL, thanks!

https://codereview.chromium.org/2337003002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/workers/WorkerThread.cpp (left):

https://codereview.chromium.org/2337003002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/workers/WorkerThread.cpp:530: if
(!scriptController->isExecutionForbidden()) {
isExecutionForbidden() always returns false here, so I removed this check.

https://codereview.chromium.org/2337003002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/workers/WorkerThread.cpp (right):

https://codereview.chromium.org/2337003002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/workers/WorkerThread.cpp:528:
m_workerReportingProxy.didLoadWorkerScript(sourceCode.length(),
cachedMetaData.get() ? cachedMetaData->size() : 0);
didLoadWorkerScript() should be called after workerGlobalScopeStarted() because
ServiceWorkerReportingProxy::didLoadWorkerScript() needs WorkerGlobalScope. I
think this change breaks nothing.

[falken, 2016-09-13 06:51:11]

https://codereview.chromium.org/2337003002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/workers/WorkerReportingProxy.h (right):

https://codereview.chromium.org/2337003002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:55: // Invoked
when the worker script or imported script is evaluated.
loaded.

https://codereview.chromium.org/2337003002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:65: // Invoked
when the new WorkerGlobalScope is started.
Would it be worth ordering these by the order they occur and commenting about
the order?

It goes GlobalScopeStarted -> didLoad (for main script) -> didInitialize ->
didEvaluate?

[falken, 2016-09-13 06:52:10]

on second thought that could be a followup patch. this lgtm

[nhiroki, 2016-09-13 07:06:46]

Thank you!

https://codereview.chromium.org/2337003002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/workers/WorkerReportingProxy.h (right):

https://codereview.chromium.org/2337003002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:55: // Invoked
when the worker script or imported script is evaluated.
On 2016/09/13 06:51:11, falken wrote:
> loaded.

Done.

https://codereview.chromium.org/2337003002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:65: // Invoked
when the new WorkerGlobalScope is started.
On 2016/09/13 06:51:11, falken wrote:
> Would it be worth ordering these by the order they occur and commenting about
> the order?

I'll work on it in a follow-up CL.

> It goes GlobalScopeStarted -> didLoad (for main script) -> didInitialize ->
> didEvaluate? 

Right.

[nhiroki, 2016-09-13 07:06:55]

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

[commit-bot: I haz the power, 2016-09-13 07:07:01]

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

[nhiroki, 2016-09-13 07:07:04]

nhiroki@chromium.org changed reviewers:
+ haraken@chromium.org

[nhiroki, 2016-09-13 07:07:05]

+haraken@, can you review Source/web/*? Thanks.

[haraken, 2016-09-13 07:49:26]

On 2016/09/13 07:07:05, nhiroki wrote:
> +haraken@, can you review Source/web/*? Thanks.

LGTM

[nhiroki, 2016-09-13 07:50:10]

The CQ bit was unchecked by nhiroki@chromium.org

[nhiroki, 2016-09-13 07:50:12]

The CQ bit was checked by nhiroki@chromium.org

[nhiroki, 2016-09-13 07:50:12]

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/2337003002/#ps20001
(title: "fix comment")

[commit-bot: I haz the power, 2016-09-13 07:50:20]

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-13 08:25:55]

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

[commit-bot: I haz the power, 2016-09-13 08:25:56]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[nhiroki, 2016-09-13 12:24:01]

The CQ bit was checked by nhiroki@chromium.org

[nhiroki, 2016-09-13 12:24:01]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org,
haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2337003002/#ps40001
(title: "fix tests")

[commit-bot: I haz the power, 2016-09-13 12:24:21]

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-13 13:48:33]

Description was changed from

==========
Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

Before this CL, script loading and evaluation are directly notified to
WorkerGlobalScope. After this CL, they are notified via WorkerReportingProxy
that is a proper route.

BUG=519111, 640843
==========

to

==========
Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

Before this CL, script loading and evaluation are directly notified to
WorkerGlobalScope. After this CL, they are notified via WorkerReportingProxy
that is a proper route.

BUG=519111, 640843
==========

[commit-bot: I haz the power, 2016-09-13 13:48:34]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-13 13:50:22]

Description was changed from

==========
Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

Before this CL, script loading and evaluation are directly notified to
WorkerGlobalScope. After this CL, they are notified via WorkerReportingProxy
that is a proper route.

BUG=519111, 640843
==========

to

==========
Worker: Notify WorkerThread lifecycle events via WorkerReportingProxy

Before this CL, script loading and evaluation are directly notified to
WorkerGlobalScope. After this CL, they are notified via WorkerReportingProxy
that is a proper route.

BUG=519111, 640843

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

[commit-bot: I haz the power, 2016-09-13 13:50:24]

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