Bug 103777 - [Resource Timing] Record and report initiator
Summary: [Resource Timing] Record and report initiator
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: James Simonsen
URL:
Keywords:
Depends on:
Blocks: 84883
  Show dependency treegraph
 
Reported: 2012-11-30 14:24 PST by James Simonsen
Modified: 2012-12-03 14:21 PST (History)
8 users (show)

See Also:


Attachments
Patch (6.93 KB, patch)
2012-11-30 14:42 PST, James Simonsen
no flags Details | Formatted Diff | Diff
Patch (8.76 KB, patch)
2012-11-30 15:50 PST, James Simonsen
no flags Details | Formatted Diff | Diff
Patch for landing (28 bytes, patch)
2012-12-03 13:27 PST, James Simonsen
no flags Details | Formatted Diff | Diff
Patch for landing (8.71 KB, patch)
2012-12-03 14:17 PST, James Simonsen
webkit.review.bot: commit-queue-
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-11-30 14:24:39 PST
[Resource Timing] Record and report initiator
Comment 1 James Simonsen 2012-11-30 14:42:51 PST
Created attachment 177029 [details]
Patch
Comment 2 James Simonsen 2012-11-30 14:44:12 PST
Nate, this is the Resource Timing patch I'd discussed with you earlier. I record the timing information in a map on CachedResourceLoader. Let me know what you think.
Comment 3 Nate Chapin 2012-11-30 15:03:12 PST
Comment on attachment 177029 [details]
Patch

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

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:520
> +#if ENABLE(RESOURCE_TIMING)
> +    InitiatorInfo info = { request.initiatorName(), monotonicallyIncreasingTime() };
> +    m_initiatorMap.add(resource.get(), info);
> +#endif

Is it intentional that this won't populate m_initiatorMap for revalidations?

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:709
> +        if (initiatorIt != m_initiatorMap.end()) {

What are the cases where we won't find the resource in the map?

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:712
> +            if (document())
> +                document()->domWindow()->performance()->addResourceTiming(info.name, document(), resource->resourceRequest(), resource->response(), info.startTime, resource->loadFinishTime());

Probably want to ASSERT(document()) here?
Comment 4 James Simonsen 2012-11-30 15:50:22 PST
Created attachment 177038 [details]
Patch
Comment 5 James Simonsen 2012-11-30 15:50:45 PST
(In reply to comment #3)
> (From update of attachment 177029 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177029&action=review
> 
> > Source/WebCore/loader/cache/CachedResourceLoader.cpp:520
> > +#if ENABLE(RESOURCE_TIMING)
> > +    InitiatorInfo info = { request.initiatorName(), monotonicallyIncreasingTime() };
> > +    m_initiatorMap.add(resource.get(), info);
> > +#endif
> 
> Is it intentional that this won't populate m_initiatorMap for revalidations?

Nope. Good catch.

> 
> > Source/WebCore/loader/cache/CachedResourceLoader.cpp:709
> > +        if (initiatorIt != m_initiatorMap.end()) {
> 
> What are the cases where we won't find the resource in the map?

loadDone() is called twice. See the traces below.

> 
> > Source/WebCore/loader/cache/CachedResourceLoader.cpp:712
> > +            if (document())
> > +                document()->domWindow()->performance()->addResourceTiming(info.name, document(), resource->resourceRequest(), resource->response(), info.startTime, resource->loadFinishTime());
> 
> Probably want to ASSERT(document()) here?

Done.



Traces:

(gdb) bt
#0  WebCore::CachedResourceLoader::loadDone (this=0x7fffe367a280, resource=
    0x7fffe2456500)
    at ../../third_party/WebKit/Source/WebCore/loader/cache/CachedResourceLoader
#1  0x00007ffff308502c in WebCore::SubresourceLoader::releaseResources (this=
    0x7fffe2487000)
    at ../../third_party/WebKit/Source/WebCore/loader/SubresourceLoader.cpp:318
#2  0x00007ffff307f8cc in WebCore::ResourceLoader::didFinishLoading (this=
    0x7fffe2487000, finishTime=0)
    at ../../third_party/WebKit/Source/WebCore/loader/ResourceLoader.cpp:314
#3  0x00007ffff3084bdc in WebCore::SubresourceLoader::didFinishLoading (this=
    0x7fffe2487000, finishTime=0)
    at ../../third_party/WebKit/Source/WebCore/loader/SubresourceLoader.cpp:277
#4  0x00007ffff3080045 in WebCore::ResourceLoader::didFinishLoading (this=
    0x7fffe2487000, finishTime=0)
    at ../../third_party/WebKit/Source/WebCore/loader/ResourceLoader.cpp:453
#5  0x00007ffff2a61ee2 in WebCore::ResourceHandleInternal::didFinishLoading (
    this=0x7fffe3678000, finishTime=0)
    at ../../third_party/WebKit/Source/WebCore/platform/network/chromium/Resourc



- and - 


