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.

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

Changing useCache boolean to updateViaCache enum #1107

Merged
merged 2 commits into from
Jun 7, 2017

Conversation

jakearchibald
Copy link
Contributor

No description provided.

@jakearchibald
Copy link
Contributor Author

jakearchibald commented Apr 7, 2017

TODO:

  • Update HTML spec
  • Tests

@jakearchibald
Copy link
Contributor Author

HTML changes whatwg/html#2515 - I need to do some work here.

docs/index.bs Outdated
@@ -201,7 +201,7 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe

A [=/service worker registration=] has an associated <dfn export id="dfn-last-update-check-time">last update check time</dfn>. It is initially set to null.

A [=/service worker registration=] has an associated <dfn export id="dfn-use-cache">use cache</dfn> (a boolean). It is initially set to false.
A [=/service worker registration=] has an associated <dfn export id="dfn-update-via-cache">update via cache mode</dfn>, which is "`imports`", "`all`", or "`never`". It is initially set to "`imports`".
Copy link
Collaborator

Choose a reason for hiding this comment

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

The IDL definition for the enum uses "none" instead of "never". I think "none" sounds better considering service-workers mode uses "none".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh wow, good catch

docs/index.bs Outdated
@@ -2276,7 +2282,7 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
1. Let |registration| be |serviceWorker|'s [=containing service worker registration=].
1. Set |request|'s [=service-workers mode=] to "`foreign`".
1. Set |request|'s [=request/cache mode=] to "<code>no-cache</code>" if any of the following are true:
* |registration|'s [=service worker registration/use cache=] is false.
* |registration|'s [=service worker registration/update via cache mode=] is "`never`".
Copy link
Collaborator

Choose a reason for hiding this comment

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

"none" or "never" depending on the choice.

docs/index.bs Outdated
@@ -2392,7 +2398,7 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe

A <a>job</a> has a <dfn id="dfn-job-worker-type">worker type</dfn> ("<code>classic</code>" or "<code>module</code>").

A <a>job</a> has a <dfn id="dfn-job-use-cache">use cache</dfn> (a boolean).
A <a>job</a> has an <dfn id="dfn-job-update-via-cache-mode">update via cache mode</dfn>, which is "`imports`", "`all`", or "`never`".
Copy link
Collaborator

Choose a reason for hiding this comment

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

"none" or "never" depending on the choice.

docs/v1/index.bs Outdated
@@ -190,7 +190,7 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe

A [=/service worker registration=] has an associated <dfn export id="dfn-last-update-check-time">last update check time</dfn>. It is initially set to null.

A [=/service worker registration=] has an associated <dfn export id="dfn-use-cache">use cache</dfn> (a boolean). It is initially set to false.
A [=/service worker registration=] has an associated <dfn export id="dfn-update-via-cache">update via cache mode</dfn>, which is "`imports`", "`all`", or "`never`". It is initially set to "`imports`".
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as nightly: Use "none" or "never" consistently.

@jungkees
Copy link
Collaborator

@jakearchibald, sorry for the late feedback. I suppose you're extremely busy for now. Please check this out when you find time.

@jakearchibald
Copy link
Contributor Author

@jungkees updated this, and also updated the tests web-platform-tests/wpt#5515

@jungkees
Copy link
Collaborator

jungkees commented Jun 7, 2017

LGTM. I'll merge this as HTML's already merged the related changes and the tests are underway.

@jungkees jungkees merged commit c92c148 into master Jun 7, 2017
@jakearchibald
Copy link
Contributor Author

Cheers @jungkees!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants