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 2 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
70 changes: 47 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 @@ -96419,6 +96399,37 @@ interface <dfn>WorkerGlobalScope</dfn> : <span>EventTarget</span> {
immediately invoked <span data-x="dom-MessagePort-postMessage">the method of the same name</span>
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.

on the port, with the same arguments, and returned the same return value.</p>

<dl class="domintro">
<dt><var>dedicatedWorkerGlobal</var> . <code subdfn data-x="dom-DedicatedWorkerGlobalScope-
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.

close">close</code>()</dt>
<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.

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.

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


<div w-nodev>

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


<p>To <dfn>close a worker</dfn>, given a <var>worker global scope</var>, run these steps
(atomically):</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>worker
global scope</var>'s <span>event loop</span>'s <span data-x="task queue">task queues</span>.</p>

<li><p>Set <var>worker global scope</var>'s <span data-x="dom-WorkerGlobalScope-
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.

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.


</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 +96453,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 +96473,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>
<dd>Aborts <var>sharedWorkerGlobal</var>.</dd>
</dl>

<div w-nodev>
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.

Expand All @@ -96468,8 +96486,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>When a script invokes the <dfn><code data-x="dom-SharedWorkerGlobalScope-
close">close()</code></dfn> method on a <code>SharedWorkerGlobalScope</code> object, the user
agent must <span>close a worker</span> with the <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