Worker: Move close() from WorkerGlobalScope to derived interfaces

Worker: Move close() from WorkerGlobalScope to derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

Specs:
WorkerGlobalScope: https://html.spec.whatwg.org/multipage/workers.html#workerglobalscope
DedicatedWorkerGlobalScope: https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the-dedicatedworkerglobalscope-interface
SharedWorkerGlobalScope: https://html.spec.whatwg.org/multipage/workers.html#shared-workers-and-the-sharedworkerglobalscope-interface
ServiceWorkerGlobalScope: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#service-worker-global-scope

BUG=611640
Committed: https://crrev.com/cd0f180962882117b13fd369372bdb92f4b5ecc7
Cr-Commit-Position: refs/heads/master@{#394077}

[nhiroki, 2016-05-13 08:20:03]

Description was changed from

==========
Worker: Move close() from WorkerGlobalScope to the derived interfaces

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

to

==========
Worker: Move close() from WorkerGlobalScope to the derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

BUG=611640
==========

[nhiroki, 2016-05-13 08:22:50]

Patchset #1 (id:1) has been deleted

[nhiroki, 2016-05-13 08:25:44]

Description was changed from

==========
Worker: Move close() from WorkerGlobalScope to the derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

BUG=611640
==========

to

==========
Worker: Move close() from WorkerGlobalScope to the derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

Specs:
WorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#workerglobalscope
DedicatedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the...
SharedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#shared-workers-and-the-sh...
ServiceWorkerGlobalScope:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#se...


BUG=611640
==========

[nhiroki, 2016-05-13 08:26:36]

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

[nhiroki, 2016-05-13 08:26:37]

PTAL, thanks!

[nhiroki, 2016-05-13 08:28:01]

Description was changed from

==========
Worker: Move close() from WorkerGlobalScope to the derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

Specs:
WorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#workerglobalscope
DedicatedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the...
SharedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#shared-workers-and-the-sh...
ServiceWorkerGlobalScope:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#se...


BUG=611640
==========

to

==========
Worker: Move close() from WorkerGlobalScope to the derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

Specs:
WorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#workerglobalscope
DedicatedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the...
SharedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#shared-workers-and-the-sh...
ServiceWorkerGlobalScope:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#se...

BUG=611640
==========

[kinuko, 2016-05-15 14:09:38]

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

https://codereview.chromium.org/1970003004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerGlobalScope.h:127: virtual bool
isClosing() const { return false; }
It might be easier to follow the code to just keep close() and m_closing on this
class but disable in the subclasses that don't implement close, otherwise I
think we should have explicit comments here, e.g. "Should return true when the
WorkerGlobalScope is closing (e.g. via close() method).  If this returns true
the worker is going to be shutdown after the current task execution.  Workers
that don't support close operation should always return false"

[nhiroki, 2016-05-16 09:12:13]

Thank you for reviewing. Updated. PTAL.

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

https://codereview.chromium.org/1970003004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerGlobalScope.h:127: virtual bool
isClosing() const { return false; }
On 2016/05/15 14:09:37, kinuko wrote:
> It might be easier to follow the code to just keep close() and m_closing on
this
> class but disable in the subclasses that don't implement close, otherwise I
> think we should have explicit comments here, e.g. "Should return true when the
> WorkerGlobalScope is closing (e.g. via close() method).  If this returns true
> the worker is going to be shutdown after the current task execution.  Workers
> that don't support close operation should always return false"

SGTM. The latest patchset keeps close() and m_closing on this class, and adds a
header comment on close().

[kinuko, 2016-05-16 09:40:49]

lgtm

https://codereview.chromium.org/1970003004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/workers/WorkerGlobalScope.h (right):

https://codereview.chromium.org/1970003004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerGlobalScope.h:132: bool isClosing()
{ return m_closing; }
nit: could be const method

[nhiroki, 2016-05-17 01:16:43]

Description was changed from

==========
Worker: Move close() from WorkerGlobalScope to the derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

Specs:
WorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#workerglobalscope
DedicatedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the...
SharedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#shared-workers-and-the-sh...
ServiceWorkerGlobalScope:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#se...

BUG=611640
==========

to

==========
Worker: Move close() from WorkerGlobalScope to derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

Specs:
WorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#workerglobalscope
DedicatedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the...
SharedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#shared-workers-and-the-sh...
ServiceWorkerGlobalScope:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#se...

BUG=611640
==========

[nhiroki, 2016-05-17 01:19:16]

Thank you!

https://codereview.chromium.org/1970003004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/workers/WorkerGlobalScope.h (right):

https://codereview.chromium.org/1970003004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/workers/WorkerGlobalScope.h:132: bool isClosing()
{ return m_closing; }
On 2016/05/16 09:40:49, kinuko wrote:
> nit: could be const method

Done.

[nhiroki, 2016-05-17 01:19:57]

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

[nhiroki, 2016-05-17 01:19:58]

+tkent@, can you review Source/web/ServiceWorkerGlobalScopeProxy.cpp ? Thanks!

[tkent, 2016-05-17 01:54:23]

lgtm.
This CL has a web-exposed API change, but it has no compatibility risk as the
description says. Intent-to-remove is unnecessary.

[nhiroki, 2016-05-17 02:04:07]

The CQ bit was checked by nhiroki@chromium.org

[nhiroki, 2016-05-17 02:04:07]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org
Link to the patchset: https://codereview.chromium.org/1970003004/#ps80001
(title: "add const")

[commit-bot: I haz the power, 2016-05-17 02:04:38]

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

[commit-bot: I haz the power, 2016-05-17 07:30:37]

Description was changed from

==========
Worker: Move close() from WorkerGlobalScope to derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

Specs:
WorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#workerglobalscope
DedicatedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the...
SharedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#shared-workers-and-the-sh...
ServiceWorkerGlobalScope:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#se...

BUG=611640
==========

to

==========
Worker: Move close() from WorkerGlobalScope to derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

Specs:
WorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#workerglobalscope
DedicatedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the...
SharedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#shared-workers-and-the-sh...
ServiceWorkerGlobalScope:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#se...

BUG=611640
==========

[commit-bot: I haz the power, 2016-05-17 07:30:38]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-05-17 07:32:48]

Description was changed from

==========
Worker: Move close() from WorkerGlobalScope to derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

Specs:
WorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#workerglobalscope
DedicatedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the...
SharedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#shared-workers-and-the-sh...
ServiceWorkerGlobalScope:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#se...

BUG=611640
==========

to

==========
Worker: Move close() from WorkerGlobalScope to derived interfaces

close() was originally defined in WorkerGlobalScope, but it was moved to derived
interfaces by the recent spec change (https://github.com/whatwg/html/pull/1119).

This CL moves close() from WorkerGlobalScope to the derived interfaces, namely,
DedicatedWorkerGlobalScope and SharedWorkerGlobalScope, and also removes close()
from ServiceWorkerGlobalScope.

These changes should not break applications in the wild. For DedicatedWorker and
SharedWorker, close() is still valid after this change. For ServiceWorker,
close() is not accessible after this change, but it raised an exception and was
not usable in the first place.

Specs:
WorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#workerglobalscope
DedicatedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#dedicated-workers-and-the...
SharedWorkerGlobalScope:
https://html.spec.whatwg.org/multipage/workers.html#shared-workers-and-the-sh...
ServiceWorkerGlobalScope:
https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#se...

BUG=611640

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

[commit-bot: I haz the power, 2016-05-17 07:32:49]

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