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 tree / graph
 
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
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

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.