Move worker's close() to dedicated worker and shared worker #1119
Conversation
As WorkerGlobalScope's close() method should not be exposed to one of its derived interfaces, ServiceWorkerGlobalScope, close() needs to be moved to the derived interfaces which explicitly requires it. This commit moves close() to DedicatedWorkerGlobalScope and SharedWorkerGlobalScope. Related issue: w3c/ServiceWorker#865
|
|
||
| <li><p>Set the worker's <code>WorkerGlobalScope</code> object's <span | ||
| data-x="dom-WorkerGlobalScope-closing">closing</span> flag to true. (This prevents any further | ||
| tasks from being queued.)</p></li> |
There was a problem hiding this comment.
I think instead this should become a generic algorithm that takes a global object that dedicated.close() and shared.close() can use. I don't like having two algorithms saying the same thing.
There was a problem hiding this comment.
Right. I added a commit that addressed this by extracting "close a worker". Please take a look.
Extract close a worker algorithm that can be invoked by the close() method of a worker. At the time of this patch, a dedicated worker and a shared worker invoke it.
| <p>When a script invokes the <dfn><code data-x="dom-DedicatedWorkerGlobalScope- | ||
| close">close()</code></dfn> method on a <code>DedicatedWorkerGlobalScope</code> object, the user | ||
| agent must <span>close a worker</span> with the <code>DedicatedWorkerGlobalScope</code> | ||
| object.</p> |
There was a problem hiding this comment.
Roughly, this should read: "The close() method, when invoked, must [close a worker] with this DedicatedWorkerGlobalScope object." That matches the style we have been using. Analogous for the close() method of SharedWorkerGlobalScope objects.
|
Just wait.. didn't see the other comments than the first one. I'll ask a review after addressing them all. |
|
@annevk, I think it's ready for review. Please. |
| <dd>Clones <var>message</var> and transmits it to the <code>Worker</code> object associated with | ||
| the worker that is represented by this <var>dedicatedWorkerGlobal</var> object. | ||
| <var>transfer</var> can be passed as a list of objects that are to be transfered rather than | ||
| cloned.</dd> |
There was a problem hiding this comment.
Oh hey, thank you for adding this!
Instead of associated with the worker that is represented by this <var>dedicatedWorkerGlobal</var> object just say associated with <var>dedicatedWorkerGlobal</var>.
| @@ -96414,11 +96394,46 @@ interface <dfn>WorkerGlobalScope</dfn> : <span>EventTarget</span> { | |||
| <p>All messages received by that port must immediately be retargeted at the | |||
| <code>DedicatedWorkerGlobalScope</code> object.</p> | |||
|
|
|||
| <dl class="domintro"> | |||
| <dt><var>dedicatedWorkerGlobal</var> . <code subdfn data-x="dom-DedicatedWorkerGlobalScope- | |||
| postMessage">postMessage</code>(<var>message</var> [, <var>transfer</var> ])</dt> | |||
There was a problem hiding this comment.
You cannot have a newline here. You need to break before data-x.
|
Looks really great otherwise, it just failed on the build system. Hopefully at some point we have active integration with the build system so I don't have to tell you about it so late :-( |
|
\o/ |
1 similar comment
|
\o/ |
This tests fail on Chromium because blink already removed the API. This change is to follow the spec changes discussed here: whatwg/html#1119 ,and here is the blink work: http://crbug.com/611640
As WorkerGlobalScope's close() method should not be exposed to one of
its derived interfaces, ServiceWorkerGlobalScope, close() needs to be
moved to the derived interfaces which explicitly require it. This
commit moves close() to DedicatedWorkerGlobalScope and
SharedWorkerGlobalScope.
Related issue: w3c/ServiceWorker#865