Better define which entries are exposed for nested contexts #177

yoavweiss · 2018-10-31T16:49:38Z

Closes #166

yoavweiss · 2018-10-31T16:52:01Z

cc @bzbarsky - FYI. Would be great if you could review as well

yoavweiss · 2018-10-31T16:52:26Z

Related tests are at web-platform-tests/wpt#13823

annevk · 2018-11-02T11:27:38Z

index.html

+<a data-cite="FETCH#concept-fetch">fetched</a> by the current
+<a data-cite="HTML#browsing-context">browsing context</a>'s
+<a data-cite="HTML#active-document">active document</a>'s
+<a data-cite="HTML#relevant-settings-object">relevent settings object</a>


You dropped the bit about workers. It's also rather hard to review this as it's unclear what the context for "current" is. The user agent? If you're already in some scope, this seems wrong.

What would be a better way to define "current"? You're right about workers. How can I better refer them here?

Where in the spec is this used? If this is some API it ideally would return some pre-existing collection that other specifications produce.

So, maybe something like """

Resource requests which client is not null and which destination is not "document", "object" or "embed" should be added to the request client's relevant global object's performance timeline.

Resource requests which destination is "document", which were triggered by the "process the iframe attribute" step, and which client has a document which has a browsing context which has a parent browsing context, should be added to the request's client's document's browsing context's parent browsing context's relevant global object's performance timeline
"""

There's probably a better way to phrase this, but let me know if that would properly define what we need.

Things with destinations of "object" or "embed" should always be added, no?

I'm not sure I understand why we need all the conditions listed in the "document" case. But going from request client to browsing context to parent browsing context looks wrong to me, per the discussion above: the client is already the settings object of the parent.

A settings object has no "relevant global". I'm not sure what the best way in spec terms is to go from a settings object to the correct performance timeline object. @domenic, do you know?

isn't the "incumbent settings object" in location object navigate the settings object of the iframe's browsing context (rather than its parent)?

The incumbent settings object is the place where the href set or assign call happens. It could be anything at all, including the iframe itself, its parent, or some other window that has its hands on the Location object.

Similarly in step 13 of shared-declarative-refresh-steps, isn't the source browsing context the iframe's document browsing context?

Yes it is. I'm not sure why that matters here... we want to exclude the loads done by https://html.spec.whatwg.org/#shared-declarative-refresh-steps from resource timing, right?

isn't the "incumbent settings object" in location object navigate the settings object of the iframe's browsing context (rather than its parent)?

The incumbent settings object is the place where the href set or assign call happens. It could be anything at all, including the iframe itself, its parent, or some other window that has its hands on the Location object.

OK, thanks for correcting.

Similarly in step 13 of shared-declarative-refresh-steps, isn't the source browsing context the iframe's document browsing context?

Yes it is. I'm not sure why that matters here... we want to exclude the loads done by https://html.spec.whatwg.org/#shared-declarative-refresh-steps from resource timing, right?

We want to exclude it from the parent's resource timing, so for this case the entry won't be added to the parent's timeline, right?

Regardless, I'll add the conditions to only include "document" destinations triggered by "process the iframe-attribute"

We want to exclude it from the parent's resource timing

Right. That would be handled if we only add navigations that are caused by processing frame or iframe attributes, afaict...

aside: should the same exclusions be added to prevent Service Workers from observing the same URLs?

I haven't been following the service worker stuff closely, but my understanding is that generally navigations do want to be seen by service workers, but how one determines which service worker should see the navigation is not entirely clear to me and is not really related to resource timing.

For example, in the case of <iframe src="some-cross-origin-url"> we want the cross-origin url to show up in resource timing on the parent page of the iframe but the service worker that gets used should be the one for the target origin, not the one for the page the iframe is in, right?

annevk · 2018-11-02T11:29:17Z

index.html

+"document", "object" or "embed", and was not triggered by 
+<a data-cite="HTML#process-the-iframe-attributes">process the iframe attributes</a>,
+<a data-cite="HTML#process-the-object-attributes">process the object attributes</a>, or
+<a data-cite="HTML#process-the-embed-attributes">process the embed attributes</a>, 


Hmm, I don't think iframe should be special here.

