Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move worker's close() to dedicated worker and shared worker #1119

Merged
merged 6 commits into from
Apr 27, 2016
Merged

Move worker's close() to dedicated worker and shared worker #1119

Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
535e285
Move worker's close() to dedicated worker and shared worker
jungkees Apr 25, 2016
fd020a2
Extract close a worker algorithm
jungkees Apr 27, 2016
e798194
Comply with the style guide
jungkees Apr 27, 2016
c80d871
Reorder blocks and rename a variable
jungkees Apr 27, 2016
048da21
Remove atomically word; rename a variable
jungkees Apr 27, 2016
933c8b2
Fix markup alignment
jungkees Apr 27, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 51 additions & 23 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96297,8 +96297,6 @@ interface <dfn>WorkerGlobalScope</dfn> : <span>EventTarget</span> {
readonly attribute <span>WorkerNavigator</span> <span data-x="dom-worker-navigator">navigator</span>;
void <span data-x="dom-WorkerGlobalScope-importScripts">importScripts</span>(DOMString... urls);

void <span data-x="dom-WorkerGlobalScope-close">close</span>();

attribute <span>OnErrorEventHandler</span> <span data-x="handler-WorkerGlobalScope-onerror">onerror</span>;
attribute <span>EventHandler</span> <span data-x="handler-WorkerGlobalScope-onlanguagechange">onlanguagechange</span>;
attribute <span>EventHandler</span> <span data-x="handler-WorkerGlobalScope-onoffline">onoffline</span>;
Expand Down Expand Up @@ -96340,9 +96338,6 @@ interface <dfn>WorkerGlobalScope</dfn> : <span>EventTarget</span> {
<dt><var>workerGlobal</var> . <code subdfn data-x="dom-WorkerGlobalScope-importScripts">importScripts</code>(<var>urls</var>...)</dt>
<dd>Fetches each <span>URL</span> in <var>urls</var>, executes them one-by-one in the order they
are passed, and then returns (or throws if something went amiss).</dd>

<dt><var>workerGlobal</var> . <code subdfn data-x="dom-WorkerGlobalScope-close">close</code>()</dt>
<dd>Aborts <var>workerGlobal</var>.</dd>
</dl>

<div w-nodev>
Expand All @@ -96359,24 +96354,6 @@ interface <dfn>WorkerGlobalScope</dfn> : <span>EventTarget</span> {
<code>WorkerGlobalScope</code> object, this is not problematic as it cannot be observed from
script.</p>

<hr>

<p>When a script invokes the <dfn><code data-x="dom-WorkerGlobalScope-close">close()</code></dfn>
method on a <code>WorkerGlobalScope</code> object, the user agent must run the following steps
(atomically):</p>

<ol>

<li><p>Discard any <span data-x="concept-task">tasks</span> that have been added to the
<code>WorkerGlobalScope</code> object's <span>event loop</span>'s <span data-x="task queue">task
queues</span>.</p>

<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>

</ol>

</div>

<hr>
Expand All @@ -96403,6 +96380,9 @@ interface <dfn>WorkerGlobalScope</dfn> : <span>EventTarget</span> {
<pre class="idl">[Global=(Worker,DedicatedWorker),Exposed=DedicatedWorker]
/*sealed*/ interface <dfn>DedicatedWorkerGlobalScope</dfn> : <span>WorkerGlobalScope</span> {
void <span data-x="dom-DedicatedWorkerGlobalScope-postMessage">postMessage</span>(any message, optional sequence&lt;<span data-x="idl-object">object</span>&gt; transfer = []);

void <span data-x="dom-DedicatedWorkerGlobalScope-close">close</span>();

attribute <span>EventHandler</span> <span data-x="handler-DedicatedWorkerGlobalScope-onmessage">onmessage</span>;
};</pre>

Expand All @@ -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>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot have a newline here. You need to break before data-x.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Done.

<dd>Clones <var>message</var> and transmits it to the <code>Worker</code> object associated with
<var>dedicatedWorkerGlobal</var>. <var>transfer</var> can be passed as a list of objects that are
to be transfered rather than cloned.</dd>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

<dt><var>dedicatedWorkerGlobal</var> . <code subdfn data-x="dom-DedicatedWorkerGlobalScope-
close">close</code>()</dt>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot have a newline here. You need to break before data-x.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

<dd>Aborts <var>dedicatedWorkerGlobal</var>.</dd>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should appear before the postMessage() method.

The postMessage() method should probably get its own entry at some point in there too.

</dl>

<p>The <dfn><code data-x="dom-DedicatedWorkerGlobalScope-postMessage">postMessage()</code></dfn>
method on <code>DedicatedWorkerGlobalScope</code> objects must act as if, when invoked, it
immediately invoked <span data-x="dom-MessagePort-postMessage">the method of the same name</span>
on the port, with the same arguments, and returned the same return value.</p>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

<div w-nodev>

<p>To <dfn>close a worker</dfn>, given a <var>workerGlobal</var>, run these steps:</p>

<ol>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to say atomically here I think. We should call it out when something is a non-atomic operation and I think we mostly do, through "in parallel" and such.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess "atomically" was used to consider the two steps as an atomic operation so it would say other threads running in parallel won't queue a task between the step 1 and step 2. But I'm not sure what the original intent was. Just removed it as commented. I think changing the order of the steps (i.e. set the flag first and discard the tasks in the queue) seem to work as well without "atomically". If you feel that'd be better here, let me know.


<li><p>Discard any <span data-x="concept-task">tasks</span> that have been added to
<var>workerGlobal</var>'s <span>event loop</span>'s <span data-x="task queue">task
queues</span>.</p>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable needs to be renamed here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks.

<li><p>Set <var>workerGlobal</var>'s <span data-x="dom-WorkerGlobalScope-closing">closing</span>
flag to true. (This prevents any further tasks from being queued.)</p></li>

</ol>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We should move this prior to the close() method. Typically the algorithm comes first, then the method that uses it.
  2. Please rename the argument to workerGlobal. I think we should slowly move away from spaces in arguments since they read rather confusingly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also remove the newline directly after <ol> and before </ol>. I realize they were there before, but we're trying to cut down on those.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Done.


<p>The <dfn><code data-x="dom-DedicatedWorkerGlobalScope-close">close()</code></dfn> method, when
invoked, must <span>close a worker</span> with this <code>DedicatedWorkerGlobalScope</code>
object.</p>

</div>

<hr>

<p>The following are the <span>event handlers</span> (and their corresponding <span data-x="event
handler event type">event handler event types</span>) that must be supported, as <span>event
handler IDL attributes</span>, by objects implementing the <code>DedicatedWorkerGlobalScope</code>
Expand All @@ -96442,6 +96457,9 @@ interface <dfn>WorkerGlobalScope</dfn> : <span>EventTarget</span> {
/*sealed*/ interface <dfn>SharedWorkerGlobalScope</dfn> : <span>WorkerGlobalScope</span> {
readonly attribute DOMString <span data-x="dom-SharedWorkerGlobalScope-name">name</span>;
readonly attribute <span>ApplicationCache</span> <span data-x="dom-SharedWorkerGlobalScope-applicationCache">applicationCache</span>; // deprecated

void <span data-x="dom-SharedWorkerGlobalScope-close">close</span>();

attribute <span>EventHandler</span> <span data-x="handler-SharedWorkerGlobalScope-onconnect">onconnect</span>;
};</pre>

Expand All @@ -96459,6 +96477,10 @@ interface <dfn>WorkerGlobalScope</dfn> : <span>EventTarget</span> {
<dt><var>sharedWorkerGlobal</var> . <code subdfn data-x="dom-SharedWorkerGlobalScope-name">name</code></dt>
<dd>Returns <var>sharedWorkerGlobal</var>'s <span
data-x="concept-SharedWorkerGlobalScope-name">name</span>.</dd>

<dt><var>sharedWorkerGlobal</var> . <code subdfn data-x="dom-SharedWorkerGlobalScope-
close">close</code>()</dt>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You cannot have a newline here. You need to break before data-x.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

<dd>Aborts <var>sharedWorkerGlobal</var>.</dd>
</dl>

<div w-nodev>
Expand All @@ -96468,8 +96490,14 @@ interface <dfn>WorkerGlobalScope</dfn> : <span>EventTarget</span> {
data-x="concept-SharedWorkerGlobalScope-name">name</span>. Its value represents the name that can
be used to obtain a reference to the worker using the <code>SharedWorker</code> constructor.</p>

<p>The <dfn><code data-x="dom-SharedWorkerGlobalScope-close">close()</code></dfn> method, when
invoked, must <span>close a worker</span> with this <code>SharedWorkerGlobalScope</code>
object.</p>

</div>

<hr>

<p>The following are the <span>event handlers</span> (and their corresponding <span data-x="event
handler event type">event handler event types</span>) that must be supported, as <span>event
handler IDL attributes</span>, by objects implementing the <code>SharedWorkerGlobalScope</code>
Expand Down