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

navigator.serviceWorker.ready has surprising calling-frame behavior #356

Closed
dominiccooney opened this issue Jul 4, 2014 · 52 comments
Closed

Comments

@dominiccooney
Copy link

"ready" step 3.4 says:

Let document be entry settings object's responsible document.

This means that someframe.contentWindow.navigator.serviceWorker.ready looks at registrations from the caller's document and not someframe.contentDocument. Can you clarify this is intended because it is surprising that it does not refer to the document in the frame.

@jungkees
Copy link
Collaborator

jungkees commented Jul 7, 2014

I might have misinterpreted the term entry settings object. It says "the most-recently added script settings object in the stack ..."

Is just "settings object" fine? or "incumbent settings object"?

@dominiccooney
Copy link
Author

My understanding is "entry settings objects" are to do with script contexts. In Blink these are not implemented explicitly but by examining a stack of V8 scripting contexts.

It seems strange to look up a document from the script context when you have a document implied by the relationship between window, document, navigator and navigator.serviceWorker. Probably the spec concept you need to use is Browsing Context, not Entry Settings Object.

In the case of "ready" I think the only script-related thing is which script context the Promise's prototype chain comes from.

@jungkees
Copy link
Collaborator

In my understanding, "settings object" is a conceptual term that abstracts various settings of document's scripts. That is, for example, a certain browsing context's active document can have its settings object and this settings object can be used to explain related concepts (e.g. responsible document, etc.) in spec language.

In ready attribute's 2.2, "Let document be settings object's responsible document." I intended this document be the document that has the script within which the ready method was invoked from.

I think I need help to clarify this. @domenic @bzbarsky can you help?

@bzbarsky
Copy link

Why do you want the document that has the script the ready method was invoked from instead of, say, the document which corresponds to the ready method's Realm?

Basically, can you describe in non-spec terms, ideally in terms of several different examples, what sort of behavior you're looking for and why?

@jungkees
Copy link
Collaborator

Why do you want the document that has the script the ready method was invoked from instead of, say, the document which corresponds to the ready method's Realm?

settings object is defined and used in HTML, XHR, Fetch and others specs already while I could not find a usage of Realm in those web specs.

Basically, can you describe in non-spec terms, ideally in terms of several different examples, what sort of behavior you're looking for and why?

Service Workers spec describes the relationship between a document and its Service Worker registration, etc., so there are quite a few places in the algorithms where I have to refer to the document context of a Service Worker. (i.e. navigator.serviceWorker)

I think Run a worker algorithm in HTML spec is one such example as well. (See the step 4 of the algorithm using "responsible document specified by settings object" terms.)

@bzbarsky
Copy link

settings object is defined and used in HTML, XHR, Fetch and others specs already

Yes, for things like base URIs and referrers.... What are you trying to use it for here?

while I could not find a usage of Realm in those web specs

That's because Realms are pretty new.

so there are quite a few places in the algorithms where I have to refer to the document context of a
Service Worker

What does "document context" mean here? It's not just a document, since service workers are shared across documents, right?

See the step 4 of the algorithm using "responsible document specified by settings object" terms

Right. This is using the responsible document to get the right value of referrer. That's a sane thing to do.

At first glance, though, that's not what you're trying to use the responsible document for here.

@jungkees
Copy link
Collaborator

What does "document context" mean here? It's not just a document, since service workers are shared across documents, right?

I meant just a document, which contains script that runs code like navigator.serviceWorker.ready.then(reg => { });.

Right. This is using the responsible document to get the right value of referrer. That's a sane thing to do.

Actually, the algorithm is using the responsible document as the value of referrer for Fetch request. So, how could it refer to this document rather than saying "responsible document of setting's object" here? I think this is something I would like to do indeed.

@bzbarsky
Copy link

I don't see any mention of referrer in the document usage at http://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#navigator-service-worker-ready. Are you talking about some other part of the spec?

If you need a referrer, then using the responsible document of the incumbent settings object makes sense. Do note the "incumbent" part; just talking about "the settings object" doesn't work, because you need to be clear on whether you mean the incumbent one or the entry one.

@bzbarsky
Copy link

Oh, and the point is what happens when script in document 1 touches navigator.serviceWorker from document 2? Should that really wait for registrations in document 1, as opposed to document 2? Seems to me like it shouldn't....

@jungkees
Copy link
Collaborator

