Bug 85025 - [Resource Timing] Report redirect time for resources
Summary: [Resource Timing] Report redirect time for resources
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 103867
Blocks: 61152
  Show dependency treegraph
 
Reported: 2012-04-26 19:24 PDT by James Simonsen
Modified: 2014-02-05 10:59 PST (History)
10 users (show)

See Also:


Attachments
Patch (15.42 KB, patch)
2012-07-31 07:37 PDT, pdeng6
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Simonsen 2012-04-26 19:24:42 PDT
Redirect time is always 0. This should report the correct values.

We had to add some code to MainResourceLoader to track redirects while fetching the main resource for Navigation Timing. We'll have to do the same for ResourceLoader.

We also need to ensure that the redirect times reported honor the same-origin policy noted in the spec. They should be zeroed out if there are any cross-origin redirects.
Comment 1 pdeng6 2012-07-31 07:37:13 PDT
Created attachment 155530 [details]
Patch
Comment 2 Gyuyoung Kim 2012-07-31 07:44:24 PDT
Comment on attachment 155530 [details]
Patch

Attachment 155530 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13403191
Comment 3 WebKit Review Bot 2012-07-31 07:44:31 PDT
Comment on attachment 155530 [details]
Patch

Attachment 155530 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13384980
Comment 4 Build Bot 2012-07-31 07:50:50 PDT
Comment on attachment 155530 [details]
Patch

Attachment 155530 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13389698
Comment 5 Early Warning System Bot 2012-07-31 07:52:20 PDT
Comment on attachment 155530 [details]
Patch

Attachment 155530 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13401400
Comment 6 Build Bot 2012-07-31 07:53:10 PDT
Comment on attachment 155530 [details]
Patch

Attachment 155530 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13401397
Comment 7 Early Warning System Bot 2012-07-31 08:11:40 PDT
Comment on attachment 155530 [details]
Patch

Attachment 155530 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13392742
Comment 8 James Simonsen 2012-07-31 11:31:03 PDT
You'll want to base this on the patch in https://bugs.webkit.org/show_bug.cgi?id=84883

We don't want to populate the buffer from the loaders, because they don't have access to the initiator. So we're implementing this a level up.

Also, we need to support the Timing-Allow-Origin header. And test cases.
Comment 9 pdeng6 2012-08-01 06:01:07 PDT
(In reply to comment #8)
> You'll want to base this on the patch in https://bugs.webkit.org/show_bug.cgi?id=84883
> 
> We don't want to populate the buffer from the loaders, because they don't have access to the initiator. So we're implementing this a level up.
> 
> Also, we need to support the Timing-Allow-Origin header. And test cases.

Thanks for quick response; I can help Timing-Allow-Origin issue and test cases:)

About populating performance entries from kinds of CachedResourceClient, I feel it will take more engineering effort and a little confused, another issue is several entries will be populated when one time fetched resource is feed to several CachedResourceClients(W3C spec recommends one entry).

Is it possible we expose initiator informantion by virtual functions from CachedResourceClient => CachedResource => SubResourceLoader? Then we can have centralized control of timing info, redirect etc in loader.
Comment 10 James Simonsen 2012-08-01 20:27:57 PDT
(In reply to comment #9)
> About populating performance entries from kinds of CachedResourceClient, I feel it will take more engineering effort and a little confused, another issue is several entries will be populated when one time fetched resource is feed to several CachedResourceClients(W3C spec recommends one entry).

That's a good point.

> Is it possible we expose initiator informantion by virtual functions from CachedResourceClient => CachedResource => SubResourceLoader? Then we can have centralized control of timing info, redirect etc in loader.

I'm worried about the case where the first element is removed from the document, but another element requests the same resource. According to the spec, the original element should still be reported as the initiator. In the above design, we can't rely on the original element still being a client.

One option would be to pass the initiator in to the CachedResource and the first one to construct the object in the MemoryCache becomes the initiator. However, I got the sense that this was not desirable, because of layering. We don't want a lot of DOM knowledge in loader/. Maybe this one is okay though?

Another alternative would be to use more of an observer design, like Inspector. We could just note when an element requests a URL and when the loaders make a request, redirect, and finish. Resource Timing would keep track of the state on its end. That also has the benefit of not needing an extra parameter everywhere CachedResource is used.

Other ideas?
Comment 11 Adam Barth 2012-08-02 08:02:52 PDT
I'm going to defer to Nate on these questions.
Comment 12 pdeng6 2012-09-06 07:53:21 PDT
@Nate, how do you think about this?
Comment 13 Anders Carlsson 2014-02-05 10:59:06 PST
Comment on attachment 155530 [details]
Patch

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.