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

Add support for 'serviceworkers' member #507

Merged
merged 9 commits into from
Jan 17, 2017
Merged

Add support for 'serviceworkers' member #507

Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9e5567a
Add support for 'serviceworker' member
kenchris Oct 20, 2016
4a5f762
Review fixes after Marcos' comments
kenchris Dec 12, 2016
e97ab3e
Add Service Worker registration, with some help from Jungkee
kenchris Dec 13, 2016
2524071
Add support for 'type' and 'use_cache' when registering a service worker
kenchris Dec 13, 2016
3df8f7a
A few changes based on feedback by Jungkee
kenchris Dec 14, 2016
5d37374
Clean up spec text a bit
kenchris Dec 15, 2016
5c867ad
Add client to Start Register
kenchris Dec 15, 2016
4fc48b1
Fix two return values pointed out by Jungkee
kenchris Dec 15, 2016
77bdd10
Fix comments from Marcos
kenchris Dec 28, 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
298 changes: 295 additions & 3 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,14 +338,27 @@ <h3>
<ol>
<li>Instantiate an <a>installation process</a>.
</li>
<li>Let <a>manifest</a> be the result of <a>obtaining the
<li>Let <a>manifest</a> and <a>manifest URL</a> be the result of <a>obtaining the
manifest</a>.
</li>
<li>If <a>obtaining the manifest</a> results in an error, a user
agent can, at this point, fall back to using the <a>top-level
browsing context</a>' <code>Document</code>'s metadata to
populate an <a>installation process</a>' UI.
</li>
<li>If <a>obtaining the manifest</a> succeeds, and the
<var>service worker registration</var> of <a>manifest</a> is not
<code>undefined</code>, a user agent can at this point
<ul>
<li>Invoke <a>Start Register</a> with <var>scope</var> and <var>src</var>
Copy link
Member

Choose a reason for hiding this comment

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

q: should this be "Let registration be the result of Start Registration".

The way this is spec'ed is a bit strange - I was expecting some kind of flow with the resulting promise... but it all seems to magically result in a registration.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am not sure how to best solve this. Maybe you could solve in a follow up?

members of the <var>service worker registration</var>, a new <var>promise</var>,
null, <a>manifest URL</a>, plus the <var>type</var> and <var>use_cache</var>
members of the <var>service worker registration</var>,
</li>
</ul>
in which case the state of the settled <var>promise</var> determine whether the
Copy link
Member

Choose a reason for hiding this comment

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

nit: determines

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

