Bug 103927

Summary: [Resource Timing] Expose timing information for iframes
Product: WebKit Reporter: James Simonsen <simonjam>
Component: PlatformAssignee: James Simonsen <simonjam>
Status: RESOLVED FIXED    
Severity: Normal CC: japhet, pan.deng, syoichi, tonyg, webkit.review.bot, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 49246    
Bug Blocks: 84883    
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description James Simonsen 2012-12-03 14:26:57 PST
Since Resource Timing hooks into CachedResourceLoader, we need to wait for main documents to go through it before they'll appear in the Resource Timing buffer.
Comment 1 James Simonsen 2013-03-29 15:59:24 PDT
Created attachment 195812 [details]
Patch
Comment 2 Tony Gentilcore 2013-03-29 16:04:50 PDT
Comment on attachment 195812 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195812&action=review

> LayoutTests/http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_iframe_self_navigation.html:1
> +<!DOCTYPE html>

Where are the expectations for this new test?
Comment 3 James Simonsen 2013-03-29 16:14:34 PDT
Created attachment 195815 [details]
Patch
Comment 4 James Simonsen 2013-03-29 16:15:05 PDT
(In reply to comment #2)
> (From update of attachment 195812 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195812&action=review
> 
> > LayoutTests/http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_iframe_self_navigation.html:1
> > +<!DOCTYPE html>
> 
> Where are the expectations for this new test?

Oops. Forgot to 'git add'. Fixed now.
Comment 5 Tony Gentilcore 2013-03-29 17:14:36 PDT
Comment on attachment 195815 [details]
Patch

lgtm but Nate should review the CachedResourceLoader changes
Comment 6 Nate Chapin 2013-04-01 09:46:47 PDT
Comment on attachment 195815 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195815&action=review

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:535
> +        if (frame()->ownerElement() && m_documentLoader->frameLoader()->stateMachine()->committingFirstRealLoad()) {
> +            InitiatorInfo info = { frame()->ownerElement()->localName(), monotonicallyIncreasingTime() };
> +            m_initiatorMap.add(newResource.get(), info);
> +        }

This probably needs a comment, maybe a link to the spec?
Comment 7 James Simonsen 2013-04-01 16:39:12 PDT
Created attachment 196037 [details]
Patch for landing
Comment 8 James Simonsen 2013-04-01 16:40:04 PDT
(In reply to comment #6)
> (From update of attachment 195815 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195815&action=review
> 
> > Source/WebCore/loader/cache/CachedResourceLoader.cpp:535
> > +        if (frame()->ownerElement() && m_documentLoader->frameLoader()->stateMachine()->committingFirstRealLoad()) {
> > +            InitiatorInfo info = { frame()->ownerElement()->localName(), monotonicallyIncreasingTime() };
> > +            m_initiatorMap.add(newResource.get(), info);
> > +        }
> 
> This probably needs a comment, maybe a link to the spec?

Done.
Comment 9 WebKit Review Bot 2013-04-01 22:50:06 PDT
Comment on attachment 196037 [details]
Patch for landing

Clearing flags on attachment: 196037

Committed r147387: <http://trac.webkit.org/changeset/147387>
Comment 10 WebKit Review Bot 2013-04-01 22:50:10 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Zan Dobersek 2013-04-02 05:42:52 PDT
After the patch landed the following test started failing persistently on Chromium bots:
http/tests/w3c/webperf/submission/Google/resource-timing/html/test_resource_cached.html

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=http%2Ftests%2Fw3c%2Fwebperf%2Fsubmission%2FGoogle%2Fresource-timing%2Fhtml%2Ftest_resource_cached.html

It is also failing on the GTK port (with another test), covered in bug #113772.