Bug 118395

Summary: [Soup] WebTiming information not shown in the inspector
Product: WebKit Reporter: Sergio Villar Senin <svillar>
Component: WebKitGTKAssignee: Sergio Villar Senin <svillar>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, ap, barraclough, beidson, benjamin, cdumez, cgarcia, commit-queue, danw, gns, mrobinson, rakuco, svillar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch mrobinson: review+

Description Sergio Villar Senin 2013-07-04 10:36:43 PDT
Inspector is not showing timing information because the response does not carry the required information due to changes added in r116160. Patch to follow.
Comment 1 Sergio Villar Senin 2013-07-04 11:57:46 PDT
Created attachment 206106 [details]
Patch
Comment 2 Martin Robinson 2013-07-04 12:08:20 PDT
(In reply to comment #0)
> Inspector is not showing timing information because the response does not carry the required information due to changes added in r116160. Patch to follow.

Okay. The soup backend bits look reasonable. I don't know about the WebTiming test.
Comment 3 Sergio Villar Senin 2013-07-04 12:15:24 PDT
(In reply to comment #2)
> (In reply to comment #0)
> > Inspector is not showing timing information because the response does not carry the required information due to changes added in r116160. Patch to follow.
> 
> Okay. The soup backend bits look reasonable. I don't know about the WebTiming test.

BTW I've just checked that the similar fix was added to Blink.
Comment 4 Sergio Villar Senin 2013-07-18 06:22:07 PDT
(In reply to comment #2)
> (In reply to comment #0)
> > Inspector is not showing timing information because the response does not carry the required information due to changes added in r116160. Patch to follow.
> 
> Okay. The soup backend bits look reasonable. I don't know about the WebTiming test.

ap, andersca: any thoughts?
Comment 5 Sergio Villar Senin 2013-08-12 08:49:18 PDT
Adding Brady who might have an opinion on the matter. The fix is already pre-reviewed, just need someone to double-check that the test changes are OK.
Comment 6 Sergio Villar Senin 2013-08-27 07:49:50 PDT
Ping reviewers
Comment 7 Martin Robinson 2013-08-27 08:46:06 PDT
Comment on attachment 206106 [details]
Patch

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

> LayoutTests/http/tests/misc/webtiming-ssl-expected.txt:7
> -FAIL timing.secureConnectionStart should be >= timing.connectStart. Was 0 (of type number).
> +PASS timing.secureConnectionStart is >= timing.connectStart

Okay. This looks okay to me, but won't this start failing on other ports now? If not, then do they have cascading expectations to cover this? If they do, doesn't it make sense to remove those as well?
Comment 8 Sergio Villar Senin 2013-08-27 09:02:28 PDT
(In reply to comment #7)
> (From update of attachment 206106 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=206106&action=review
> 
> > LayoutTests/http/tests/misc/webtiming-ssl-expected.txt:7
> > -FAIL timing.secureConnectionStart should be >= timing.connectStart. Was 0 (of type number).
> > +PASS timing.secureConnectionStart is >= timing.connectStart
> 
> Okay. This looks okay to me, but won't this start failing on other ports now? If not, then do they have cascading expectations to cover this? If they do, doesn't it make sense to remove those as well?

Mac, Win and Wincairo ports explicitly disable this test, efl/gtk will share this fix, so the only one remaining is Qt which does not support WEB_TIMING AFAIK...
Comment 9 Martin Robinson 2013-08-27 09:03:16 PDT
Comment on attachment 206106 [details]
Patch

Thanks. This has been sitting long enough.
Comment 10 Sergio Villar Senin 2013-08-28 02:28:59 PDT
Committed r154727: <http://trac.webkit.org/changeset/154727>