Bug 127555 - [curl] Add WEB_TIMING support
Summary: [curl] Add WEB_TIMING support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 117300
  Show dependency treegraph
 
Reported: 2014-01-24 06:07 PST by sipka
Modified: 2014-04-16 12:54 PDT (History)
4 users (show)

See Also:


Attachments
proposed patch (6.37 KB, patch)
2014-01-24 06:12 PST, sipka
no flags Details | Formatted Diff | Diff
proposed patch (6.45 KB, patch)
2014-01-24 06:30 PST, sipka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description sipka 2014-01-24 06:07:42 PST
Add web_timing informations.
Comment 1 sipka 2014-01-24 06:12:49 PST
Created attachment 222094 [details]
proposed patch

Access timing information related to navigation.
Comment 2 sipka 2014-01-24 06:30:12 PST
Created attachment 222106 [details]
proposed patch
Comment 3 Brent Fulgham 2014-01-30 10:02:37 PST
Comment on attachment 222106 [details]
proposed patch

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

Looks good, but I had a question about the removal of a "didReceiveResponse" call. Also, you accidentally introduced a blank line.

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:-468
> -            client->didReceiveResponse(job, d->m_response);

Why don't we need this call to didReceiveResponse anymore? Does CurlCacheManager do this work as well?

> Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:657
> +

Extra blank line!
Comment 4 Mátyás Mustoha 2014-01-31 04:01:04 PST
(In reply to comment #3)
> Why don't we need this call to didReceiveResponse anymore? Does CurlCacheManager do this work as well?

No it doesn't, only the function name is similar.
Comment 5 sipka 2014-01-31 05:30:40 PST
(In reply to comment #3)
> (From update of attachment 222106 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=222106&action=review
> 
> Looks good, but I had a question about the removal of a "didReceiveResponse" call. Also, you accidentally introduced a blank line.
> 
> > Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:-468
> > -            client->didReceiveResponse(job, d->m_response);
> 
> Why don't we need this call to didReceiveResponse anymore? Does CurlCacheManager do this work as well?
> 
> > Source/WebCore/platform/network/curl/ResourceHandleManager.cpp:657
> > +
> 
> Extra blank line!

Instead of calling didReceiveResponse from the headerCallback function, I call it from downloadTimerCallback (and from dispatchSynchronousJob in synchronous case) to make sure that the webtiming informations will be retrieved and set properly.
Comment 6 Brent Fulgham 2014-04-16 12:23:00 PDT
Comment on attachment 222106 [details]
proposed patch

r=me
Comment 7 WebKit Commit Bot 2014-04-16 12:54:30 PDT
Comment on attachment 222106 [details]
proposed patch

Clearing flags on attachment: 222106

Committed r167377: <http://trac.webkit.org/changeset/167377>
Comment 8 WebKit Commit Bot 2014-04-16 12:54:33 PDT
All reviewed patches have been landed.  Closing bug.