The reasoning behind it is that the top level document should have visibility to iframe loads it initiated (or that were initiated in its context), but should not have visibility into iframe loads initiated by the iframe's content. I'm open to other ways to define the above.

/cc @bzbarsky

Oh, that seems somewhat of a mismatch with how fetch groups should work. Also, what if I set contentWindow.location, I guess that doesn't count?

Right, the intent is to exclude navigations due to location sets, link clicks in the iframe, etc, etc.

I'm not 100% sure how fetch groups should relate to this. Which fetch group is the load for an <iframe src="stuff"> supposed to be in?

@bzbarsky my rough idea was that each global (client) has its own fetch group and since iframe always creates a global it'd have its own (embed and object are weird here and would require some special casing of sorts; my idea was that if they end up creating a global we'd move the fetches).

Yeah, I think the way resource timing does its tracking is somewhat unfortunate as we cannot hang it on this concept. We end up with another table in already complex (and undefined) landscape. Basically requires more special code paths to be carefully vetted each time we touch things.

I think we should be able to hang resource timing on the client of the Request. Would that work?

I don't think so. With <iframe src=...> the about:blank global is the client (and with which the service worker is associated).

With <iframe src=...> the about:blank global is the client

In this case we start with https://html.spec.whatwg.org/multipage/iframe-embed-object.html#process-the-iframe-attributes then go to https://html.spec.whatwg.org/multipage/iframe-embed-object.html#otherwise-steps-for-iframe-or-frame-elements which does a https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigate and says:

Any navigation required of the user agent in the process the iframe attributes algorithm must use the iframe element's node document's browsing context as the source browsing context.

When we land in the navigate algorithm, we end up in https://html.spec.whatwg.org/multipage/browsing-the-web.html#process-a-navigate-fetch which does:

Set request's client to sourceBrowsingContext's active document's relevant settings object

How does the client end up as that about:blank global? What am I missing?

Note that the "reserved client" bits are separate here, of course.

Okay, I'm a little skeptical this is all agreed upon across implementations, but maybe this is all mostly correct and then it would probably work, indeed.

bzbarsky · 2018-11-02T13:51:09Z

index.html

-the current <a data-cite="HTML#browsing-context">browsing</a> or
-worker [[WORKERS]] context's MUST be included as
+<p>All resource <a data-cite="Fetch#concept-request">Request</a>s
+<a data-cite="FETCH#concept-fetch">fetched</a> by the current


What is the "current browsing context"? That's a longstanding issue here... The PerformanceResourceTiming is tied to a global and hence settings object. That's the settings object that should presumably be relevant here?

One option might be that given a fetch we get its client, if not null get the PerformanceResourceTiming for that settings object's global (need to word this carefully around workers) and use that. I think that will do the right thing for iframes, because loads due to attr changes use the iframe's nodee document's window as the client, which is what we want here.

Doing this on the browsing context level, if actually implemented that way (the way the spec is trying to define it here), would be a security hole: a PerformanceResourceTiming from an earlier page in the history would be required to see loads from later pages in the history. That's clearly not desirable.

bzbarsky · 2018-11-02T13:56:02Z

index.html

+"document", "object" or "embed", and was not triggered by 
+<a data-cite="HTML#process-the-iframe-attributes">process the iframe attributes</a>,
+<a data-cite="HTML#process-the-object-attributes">process the object attributes</a>, or
+<a data-cite="HTML#process-the-embed-attributes">process the embed attributes</a>, 


Right, the intent is to exclude navigations due to location sets, link clicks in the iframe, etc, etc.

I'm not 100% sure how fetch groups should relate to this. Which fetch group is the load for an <iframe src="stuff"> supposed to be in?

bzbarsky · 2018-11-02T13:57:47Z

index.html

@@ -934,6 +943,13 @@ <h3>Processing Model</h3>
 surface resource fetch events.</p>
 <div class="issue" data-number="27"></div>
 <ol data-link-for="PerformanceResourceTiming">
+<li>If the resource's <a data-cite="Fetch#concept-request">Request</a>'s
+<a data-cite="Fetch#concept-request-destination">destination</a> equals to
+"document", "object" or "embed", and was not triggered by 