In the 2nd paragraph of #356 (comment), I was talking about the run a worker algorithm in HTML not certain algorithms in SW.

@bzbarsky
Copy link

Yes, that's what I was talking about in #356 (comment) as well. The HTML spec is fine here: if a referrer value is needed, then you want the incumbent settings object's responsible document.

But this issue isn't about a spec bit that needs a referrer.

@jungkees
Copy link
Collaborator

I see. So, the responsible document should only be used for certain purpose like getting a value for a referrer.

Then, will this do? (The purpose is to simply refer to the current document within which the method-calling script is running):
"Let document be the Document which corresponds to this method's Realm."

Or any better suggestion?

@jungkees
Copy link
Collaborator

I have one more such case which I would like to have your comment:
[[HandleFetch]] step 6:
"Let document be the responsible document specified by the relevant settings object for request's client."

Here request's client is a JavaScript global environment. So, I'm trying to make an algorithm step to declare a variable for a document that's providing the JavaScript global environment referred to by the request's client.

@bzbarsky
Copy link

Can you please stop trying to guess spec text and explain what behavior you're after and why? #356 (comment) is talking about "method-calling script" but then using the realm of the method itself. Which one do you really want?

For the HandleFetch bit, a Window always has a script settings object. We can probably try to guarantee that any global does, though making this play nice with ES6 Realms will be exciting.

@jungkees
Copy link
Collaborator

Can you please stop trying to guess spec text and explain what behavior you're after and why? #356 (comment) is talking about "method-calling script" but then using the realm of the method itself. Which one do you really want?

// example.com/index.html
navigator.serviceWorker.ready.then(reg => { reg.active.postMessage("cool.") });

.ready attribute returns a Promise which resolves with a registration when it acquires an active Service Worker. In the above example, in order for the Promise object returned by .ready to resolve, the document, index.html, needs a matching Service Worker registration. And under that condition, the document's Service Worker registration should have acquired an active Service Worker. Otherwise, the algorithm should wait until the conditions are met.

This is the behavior that I would like to specify in the algorithm, and I tried to use more accurate expression than just "this document", "the current document" or something.

@bzbarsky
Copy link

Thank you! That's very helpful. So a web page does:

subframe.navigator.serviceWorker.ready.then((reg) => {});

then you want to deal with the document of the subframe, right? That's what #356 (comment) was about to start with.

What do you want to happen when the caller does:

window.open("http://some-same-origin-uri").navigator.serviceWorker.ready.then((reg) => {});

Note that in this case there is one document in the window (initial about:blank) when the then() call happens but later a different document is loaded. At the same time the navigator.serviceWorker object identity doesn't change, since the new document is same-origin with the initial about:blank so the Window and everything hanging off it gets reused for the new Document.

@jungkees
Copy link
Collaborator

@bzbarsky @coonsta Thank you very much for the explanation. The document in the algorithm should basically be the current browsing context's active document. I'll have this in mind when revisiting the ready algorithm.

@bzbarsky
Copy link

Where is the term "current browsing context" defined? I don't see a definition anywhere.

@jungkees
Copy link
Collaborator

Current is not a defined term. It is just used in HTML spec to explain the relationship between the opener and the auxilary browsing context some such. I'll need to find more accurate terms to define the algorithm. Sorry I just got off the work and have a week vacation actually. I'll get back. Thanks for help! :)

@dominiccooney
Copy link
Author

Implementer feedback: Blink's implementation of ready uses the browsing context in the way @bzbarsky describes above, and not the entry settings object.

@KenjiBaheux
Copy link
Collaborator

@jungkees Should we close this? It seems that we have an agreement and there hasn't been any movement for a while.
Tentatively adjusting labels.

@jungkees
Copy link
Collaborator

Yes, now we're using context object's associated document instead of settings object's responsible document.

@jungkees
Copy link
Collaborator

Yes, "context object" and "document" concepts xref the definitions in the DOM spec. Just fixed the broken links. "associated document" exactly means the same.

@bzbarsky
Copy link

Exactly the same as what? In the DOM spec this phrase is only used for:

  1. Global objects, which explicitly define what their "associated document" is.
  2. DOMImplementation, which expliciltly defines what its "associated document" is.

There's nothing magical about the phrase "associated document" itself; it just refers to particular things defined for the particular objects it's used with.

Your spec is using it for some other random objects (e.g. ServiceWorkerRegistration, ServiceWorkerContainer) which do not define anything about associated documents.