STDERR: 	base::debug::StackTrace::StackTrace() [0x7fc909c8cae4]
STDERR: 	base::debug::(anonymous namespace)::StackDumpSignalHandler() [0x7fc909c8c7e5]
STDERR: 	<unknown> [0x7fc90280d4a0]
STDERR: 	WebCore::CachedResourceLoader::loadDone() [0x7fc90722c2a0]
STDERR: 	WebCore::SubresourceLoader::releaseResources() [0x7fc90720202c]
STDERR: 	WebCore::ResourceLoader::cancel() [0x7fc9071fcd47]
STDERR: 	WebCore::ResourceLoader::cancel() [0x7fc9071fcaf7]
STDERR: 	WebCore::cancelAll() [0x7fc9071ad2c4]
STDERR: 	WebCore::DocumentLoader::stopLoadingSubresources() [0x7fc9071b02f6]
STDERR: 	WebCore::DocumentLoader::stopLoading() [0x7fc9071ae156]
STDERR: 	WebCore::FrameLoader::stopAllLoaders() [0x7fc9071d0c3d]
STDERR: 	WebCore::FrameLoader::continueLoadAfterNavigationPolicy() [0x7fc9071d6194]
STDERR: 	WebCore::FrameLoader::callContinueLoadAfterNavigationPolicy() [0x7fc9071d5a61]
STDERR: 	WebCore::PolicyCallback::call() [0x7fc9071eeed2]
STDERR: 	WebCore::PolicyChecker::continueAfterNavigationPolicy() [0x7fc9071efd4b]
STDERR: 	WebKit::FrameLoaderClientImpl::dispatchDecidePolicyForNavigationAction() [0x7fc905f1b068]
STDERR: 	WebCore::PolicyChecker::checkNavigationPolicy() [0x7fc9071ef756]
STDERR: 	WebCore::FrameLoader::loadWithDocumentLoader() [0x7fc9071cff1c]
STDERR: 	WebCore::FrameLoader::loadWithNavigationAction() [0x7fc9071cf7b5]
STDERR: 	WebCore::FrameLoader::loadURL() [0x7fc9071cee0f]
STDERR: 	WebCore::FrameLoader::loadFrameRequest() [0x7fc9071ce6ed]
STDERR: 	WebCore::FrameLoader::urlSelected() [0x7fc9071cacca]
STDERR: 	WebCore::FrameLoader::urlSelected() [0x7fc9071caa18]
STDERR: 	WebCore::HTMLAnchorElement::handleClick() [0x7fc9073ccb7f]
STDERR: 	WebCore::HTMLAnchorElement::defaultEventHandler() [0x7fc9073caeca]
STDERR: 	WebCore::EventDispatcher::dispatchEventPostProcess() [0x7fc9066179d6]
STDERR: 	WebCore::EventDispatcher::dispatchEvent() [0x7fc906616b70]
STDERR: 	WebCore::EventDispatchMediator::dispatchEvent() [0x7fc906614e6c]
STDERR: 	WebCore::EventDispatcher::dispatchEvent() [0x7fc906615af0]
STDERR: 	WebCore::Node::dispatchEvent() [0x7fc90664ca56]
STDERR: 	WebCore::EventTarget::dispatchEvent() [0x7fc906620cbb]
STDERR: 	WebCore::NodeV8Internal::dispatchEventCallback() [0x7fc9078ae7ef]
STDERR: 	v8::internal::HandleApiCallHelper<>() [0x7fc90b90890a]
STDERR: 	v8::internal::Builtin_Impl_HandleApiCall() [0x7fc90b902fa3]
STDERR: 	v8::internal::Builtin_HandleApiCall() [0x7fc90b902f74]
STDERR: 	<unknown> [0x3e08aaf0654e]
Comment 6 Nate Chapin 2012-12-03 12:59:04 PST
Comment on attachment 177038 [details]
Patch

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

LGTM, just some formatting pedantry:

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:712
> +#if ENABLE(RESOURCE_TIMING)

newlines before and after this #if/#else/#endif block, please.

> Source/WebCore/loader/cache/CachedResourceLoader.cpp:724
> +#endif

This is long enough that an // ENABLE(RESOURCE_TIMING) would make me happy.
Comment 7 James Simonsen 2012-12-03 13:27:23 PST
Created attachment 177315 [details]
Patch for landing
Comment 8 James Simonsen 2012-12-03 13:27:51 PST
(In reply to comment #6)
> (From update of attachment 177038 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177038&action=review
> 
> LGTM, just some formatting pedantry:

Thanks.

> 
> > Source/WebCore/loader/cache/CachedResourceLoader.cpp:712
> > +#if ENABLE(RESOURCE_TIMING)
> 
> newlines before and after this #if/#else/#endif block, please.

Done.

> 
> > Source/WebCore/loader/cache/CachedResourceLoader.cpp:724
> > +#endif
> 
> This is long enough that an // ENABLE(RESOURCE_TIMING) would make me happy.

Done.
Comment 9 WebKit Review Bot 2012-12-03 14:12:40 PST
Comment on attachment 177315 [details]
Patch for landing

Rejecting attachment 177315 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ripts/update-webkit line 152.

Failed to run "['Tools/Scripts/update-webkit', '--chromium', '--force-update']" exit_code: 2
Updating OpenSource
From http://git.chromium.org/external/Webkit
   e9cfa82..99a0591  HEAD       -> origin/HEAD
error: Ref refs/remotes/origin/master is at 99a0591e2f7986bdde68fdbbfa7ba90e4741de91 but expected e9cfa82bccbe6dbf99a5b026e145a5ad84051691
 ! e9cfa82..99a0591  master     -> origin/master  (unable to update local ref)
Died at Tools/Scripts/update-webkit line 152.

Full output: http://queues.webkit.org/results/15100694
Comment 10 James Simonsen 2012-12-03 14:17:38 PST
Created attachment 177328 [details]
Patch for landing
Comment 11 WebKit Review Bot 2012-12-03 14:19:46 PST
Comment on attachment 177328 [details]
Patch for landing

Rejecting attachment 177328 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

/mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/15103668
Comment 12 James Simonsen 2012-12-03 14:21:35 PST
Committed r136439: <http://trac.webkit.org/changeset/136439>