If the destination is "object" or "embed", then the request definitely comes from the attribute-processing algorithms. Those are the only things that set destination to "object" or "embed". So we only need to special-case the "document" case here, I think.

yoavweiss · 2018-11-05T17:48:06Z

@bzbarsky @annevk - Rewritten based on discussions. PTAL?

bzbarsky · 2018-11-05T17:55:52Z

index.html

-stylesheets fetched with <code>no-cors</code> policy, perform the
-following steps:</p>
+
+<p>For each resource which <a>Request</a> has a non-null <a>client</a>, perform


s/which/whose/?

bzbarsky · 2018-11-05T17:56:47Z

index.html

-<ol data-link-for="PerformanceResourceTiming">
+<li>If the resource's <a>Request</a>'s
+<a data-cite="Fetch#concept-request-destination">destination</a> equals to
+"document", and was not triggered by 


"and Request was not triggered", right? The current wording is talking about the destination being triggered...

yoavweiss · 2018-11-06T08:24:45Z

@bzbarsky - any other outstanding issues?

bzbarsky · 2018-11-06T18:23:46Z

Nope. That's why I approved the changes. ;)

Thank you for fixing this!

yoavweiss · 2018-11-06T19:22:24Z

Thanks for reviewing! :)

Better define which entries are exposed for nested contexts

d4e8db3

yoavweiss requested a review from annevk October 31, 2018 16:51

annevk reviewed Nov 2, 2018

View reviewed changes

bzbarsky suggested changes Nov 2, 2018

View reviewed changes

Simplified processing based on PR discussions

08db21c

bzbarsky approved these changes Nov 5, 2018

View reviewed changes

Review comments

f55e6f9

yoavweiss merged commit e292d20 into w3c:gh-pages Nov 6, 2018

yoavweiss deleted the nested_contexts branch November 6, 2018 19:22

yoavweiss mentioned this pull request Nov 29, 2018

Clarify the behavior of the SW timeline #140

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better define which entries are exposed for nested contexts #177

Better define which entries are exposed for nested contexts #177

yoavweiss commented Oct 31, 2018

yoavweiss commented Oct 31, 2018

yoavweiss commented Oct 31, 2018

annevk Nov 2, 2018

yoavweiss Nov 2, 2018

annevk Nov 2, 2018

yoavweiss Nov 5, 2018

bzbarsky Nov 5, 2018

bzbarsky Nov 5, 2018

yoavweiss Nov 5, 2018 •

edited

Loading

bzbarsky Nov 5, 2018

yoavweiss Nov 5, 2018

bzbarsky Nov 5, 2018

annevk Nov 2, 2018

yoavweiss Nov 2, 2018

annevk Nov 2, 2018

bzbarsky Nov 2, 2018

annevk Nov 2, 2018

annevk Nov 2, 2018

bzbarsky Nov 2, 2018

annevk Nov 5, 2018

bzbarsky Nov 5, 2018 •

edited

Loading

annevk Nov 5, 2018

bzbarsky Nov 2, 2018

bzbarsky Nov 2, 2018

bzbarsky Nov 2, 2018

yoavweiss commented Nov 5, 2018

bzbarsky Nov 5, 2018

yoavweiss Nov 5, 2018

bzbarsky Nov 5, 2018

yoavweiss Nov 5, 2018

yoavweiss commented Nov 6, 2018

bzbarsky commented Nov 6, 2018

yoavweiss commented Nov 6, 2018

Better define which entries are exposed for nested contexts #177

Better define which entries are exposed for nested contexts #177

Conversation

yoavweiss commented Oct 31, 2018

yoavweiss commented Oct 31, 2018

yoavweiss commented Oct 31, 2018

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yoavweiss Nov 5, 2018 • edited Loading

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

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

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

bzbarsky Nov 5, 2018 • edited Loading

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

Choose a reason for hiding this comment

yoavweiss commented Nov 5, 2018

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

yoavweiss commented Nov 6, 2018

bzbarsky commented Nov 6, 2018

yoavweiss commented Nov 6, 2018

yoavweiss Nov 5, 2018 •

edited

Loading

bzbarsky Nov 5, 2018 •

edited

Loading