@jungkees
Copy link
Collaborator

The definition of "context object" in Dom spec does not confine the target object to any specific object:

The term context object means the object on which the algorithm, attribute getter, attribute setter, or method being discussed was called. When the context object is unambiguous, the term can be omitted.

@jungkees jungkees reopened this Sep 18, 2014
@jungkees
Copy link
Collaborator

I understood:

Your spec is using it for some other random objects (e.g. ServiceWorkerRegistration, ServiceWorkerContainer) which do not define anything about associated documents.

Could you share a pointer to

Global objects, which explicitly define what their "associated document" is.

It'd be of great help.

@bzbarsky
Copy link

Ah, looks like those do not in fact, not by that name. They have a .document but they no longer call it "associated document". That should be fixed too, either in HTML or in the DOM spec.

@bzbarsky
Copy link

The definition of "context object" in Dom spec does not confine the target object to any specific object:

Sure. I have no problem with your use of "context object" as long as you make it clear you're using the DOM spec's definition for it.

@domenic
Copy link
Contributor

domenic commented Sep 18, 2014

FWIW I have always found the "context object" verbiage to be very confusing, and I would never inflict it on someone if I had the choice. Explicitly naming your variables is much better IMO.

@bzbarsky
Copy link

"context object" is basically the "this" value (after munging via [ImplicitThis] if needed). Right now Web IDL invokes spec prose like so:

Let R be the result of performing (on O, if the operation is not a static operation) the actions listed in
the description of operation with values as the argument values. 

or

Set idlValue to be the result of performing the actions listed in the description of the attribute that
occur when getting (or those listed in the description of the inherited attribute, if this attribute is
declared to inherit its getter), with O as the object. 

or

If the attribute is a regular attribute, then perform the actions listed in the description of the attribute
that occur when setting, with O as the object and idlValue as the value.

(for methods, getters, setters respectively). We could give O a nice name for spec prose to reference, if desired... Suggestions?

@domenic
Copy link
Contributor

domenic commented Sep 18, 2014

What about this?

@bzbarsky
Copy link

We could do this, as long as people are clear on the difference between it and the "this value" passed to [[Call]]. This is closer to what evaluating "this" would do in an ES function. Except in the window case, of course, where it's Window, not the WindowProxy.

@domenic
Copy link
Contributor

domenic commented Sep 18, 2014

We could do this, as long as people are clear on the difference between it and the "this value" passed to [[Call]].

I guess I myself am not clear on the difference, so maybe not the best idea :(

We should perhaps take this to the WebIDL bug tracker or similar so as to avoid derailing this thread...

@annevk
Copy link
Member

annevk commented Sep 19, 2014

Ideally IDL defines context object and such terminology, yes.

@jungkees
Copy link
Collaborator

We should perhaps take this to the WebIDL bug tracker or similar so as to avoid derailing this thread...

Thanks :-)

Going back to the original issue, #356 (comment), the document referred to should be something like:

ready attribute's realm's [[globalThis]]'s browsing context's active document

I'm still lost in finding a correct expression in terms of the spec language. It should basically be the browsing context's active document, but still the browsing context should be qualified. The incumbent settings object's responsible document is really wrong here?

@bzbarsky
Copy link

It should basically be the browsing context's active document

Why, exactly? If I have a subframe, and I grab navigator.serviceWorker from that subframe, then I navigate that subframe to some other URL (possibly with a different origin!) and then I get .ready on the object I am holding, should this really somehow involve that new document in the subframe?? That seems pretty unlikely to me.

@jungkees
Copy link
Collaborator

Here's what HTMLIFrameElement.contentDocument defines:

The contentDocument IDL attribute must return the Document object of the active document of the iframe element's nested browsing context, if any and if its effective script origin is the same origin as the effective script origin specified by the incumbent settings object, or null otherwise.

So it basically refers to the active document of the (nested) browsing context at run time.

But the browsers seem to show different behaviors (with different quirks indeed.) Here's my finding:

<!-- a.html -->
<!DOCTYPE HTML>
<title>Outer page</title>
<iframe src="b.html"></iframe>
<script>
  window.title = "Outer";
  window.onload = function() {
    var subframe = document.getElementsByTagName("iframe")[0];
    var subframeGlobal = subframe.contentWindow;
    var hello = subframeGlobal.hello;
    subframeGlobal.location.assign("c.html");
    setTimeout(function() {
      hello();
    }, 0);
  };
