Extend environment concept #2004

jungkees · 2016-11-02T05:21:34Z

Add execution ready flag, which is set when the corresponding
environment settings object is created and the potential main resource
is fetched (in the case of an initial browsing context creation, the
main resource fetch doesn't occur and not considered as a condition.)
. This flag is mainly used in Service Worker's Client API (i.e. to
get a value for client.reserved)
- Define the origin algorithm that returns the creation URL's origin
. The origin algorithms defined in the environment settings objects
override the behavior.
. The primary purpose of defining this algorithm is to allow a
polymorphic dispatch depending on the type of objects (i.e. to
simplify the steps in the call sites.)

Related issue: #1992

- Add execution ready flag, which is set when the corresponding environment settings object is created and the potential main resource is fetched (in the case of an initial browsing context creation, the main resource fetch doesn't occur and not considered as a condition.) . This flag is mainly used in Service Worker's Client API (i.e. to get a value for client.reserved) - Define the origin algorithm that returns the creation URL's origin . The origin algorithms defined in the environment settings objects override the behavior. . The primary purpose of defining this algorithm is to allow a polymorphic dispatch depending on the type of objects (i.e. to simplify the steps in the call sites.) Related issue: whatwg#1992

zcorpan · 2016-11-02T07:51:57Z

Error: Multiple definitions for term "origin" Parent of first says: "An origin is one of the following:", parent of second says: "An environment has an algorithm defined as the origin that returns the environment's creation URL's origin."

Unfortunately wattsi doesn't know about the data-dfn-for attributes AFAIK. Can use data-x="concept-environment-origin" or so (in addition).

annevk · 2016-11-02T07:54:48Z

source

@@ -85800,6 +85806,14 @@ interface <dfn>NavigatorOnLine</dfn> {
   data-x="dfn-control">controls</span> the <span>environment</span>.</p></dd>
  </dl>

+  <p>An <span>environment</span> has an <dfn data-x="concept-environment-execution-ready-flag"
+  data-export="">execution ready flag</dfn>, which is initially unset.</p>


This should also have a data-dfn-for="environment".

Ah.. just missed this comment before pushing the data-x commit. Will add this.

jungkees · 2016-11-03T07:19:56Z

Added client.reserved, which references the execution ready flag, in a SW's topic branch:
w3c/ServiceWorker@d88e979 Would like to merge to master when this PR is landed.

jungkees · 2016-11-03T08:52:43Z

On reflection, for the origin, I think I can also do it in SW by defining an origin field on the service worker client. Not sure if other specs (e.g. Secure Context) will need a reference to the environment's origin in the future, though. If that's the case, having it in the environment will still be useful.

This removes the environment's origin algorithm definition. The requirement is now covered by Service Workers spec with the service worker client's origin algorithm: https://w3c.github.io/ServiceWorker/#service-worker-client-origin

jungkees · 2016-11-04T06:53:41Z

I removed the environment's origin definition from this PR and spec the algorithm in SW instead: https://w3c.github.io/ServiceWorker/#service-worker-client-origin

So, this PR is now only for adding the execution ready flag.

If we need an origin algorithm on environment in the future, I'll make a separate PR for that.

domenic · 2016-11-04T18:39:42Z

source

@@ -85800,6 +85806,10 @@ interface <dfn>NavigatorOnLine</dfn> {
   data-x="dfn-control">controls</span> the <span>environment</span>.</p></dd>
  </dl>

+  <p>An <span>environment</span> has an <dfn data-x="concept-environment-execution-ready-flag"


This should be added to the previous <dl>, I think?

I was wondering whether I should get both the fields and the flag in the same <dl> considering we put it as "An environment has the following fields". So, I guess a flag is a field? I'm fine with adding it to the <dl> if so.

A field can be a flag, yeah :). Flag is a type, kind of like a boolean but just a bit more awkward to use.

That makes sense. Addressed the comment.

domenic

LGTM with nit but would appreciate @annevk's review as well.

domenic · 2016-11-07T22:39:35Z

source

-   <li><p><span>Set up a browsing context environment settings object</span> with
-   <var>realm execution context</var>.</p></li>
+   <li><p><span>Set up a browsing context environment settings object</span> with <var>realm
+   execution context</var>, and set <var>settingsObject</var> to the result.</p></li>


let settingsObject be the result; this is the first introduction of the settingsObject variable.

Addressed the comment. Thanks.

annevk · 2016-11-08T09:05:10Z

My main concern is that I don't have the full flow in my head. And we're adding this state for lifetime management of a global, but we don't fully understand the lifetime of globals yet.

jungkees · 2016-11-08T12:57:54Z

Trying to clarify, client.reserved is the only attribute which uses this field. The purpose of .reserved is to distinguish the state before and after the moment when the UA's to be able to parse a fetched (or given default) document or execute a worker script. This hasn't been needed as we've never exposed any Client-like object before the creation of its global object and realm execution context and the main resource (for parsing/executing) is ready. Now we expose Client objects in that state (during navigations and worker creations), we should provide an API to check this state.

annevk · 2016-11-08T13:02:10Z

What is the SW issue for adding reserved? And why is that not a promise-state attribute? E.g., client.ready.then(...)?

jungkees · 2016-11-08T13:16:52Z

What is the SW issue for adding reserved?

It started as client.used in w3c/ServiceWorker#870 (comment) The name came up during the f2f I recall.

why is that not a promise-state attribute? E.g., client.ready.then(...)?

Good point. Once a Client is gotten from client-getter methods, it's a snapshot. What I missed is the value shouldn't be changed once the Client object is created. So, it should be a Client's field which is set by the object creation algorithm.

/cc @jakearchibald

jungkees · 2016-11-08T13:26:13Z

Also note that execution ready flag is used in clients.matchAll(options) to filter the clients by options.includeReserved = false. So, the flag is being used in two places actually: client.reserved and clients.matchAll(options).

annevk · 2016-11-08T13:29:33Z

Okay, so if reserved doesn't change and we read the execution ready flag when we asynchronously gather state to create Client objects I think this is all okay.

Would be good to have the issue in SW filed so we can mention that in the commit message here. Makes it easier for folks in the future to figure out how everything relates.

jungkees · 2016-11-08T13:44:40Z

Okay. Will fix .reserved and file an issue for it.

annevk · 2016-11-08T13:47:44Z

Also, could you propose a commit message for this PR that includes all the relevant info? I or @domenic can then land it since I think the text we have here is fine.

jungkees · 2016-11-08T14:24:48Z

Sure. Will do when ready with all the relevant info. What's the convention to propose the final commit message? Update the OP or just a new comment here?

annevk · 2016-11-08T14:25:58Z

A new comment using the triple backtick syntax.

jungkees · 2016-11-09T07:33:32Z

I've addressed the client.reserved fix (w3c/ServiceWorker@d2c327c) and filed an issue for tracking the history (w3c/ServiceWorker#1003).

Here' the proposed commit message:

Add environment's execution ready flag

Service Worker's client.reserved attribute and clients.matchAll(/*
options.includeReserved */) method require getting the state of a
Client object whether its environment is in reserved state or not. This
commit adds the execution ready flag to the environment concept that
distinguishes the state between before and after the UA is ready to
parse a fetched (or given default) document or execute a worker script.
The execution ready flag is set when the global object and realm
execution context are created and the main resource (for
parsing/executing) is ready.

Fixes #1992
Related SW issue: https://github.com/w3c/ServiceWorker/issues/1003

annevk reviewed Nov 2, 2016

View reviewed changes

jungkees added 2 commits November 2, 2016 16:59

Add data-x to origin to resolve multiple definition error

69f75c3

Add data-dfn-for attribute to execution ready flag

e1ced39

Remove the origin algorithm

ed8576e

This removes the environment's origin algorithm definition. The requirement is now covered by Service Workers spec with the service worker client's origin algorithm: https://w3c.github.io/ServiceWorker/#service-worker-client-origin

jungkees mentioned this pull request Nov 4, 2016

Inconsistencies due to when clients are created w3c/ServiceWorker#870

Closed

domenic reviewed Nov 4, 2016

View reviewed changes

Move execution ready flag into <dl>

655c603

domenic approved these changes Nov 7, 2016

View reviewed changes

Use let for variable introduction

de7dde4

This was referenced Nov 9, 2016

Spec client.reserved w3c/ServiceWorker#1003

Closed

Add environment's execution ready flag #1992

Closed

annevk merged commit d358c78 into whatwg:master Nov 9, 2016

jungkees deleted the extend-environment branch November 9, 2016 13:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend environment concept #2004

Extend environment concept #2004

jungkees commented Nov 2, 2016 •

edited

Loading

zcorpan commented Nov 2, 2016

annevk Nov 2, 2016

jungkees Nov 2, 2016

jungkees commented Nov 3, 2016

jungkees commented Nov 3, 2016

jungkees commented Nov 4, 2016

domenic Nov 4, 2016

jungkees Nov 6, 2016

domenic Nov 6, 2016

jungkees Nov 7, 2016

domenic left a comment

domenic Nov 7, 2016

jungkees Nov 8, 2016

annevk commented Nov 8, 2016

jungkees commented Nov 8, 2016 •

edited

Loading

annevk commented Nov 8, 2016

jungkees commented Nov 8, 2016 •

edited

Loading

jungkees commented Nov 8, 2016

annevk commented Nov 8, 2016

jungkees commented Nov 8, 2016

annevk commented Nov 8, 2016

jungkees commented Nov 8, 2016

annevk commented Nov 8, 2016

jungkees commented Nov 9, 2016 •

edited

Loading

Extend environment concept #2004

Extend environment concept #2004

Conversation

jungkees commented Nov 2, 2016 • edited Loading

zcorpan commented Nov 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jungkees commented Nov 3, 2016

jungkees commented Nov 3, 2016

jungkees commented Nov 4, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk commented Nov 8, 2016

jungkees commented Nov 8, 2016 • edited Loading

annevk commented Nov 8, 2016

jungkees commented Nov 8, 2016 • edited Loading

jungkees commented Nov 8, 2016

annevk commented Nov 8, 2016

jungkees commented Nov 8, 2016

annevk commented Nov 8, 2016

jungkees commented Nov 8, 2016

annevk commented Nov 8, 2016

jungkees commented Nov 9, 2016 • edited Loading

jungkees commented Nov 2, 2016 •

edited

Loading

jungkees commented Nov 8, 2016 •

edited

Loading

jungkees commented Nov 8, 2016 •

edited

Loading

jungkees commented Nov 9, 2016 •

edited

Loading