<a>installation succeeded</a> or not.
</li>
<li>If the <a>installation succeeded</a>, <a>queue a task</a> on
the <a>application life-cycle task source</a> to <a>fire an
event</a> named <code>appinstalled</code> at the
Expand Down Expand Up @@ -604,8 +617,8 @@ <h2>
</p>
<p>
A developer specifies the navigation scope via the
<a><code>scope</code> member</a>. In the case where the
<a><code>scope</code> member</a> is missing or in error, the navigation
<a data-lt="member-scope"><code>scope</code> member</a>. In the case where the
<a data-lt="member-scope"><code>scope</code> member</a> is missing or in error, the navigation
scope is treated as <dfn>unbounded</dfn> (represented as the value
<code>undefined</code>). In such a case, the manifest is applied to all
URLs the application context is <a>navigated</a> to (see related
Expand Down Expand Up @@ -1239,6 +1252,11 @@ <h3>
<code>start_url</code> member</a> with <var>manifest</var>,
<var>manifest URL,</var> and <var>document URL</var> as arguments.
</li>
<li>Let <var>service worker registration</var> of <var>parsed manifest</var>
be the result of running the <a>steps for processing the
<code>serviceworker</code> member</a> with <var>manifest</var>,
<var>manifest URL</var>, and <var>serviceworker</var> as arguments.
</li>
<li>Let <var>display mode</var> of <var>parsed manifest</var> be the
result of running the <a>steps for processing the
<code>display</code> member</a> with <var>manifest</var> as the
Expand Down Expand Up @@ -2033,6 +2051,99 @@ <h3>
</p>
</section>
</section>

<section>
<h3>
<code>serviceworker</code> member
</h3>
<p>
The <dfn><code>serviceworker</code> member</dfn> describes a
service worker as defined in [[!SERVICE-WORKERS-1]].
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Is there some unversioned reference we can use here instead of V1?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not as far as I could see. @jungkees ?

</p>
<p>
The <a><code>serviceworker</code> member</a> represents a <a
href="https://w3c.github.io/ServiceWorker/#service-worker-registration-concept">
service worker registration</a>. Other service worker registrations can be done, for instance
Copy link
Member

Choose a reason for hiding this comment

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

The text "Other service worker registrations can be done" should be in a non-normative note.

Copy link
Member

Choose a reason for hiding this comment

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

Also, you are redefining the rules - which is a bit dangerous. That should point to the SW spec, like:

"For rules on how SWs interact, then see section X of Service Workers"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Made it a note now. @jungkees can you point me to the best place to refer to?

by a script; if these have different scopes they will be considered separate
Copy link
Member

Choose a reason for hiding this comment

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

We should probably not re-specify the rules that are in the service worker spec; we should reference them.

registrations. If they have the same scope and script URL, they coalesce.
If they have different script URLs, last one wins.
</p>
<p>
The <dfn>steps for processing the <code>serviceworker</code>
member</dfn> are given by the following algorithm. The algorithm
takes a <a>manifest</a> <var>manifest</var>. This algorithm returns a
registration object <var>registration</var>, which can be <code>undefined</code>.
Copy link
Member

Choose a reason for hiding this comment

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

nit: link to registration object?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

</p>
<ol>
<li>Let <var>unprocessed registration</var> be the result of calling the
<a>[[\GetOwnProperty]]</a> internal method of <var>manifest</var>
with argument "<code>serviceworker</code>".
<li>Let <var>src</var> be the result of running the <a>steps
for processing the <code>src</code> member of a service worker</a>
with <var>unprocessed registration</var> and <var>manifest URL</var>.
</li>
<li>If <var>src</var> is <code>undefined</code>, or if the result of
running <a>Is origin potentially trustworthy</a>
Copy link
Member

Choose a reason for hiding this comment

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

nit: capital "Is"

Copy link
Member

Choose a reason for hiding this comment

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

Probably warn developer here too

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

with the origin of <var>src</var> is <code>Not Trusted</code>, abort
these steps and return <code>undefined</code>.
</li>
<li>Otherwise, let <var>registration</var> be an object with
properties <code>src</code>, <code>scope</code>, <code>type</code> and
<code>use_cache</code>. All properties initially set to
<code>undefined</code>.
</li>
<li>Set <var>registration</var>'s <code>src</code> property to be
<var>src</var>.
</li>
<li>Let <var>scope</var> be the result of running the <a>steps
Copy link

Choose a reason for hiding this comment

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

Similar to type and use_cache, SW expects a default value for scope too. If scope is undefined, please give null to Start Register as a default value. That makes Start Register set the scope to the same location as the given script url.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When the value is undefined here, it means that there was a parsing error (check sub algorithms), and I thus don't attempt to register at all.

Copy link

Choose a reason for hiding this comment

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

I understood the intention. Please check if it actually doesn't attempt to register in that case (i.e. when the sub algorithm returns undefined.) I left a comment here: #507 (comment).

for processing the <code>scope</code> member of a service worker</a>
Copy link

Choose a reason for hiding this comment

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

When this algorithm returns undefined, the steps to install the web application gets to pass nothing to Start Register. In this case, shouldn't there be an error handling such that Start Register is not invoked?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let me check that, it should not be invoked no

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

          <li>If <a>obtaining the manifest</a> succeeds, and the
          <var>service worker registration</var> of <a>manifest</a> is not
          <code>undefined</code>, a user agent can at this point...

It is not invoked if it fails

Copy link

Choose a reason for hiding this comment

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

That looks good.

passing <var>unprocessed registration</var>.
</li>
<li>If <var>scope</var> is not <code>undefined</code>,
set <var>registration</var>'s <code>scope</code>
property to be <var>scope</var>.
</li>
<li>Let <var>type</var> be the result of running the <a>steps
for processing the <code>type</code> member of a service worker</a>
passing <var>unprocessed registration</var>.
</li>
<li>If <var>type</var> is not <code>undefined</code>
set <var>registration</var>'s <code>type</code>
property to be <var>type</var>.
</li>
<li>If <var>type</var> is <code>undefined</code> abort these steps and
return <code>undefined</code>.
Copy link

Choose a reason for hiding this comment

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

Service workers expect a default value for type member. If type is undefined here, please set the default value to "classic" when passing it to Start Register.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is set in the sub algorithm. Undefined here means that the sub algorithm didn't get a valid value

Copy link

Choose a reason for hiding this comment

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

Understood. I saw you return "classic" when no value is given. It looks good.

Copy link
Member

Choose a reason for hiding this comment

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

Warn developer? ... so touchy, these service workers!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

:)

</li>
<li>Let <var>use cache</var> be the result of running the <a>steps
for processing the <code>use_cache</code> member of a service worker</a>
passing <var>unprocessed registration</var>.
</li>
<li>If <var>use cache</var> is not <code>undefined</code>,
set <var>registration</var>'s <code>use_cache</code>
property to be <var>use cache</var>.
</li>
<li>If <var>use cache</var> is <code>undefined</code> abort these steps
and return <code>undefined</code>.
Copy link

Choose a reason for hiding this comment

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

Likewise, If use_cache is undefined here, please set the default value to false when passing it to Start Register.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same, ok Please keep me up to date :)

Copy link

Choose a reason for hiding this comment

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

Same as above. I saw you return false when no value is given. It looks good.

</li>
<li>Return <var>registration</var>.
</li>
</ol>
<div class="example">
In the following example, the web application is listing
a service worker for the <code>/foo</code> scope, bypassing the user agent cache
when fetching the <code>"sw.js"</code> source:
<pre class="example">
Copy link
Member

Choose a reason for hiding this comment

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

nit: you can indent this normally, ReSpec handles that for you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

"serviceworker": {
"src": "sw.js",
"scope": "/foo",
"use_cache": false
}
Copy link
Contributor

@RobDolinMS RobDolinMS Oct 24, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@kenchris - Should line 1971 be }] to match line 1967?

</pre>
</div>
</section>



<section>
<h3>
<code title="">theme_color</code> member
Expand Down Expand Up @@ -2814,6 +2925,179 @@ <h3>
</ol>
</section>
</section>
<section>
<h2>
The serviceworker object and its members
</h2>
<p>
A <dfn>serviceworker object</dfn> represents a service worker
registration for the web application.
</p>
<section>
<h3>
<code>src</code> member
</h3>
<p>
The <dfn data-lt="serviceworker-src"><code>src</code> member</dfn> of a
<a>serviceworker object</a> is a <a>URL</a> representing a
service worker.
Copy link
Member

Choose a reason for hiding this comment

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

You might want to check if this is the same in the SW spec. Just the wording seems odd as SWs are just script resources.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jungkees ^?