</script>

<!-- b.html -->
<!DOCTYPE HTML>
<title>Inner page B</title>
This is a page B.
<script>
  window.title = "Page B";
  function hello() {
    console.log("hello in page B called.");
  }
</script>

<!-- c.html -->
<!DOCTYPE HTML>
<title>Inner page C</title>
This is a page C.
<script>
  window.title = "Page C";
  function hello() {
    console.log("hello in page C called.");
  }
</script>
  • Chrome says: "hello in page B called." (same when page a.html reloaded.)
  • FF says: "hello in page B called." in the initial load and "hello in page C called." when reloaded.
  • IE says: Can't execute code from a freed script.

The active document of the browsing context sounds reasonable to me as .ready returns a promise to a SW registration with an active worker that controls documents, and the document to be controlled here is the new document created by the navigation. Note that Service Worker scope matching still happens even if the new document has a different origin. (If no scope matched for it, it loads without SW.)

@bzbarsky
Copy link

Here's what HTMLIFrameElement.contentDocument defines:

.contentDocument takes an object in document A (the iframe element), and goes from it to some other document B (the document loaded in the iframe right this second).

It's not doing anything with the active document of the browsing context A is loaded in.

In your situation, on the other hand, you're taking a navigator object that hangs off some Window (which has some corresponding Document) and then having a method on it inspect some totally different Document. That's a really bizarre thing to do, unless you think this ready property is in some way a property of the browsing context itself, not of the window the Navigator came from.

Your testcase is racy, because it might run the hello function while b.html is still loaded in the subframe.

that controls the document

WHICH DOCUMENT?

and the document here has already been loaded again with a new URL

Uh.. no. URLs of documents are immutable (modulo document.open). Performing a navigation creates a new document!

@bzbarsky
Copy link

Your testcase is racy, because it might run the hello function while b.html is still loaded in the subframe.

The right way to fix that would be to run the function off the load event for the <iframe> element. But a cheap hack that will be good enough for manual testing purposes would be to use a 2s timeout instead of a 0ms one.

Furthermore, note that on reload via the reload button some browsers will preserve which documents are loaded in subframes and some will not, which is why Firefox logs C when you reload. If you want a clean reload that doesn't try to preserve pre-reload state you need to either shift-reload or focus the url bar and hit enter.

@jungkees
Copy link
Collaborator

WHICH DOCUMENT?

I rephrased the part of the text as:

The active document of the browsing context sounds reasonable to me as .ready returns a promise to a SW registration with an active worker that controls documents, and the document to be controlled here is the new document created by the navigation.

