Worker: Canonicalize names of WorkerThread lifecycle events in WorkerReportingProxy

Worker: Canonicalize names of WorkerThread lifecycle events in WorkerReportingProxy

This CL sorts handlers of the lifecycle events in call order and canonicalizes
their names.

BUG=640843
Committed: https://crrev.com/71c9c5891cfe1cbc631823322894685cb7dc8975
Cr-Commit-Position: refs/heads/master@{#418530}

[nhiroki, 2016-09-14 03:50:30]

Patchset #1 (id:1) has been deleted

[nhiroki, 2016-09-14 03:54:26]

Description was changed from

==========
Worker: Clean up WorkerReportingProxy

BUG=
==========

to

==========
Worker: Clean up WorkerReportingProxy

This CL sorts lifecycle event handlers in call order and renames them with
canonicalizes their names for cleanup.

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

[nhiroki, 2016-09-14 03:54:34]

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

[commit-bot: I haz the power, 2016-09-14 03:54:53]

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-14 03:57:28]

Description was changed from

==========
Worker: Clean up WorkerReportingProxy

This CL sorts lifecycle event handlers in call order and renames them with
canonicalizes their names for cleanup.

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

to

==========
Worker: Canonicalize names of WorkerThread lifecycle events in
WorkerReportingProxy

This CL sorts handlers of the lifecycle events in call order and canonicalizes
their names.

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

[nhiroki, 2016-09-14 03:57:55]

Description was changed from

==========
Worker: Canonicalize names of WorkerThread lifecycle events in
WorkerReportingProxy

This CL sorts handlers of the lifecycle events in call order and canonicalizes
their names.

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

to

==========
Worker: Canonicalize names of WorkerThread lifecycle events in
WorkerReportingProxy

This CL sorts handlers of the lifecycle events in call order and canonicalizes
their names.

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

[nhiroki, 2016-09-14 03:57:55]

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

[nhiroki, 2016-09-14 03:58:24]

falken@, can you review this? Thanks!

[commit-bot: I haz the power, 2016-09-14 05:02:47]

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

[commit-bot: I haz the power, 2016-09-14 05:02:48]

Dry run: This issue passed the CQ dry run.

[falken, 2016-09-14 06:07:13]

nice cleanup!

https://codereview.chromium.org/2337253005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/workers/WorkerReportingProxy.h (right):

https://codereview.chromium.org/2337253005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:55: // Worker
lifecycle events. These are called in this order.
nit: comments like this that apply to code many lines below are likely to get
out of date, also it's not necessarily true since didLoadWorkerScript is called
after didEvaluateWorkerScript for importScripts.

Maybe it's better to keep them in order but not mention it directly. Then for
each function what came before it when it might not be clear, e.g.:

// Invoked when the new WorkerGlobalScope is created. This is called after
didLoadWorkerScript.
virtual void didCreateWorkerGlobalScope(WorkerOrWorkletGlobalScope*) { }

// Invoked when the the WorkerGlobalScope is initialized. This is called after
didCreateWorkerGlobalScope.
virtual void didInitializeWorkerContext() { }

https://codereview.chromium.org/2337253005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:80: // interface)
"This is be" -> "This is"

[nhiroki, 2016-09-14 07:13:56]

Thanks for your comments! Updated.

https://codereview.chromium.org/2337253005/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/workers/WorkerReportingProxy.h (right):

https://codereview.chromium.org/2337253005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:55: // Worker
lifecycle events. These are called in this order.
On 2016/09/14 06:07:13, falken wrote:
> nit: comments like this that apply to code many lines below are likely to get
> out of date, also it's not necessarily true since didLoadWorkerScript is
called
> after didEvaluateWorkerScript for importScripts.

Good point :)

> Maybe it's better to keep them in order but not mention it directly. Then for
> each function what came before it when it might not be clear, e.g.:
> 
> // Invoked when the new WorkerGlobalScope is created. This is called after
> didLoadWorkerScript.
> virtual void didCreateWorkerGlobalScope(WorkerOrWorkletGlobalScope*) { }
> 
> // Invoked when the the WorkerGlobalScope is initialized. This is called after
> didCreateWorkerGlobalScope.
> virtual void didInitializeWorkerContext() { }

Added comments.

https://codereview.chromium.org/2337253005/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:80: // interface)
On 2016/09/14 06:07:13, falken wrote:
> "This is be" -> "This is"

Done.

[nhiroki, 2016-09-14 07:14:00]

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

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

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

[falken, 2016-09-14 07:19:39]

lgtm

[nhiroki, 2016-09-14 07:26:53]

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

[nhiroki, 2016-09-14 07:26:53]

+keishi, can you review core/loader/ and web/ as an OWNER? This is a mechanical
change to rename/reorder functions for cleanup. Thanks!

[haraken, 2016-09-14 07:31:14]

core/ and web/ LGTM

[keishi, 2016-09-14 07:34:19]

LGTM

[nhiroki, 2016-09-14 07:34:54]

Thank you :)

[nhiroki, 2016-09-14 07:35:01]

The CQ bit was unchecked by nhiroki@chromium.org

[nhiroki, 2016-09-14 07:35:02]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2016-09-14 07:35:17]

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-14 10:32:34]

Description was changed from

==========
Worker: Canonicalize names of WorkerThread lifecycle events in
WorkerReportingProxy

This CL sorts handlers of the lifecycle events in call order and canonicalizes
their names.

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

to

==========
Worker: Canonicalize names of WorkerThread lifecycle events in
WorkerReportingProxy

This CL sorts handlers of the lifecycle events in call order and canonicalizes
their names.

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

[commit-bot: I haz the power, 2016-09-14 10:32:35]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-09-14 10:34:00]

Description was changed from

==========
Worker: Canonicalize names of WorkerThread lifecycle events in
WorkerReportingProxy

This CL sorts handlers of the lifecycle events in call order and canonicalizes
their names.

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

to

==========
Worker: Canonicalize names of WorkerThread lifecycle events in
WorkerReportingProxy

This CL sorts handlers of the lifecycle events in call order and canonicalizes
their names.

BUG=640843

Committed: https://crrev.com/71c9c5891cfe1cbc631823322894685cb7dc8975
Cr-Commit-Position: refs/heads/master@{#418530}
==========

[commit-bot: I haz the power, 2016-09-14 10:34:01]

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