[Webkit-unassigned] [Bug 42432] [GTK] Enable Web Timing

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=42432





--- Comment #16 from Zan Dobersek <zandobersek at gmail.com>  2011-06-21 05:21:27 PST ---
(In reply to comment #15)
> Created an attachment (id=97962)
 --> (https://bugs.webkit.org/attachment.cgi?id=97962&action=review) [details]
> Updated patch w/ a new approach to connection times
> 
> This is an updated patch that handles the mentioned connection problems in a new way.

In this post I'll describe changes in a more detailed way along with replies to other comments.

This is more of a work-in-progress patch, just to present new methods that are used.

For starters I've removed compilation guards - ResourceLoadTiming object is also used by the Web Inspector so there might even be some progression on that part. The ResourceLoadTiming object itself is not guarded by any compilation guards and neither should be any use of it. PerformanceTiming object is simply using ResourceLoadTiming to provide needed information so this is more implementation of ResourceLoadTiming usage than implementation of Web Timing specification itself.

Also this patch updates ResourceLoadTiming only for HTTP requests as it seems useless to time events in almost-instant local file loading. But I'm not sure about that as I write, so I should look into it.

(In reply to comment #14)
> (From update of attachment 95285 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=95285&action=review
> 
> I'm still concerned about Sergio's comment. Seems that this question should be resolved before we can land this:
> 
> > This seems correct but the problem is that you are handling the "connection-created" signal of any request issued to the SoupSession. Take into account that this callback will be called whenever the SoupSession needs to create a connection (and not only for this particular ResourceHandle). There are some other problems too, see below in the g_signal_connect() part
> 

In the updated patch every time a new HTTP request is started (in startHTTPRequest) the ResourceResponse is told that the connection will be reused. This flag can be set to false though if the connection-created signal is triggered on the SoupSession object. This flag is then checked in the PerformanceTiming object whenever connectStart or connectEnd attributes are requested - if true, the time of previous event is returned, otherwise the times in ResourceLoadTiming are used.

The only problem is that there's no appropriate signals in libsoup to connect to to acquire connectStart and connectEnd times. Because of that I've added two corresponding signals to the SoupMessage object. I'll attach a patch here just for the sake of convenience but this should really be discussed with libsoup devs elsewhere, but I'd like to hear some thoughts about this approach.

> > Source/WebCore/GNUmakefile.am:583
> > +if ENABLE_WEB_TIMING
> > +FEATURE_DEFINES += ENABLE_WEB_TIMING=1
> > +webcore_cppflags += -DENABLE_WEB_TIMING=1
> > +endif  # END ENABLE_WEB_TIMING
> 
> What affect does this have on runtime performance. Do other ports enable it by default or only to run tests?
> 

I don't think this would have any significant impact on performance, but there was some debate on the mailing list about how users with this feature enabled could be under threat of fingerprinting and privacy breach[1]. Chromium port enables this by default but I believe the best way for our port would be to have this feature disabled in configure.ac and enabled in build-webkit script for testing purposes.

> > Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:416
> > +    g_signal_handlers_disconnect_matched(handle->defaultSession(), G_SIGNAL_MATCH_DATA,
> > +                                         0, 0, 0, 0, handle);
> 
> This can be one line.

Fixed.

> 

All other comments about layout tests are noted. These issues will be addressed as soon as we get the connections problems resolved. Also there are some evident bugs that I'd like to address before landing the implementation (mostly about redirects and how they're handled).

[0] https://lists.webkit.org/pipermail/webkit-dev/2011-May/016803.html

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.