(I haven't gone through all the comments yet. Will do. Thanks!)

@bzbarsky
Copy link

Why is the document to be controlled here the new document, exactly? That doesn't make any sense to me.

Let me try a concrete example.

Say I load a document in a subframe then I grab a reference to that document. Then I navigate that subframe to some other URI. Then I take my reference to the now-unloaded document and createElement("img") and set its src.

Should that load be controlled by a serviceworker? Which one?

@slightlyoff
Copy link
Contributor

hey @bzbarsky,

Let me try to answer your last question first and perhaps work backward to something that makes sense for the spec.

Logically, in the situation you just posed, the img that was created, the node is still "owned" by the now disconnected document (and not the new one). That is to say, img.ownerDocument should be whatever is on the left-hand side of createElement() (unless there's some subtlety I've missed). Therefore the img load should be controlled by whatever SW that document started life under.

There is a situation in which this can be a different SW than whatever is controlling the parent document, and that's when the parent registered the SW and, at some point later, created the iframe (which received a SW for the subdocument upon navigation).

Now lets expand that to the 3-document case (assuming they're all on the same origin). Assuming parent has no SW, first subdocument does, and we load a new one in palce of the first subdocument (but which we keep a reference to). Since the first subdocument isn't dead and has the SW version on "life support", the new document will get the same SW controlling it. A call to e.replace() in onisntall() will change the SW for ALL of the document objects in question.

In terms of the spec language, the elusive quantity is some way of saying "when a navigation happens, the resulting document is fused to a SW until (and unless) and updated or newly-registered SW script version calls e.replace()"

@bzbarsky
Copy link

Therefore the img load should be controlled by whatever SW that document started life under.

Yes, totally agreed with the reasoning and the conclusion.

Now lets expand that to the 3-document case (assuming they're all on the same origin).

We should consider the non-same-origin case too, fwiw (at least for the two documents in the subframe). That's the one that makes the "active document" thing really totally bogus. ;) I don't think it changes any of the reasoning here, though.

when a navigation happens, the resulting document is fused to a SW until (and unless) and updated
or newly-registered SW script version calls e.replace()

That seems reasonable.

So circling back to the actual topic of discussion here, which is what the ready getter should return....

It seems to me that what needs to happen is that a ServiceWorkerContainer needs to be bound to either a Window or a Document (depending on how we want to handle various edge cases like (1) service worker registration racing against loads that replace an initial about:blank document and (2) document.open, which keeps the same Document but creates a new Window). Then its .ready getter should return the ServiceWorkerRegistration for either the document of the Window it's bound to or for the Document it's bound to.

Binding a ServiceWorkerContainer to a Window is a fairly natural thing to do, since it needs to have some prototype and that prototype needs to come from some Window, so this needs to be defined anyway in practice (probably by saying that a Navigator needs to be associated from the Window it's gotten from, if there is no spec that says this already, and the ServiceWorkerContainer needs to be associated with the same Window as the Navigator it's gotten from).

In the document.open() case, the Window will change, but the new Window has the same Document as the old one, so for purposes of getting the document it doesn't matter which one we use.

In the case of script doing var win = window.open("same-origin-URI"); and then immediately touching win.navigator.serviceWorker.ready it will presumably get a registration for the initial about:blank document, which also presumably matches the document where the code that called window.open lives. When the navigation completes, the Window will stay the same but get a new Document, which may possibly have a different ServiceWorkerRegistration (e.g. if the registration happens right after the window.open call?). Presumably that's fine; getting .ready will just return the new thing. I don't know whether we also want to return a different ServiceWorkerContainer in that situation or not.

Incidentally, having an attribute that returns a new Promise each time is pretty weird. That's what the spec seems to say, but certainly not what Gecko has implemented...

@domenic
Copy link
Contributor

domenic commented Oct 16, 2014

Incidentally, having an attribute that returns a new Promise each time is pretty weird. That's what the spec seems to say, but certainly not what Gecko has implemented...

This should indeed be avoided if possible... The only case in which it should return a new promise is if you need to transition back to un-ready, in which case the internal promise gets re-set and the getter starts returning that new promise.

@jungkees
Copy link
Collaborator

Updated the ready algorithm: 37026a5. Defined the associated Document to get a handle to the right document, and ready promise / ready service worker registration to not create a new promise each time.

@jungkees
Copy link
Collaborator

ServiceWorkerContainer's ready service worker registration has been removed as not needed: 485e0f1.

@bzbarsky
Copy link

Thank you. A few quibbles:

  1. I'm not sure what "Run the following substeps in parallel" means. In parallel with each other, or with step 4?

They can't run in parallel with step 4 until you get past step 3 susbstep 2, since you'd want to get the document URL before returning control to scripts that can modify it.

  1. "has an associated Document object of the same Window object as the Navigator object from which it is gotten" doesn't quite make sense. I think you want this to say: "A ServiceWorkerContainer's associated Window is the associated Window of the Navigator object that the ServiceWorkerContainer is retrieved from. A ServiceWorkerContainer's associated Document is the newest Document object of its associated Window." Here "newest Document Object" is the terminology that https://html.spec.whatwg.org/multipage/browsers.html#dom-document-2 uses; you could probably even link to it here if desired, since that's the behavior you're after.

And we should add some document.open() tests to the test suite, I guess. ;)

@jungkees
Copy link
Collaborator

  1. I'm not sure what "Run the following substeps in parallel" means. In parallel with each other, or with step 4?

In parallel with step 4.

Your comment has been addressed for both (1) and (2): 6088b93. Thanks.

One question:

"A ServiceWorkerContainer's associated Document is the newest Document object of its associated Window."

What's the difference (semantically) between Document and Document object in the text? (I mean generally in IDL terms.)

@bzbarsky
Copy link

Technically "ServiceWorkerContainer" is an interface and "ServiceWorkerContainer object" is an instance of that interface. Good point. It's easy to slip into just saying "the Foo" when "the Foo object" or "object implementing Foo" is meant, esp. in the case of singleton-like interfaces.

@jungkees
Copy link
Collaborator

I believe we can close the thread.

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

No branches or pull requests

7 participants