</p>
<p>
The <dfn>steps for processing the <code>src</code> member of a
service worker</dfn> are given by the following algorithm. The algorithm takes
a <a>serviceworker object</a> <var>registration</var>, and a <a>URL</a> <var>manifest
URL</var>, which is the <a>URL</a> from which the
<var>manifest</var> was fetched. This algorithm will return a
<a>URL</a> or <code>undefined</code>.
</p>
<ol>
<li>Let <var>value</var> be the result of calling the
<a>[[\GetOwnProperty]]</a> internal method of <var>registration</var>
passing " <code>src</code>" as the argument.
</li>
<li>Let <var>type</var> be <a>Type</a>(<var>value</var>).
</li>
<li>If <var>type</var> is not "string", then:
<ol>
<li>If <var>type</var> is not "<code>undefined</code>", issue a
developer warning that the type is unsupported.
</li>
<li>Return <code>undefined</code>.
</li>
</ol>
</li>
<li>If <a>Trim</a>(value) is the empty string, then return
<code>undefined</code>.
</li>
<li>Otherwise, <a>parse</a> <var>value</var> using <var>manifest
URL</var> as the base URL and return the result.
</li>
</ol>
</section>
<section>
<h3>
<code>scope</code> member
</h3>
<p>
The <dfn data-lt="serviceworker-scope"><code>scope</code> member</dfn> of a
<a>serviceworker object</a> is the service worker's associated
<a href="https://w3c.github.io/ServiceWorker/#dfn-scope-url">scope URL</a>.
Copy link
Member

Choose a reason for hiding this comment

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

nit, scope URL should be defined at the end of the document, so not to include the link to the SW spec here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

</p>
<p>
The <dfn>steps for processing the <code>scope</code> member of a
service worker</dfn> are given by the following algorithm. The algorithm takes
a <a>serviceworker object</a> <var>registration</var>, and a <a>URL</a> <var>manifest
URL</var>, which is the <a>URL</a> from which the
<var>manifest</var> was fetched. This algorithm will return a
<a>URL</a> or <code>undefined</code>.
</p>
<ol>
<li>Let <var>value</var> be the result of calling the
<a>[[\GetOwnProperty]]</a> internal method of <var>registration</var>
passing " <code>scope</code>" as the argument.
</li>
<li>Let <var>type</var> be <a>Type</a>(<var>value</var>).
</li>
<li>If <var>type</var> is not "string", then:
<ol>
<li>If <var>type</var> is not "<code>undefined</code>", issue a
developer warning that the type is unsupported.
</li>
<li>Return <code>undefined</code>.
</li>
</ol>
</li>
<li>If <a>Trim</a>(value) is the empty string, then return
<code>undefined</code>.
</li>
<li>Otherwise, <a>parse</a> <var>value</var> using <var>manifest
URL</var> as the base URL and return the result.
</li>
</ol>
</section>
<section>
<h3>
<code>type</code> member
</h3>
<p>
The <dfn data-lt="serviceworker-type"><code>type</code> member</dfn> of a
<a>serviceworker object</a> is the service worker's
<a href="https://html.spec.whatwg.org/multipage/workers.html#workertype">worker type</a>.
</p>
<p>
The <dfn>steps for processing the <code>type</code> member of a
service worker</dfn> are given by the following algorithm. The algorithm takes
a <a>serviceworker object</a> <var>registration</var>, and a <a>URL</a> <var>manifest
URL</var>, which is the <a>URL</a> from which the
<var>manifest</var> was fetched. This algorithm will return a
Copy link
Member

Choose a reason for hiding this comment

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

Hmm... what if the manifest redirected during fetch... we might need to clarify that somewhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you help with that? :-)

string.
</p>
<ol>
<li>Let <var>value</var> be the result of calling the
<a>[[\GetOwnProperty]]</a> internal method of <var>registration</var>
passing " <code>type</code>" as the argument.
</li>
<li>Let <var>type</var> be <a>Type</a>(<var>value</var>).
</li>
<li>If <var>type</var> is not "string", then:
<ol>
<li>If <var>type</var> is not "<code>undefined</code>", issue a
developer warning that the type is unsupported.
</li>
<li>Return <code>"classic"</code>.
</li>
</ol>
</li>
<li>If <a>Trim</a>(value) is the empty string, then:
<ol>
<li>Issue a developer warning that the type is unsupported.
Copy link
Member

Choose a reason for hiding this comment

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

This seems odd... you are returning undefined here, but "classic" above?

Copy link
Member

Choose a reason for hiding this comment

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

I.e., should we recover to "classic" in error cases for backwards compat?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is how it works for Link element. What do you think @jungkees ?

</li>
<li>Return <code>undefined</code>.
</li>
</ol>
<li>Return <var>type</var>.
Copy link

Choose a reason for hiding this comment

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

Shouldn't the return value be value.[[Value]] instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have

  • Let type be Type(value). above. That is how @marcoscaceres did it elsewhere. I think that should be fine. Why the value.[[Value]]? looks like a private slot

    •

    Copy link

    Choose a reason for hiding this comment

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

    Ah.. I got it. It has to do with the prefix ! for an ECMAScript abstract operation. So, the value here is the actual value for this member already. But then it still needs to return value instead of type. Referring to other cases, it should return Trim(value) here I suppose.

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    right, fixed

    </li>
    </ol>
    </section>
    <section>
    <h3>
    <code>use_cache</code> member
    </h3>
    <p>
    The <dfn data-lt="serviceworker-type"><code>use_cache</code> member</dfn> of a
    <a>serviceworker object</a> determines whether the user agent
    <a href="https://w3c.github.io/ServiceWorker/#dfn-use-cache">cache</a> should
    be used when fetching the service worker.
    </p>
    <p>
    The <dfn>steps for processing the <code>use_cache</code> member of a
    service worker</dfn> are given by the following algorithm. The algorithm takes
    a <a>serviceworker object</a> <var>registration</var>, and a <a>URL</a> <var>manifest
    URL</var>, which is the <a>URL</a> from which the
    <var>manifest</var> was fetched. This algorithm will return a
    boolean.
    </p>
    <ol>
    <li>Let <var>value</var> be the result of calling the
    <a>[[\GetOwnProperty]]</a> internal method of <var>registration</var>
    passing " <code>use_cache</code>" as the argument.
    </li>
    <li>Let <var>type</var> be <a>Type</a>(<var>value</var>).
    </li>
    <li>If <var>type</var> is not "boolean", then:
    <ol>
    <li>If <var>type</var> is not "<code>undefined</code>", issue a
    developer warning that the type is unsupported and
    return <code>undefined</code>.
    </li>
    <li>Otherwise, return False</li>
    Copy link
    Member

    Choose a reason for hiding this comment

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

    nit: return <code>false</code>

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    k

    </ol>
    </li>
    <li>Return <var>use_cache</var>.
    Copy link

    Choose a reason for hiding this comment

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

    Shouldn't the return value be value.[[Value]] instead?

    Copy link

    Choose a reason for hiding this comment

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

    It seems it should return value here. See "steps for processing the prefer_related_applications member" as an example.

    Copy link
    Collaborator Author

    Choose a reason for hiding this comment

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

    correct

    </li>
    </ol>
    </section>
    </section>
    <section>
    <h2>
    Application object and its members
    Expand Down Expand Up @@ -2988,6 +3272,14 @@ <h2>
    <dfn><code>default-src</code></dfn></a> directives are defined in
    [[!CSP3]].
    </p>
    <p>
    The <a href="https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy">
    <dfn>Is origin potentially trustworthy</dfn></a> is defined in [[!SECURE-CONTEXTS]].
    </p>
    <p>
    The <a href="https://w3c.github.io/ServiceWorker/#start-register"><dfn>Start Register</dfn></a>
    is defined in [[!SERVICE-WORKERS-1]].
    </p>
    <p>
    The <a href=
    "https://www.ecma-international.org/ecma-402/1.0/#sec-6.2.2"><dfn>IsStructurallyValidLanguageTag</dfn></a>
    Expand Down