Bug 99695 - [GTK] Add API to prefetch DNS of a given hostname to WebKit2 GTK+ API
Summary: [GTK] Add API to prefetch DNS of a given hostname to WebKit2 GTK+ API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2012-10-18 01:01 PDT by Carlos Garcia Campos
Modified: 2013-01-29 05:23 PST (History)
12 users (show)

See Also:


Attachments
Patch (6.38 KB, patch)
2012-10-18 01:05 PDT, Carlos Garcia Campos
andersca: review-
buildbot: commit-queue-
Details | Formatted Diff | Diff
Update patch to make it specific to GTK port (6.46 KB, patch)
2012-10-23 10:26 PDT, Carlos Garcia Campos
andersca: review-
Details | Formatted Diff | Diff
New patch using injected bundle (6.26 KB, patch)
2013-01-10 05:33 PST, Carlos Garcia Campos
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2012-10-18 01:01:44 PDT
To be able to use soup_session_prefetch_dns() in WebKit2
Comment 1 Carlos Garcia Campos 2012-10-18 01:05:27 PDT
Created attachment 169365 [details]
Patch
Comment 2 WebKit Review Bot 2012-10-18 01:08:07 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 3 Build Bot 2012-10-18 01:22:08 PDT
Comment on attachment 169365 [details]
Patch

Attachment 169365 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14391939
Comment 4 Carlos Garcia Campos 2012-10-18 09:34:52 PDT
Any idea how to fix the mac build? it seems WebCore/DNS.h is not found, but I don't see why DNS.h is different than other WebCore headers.
Comment 5 Anders Carlsson 2012-10-18 11:52:21 PDT
Comment on attachment 169365 [details]
Patch

I don't think this is worth it - prefetching DNS is such a small part of the load.
Comment 6 Adam Barth 2012-10-18 15:51:05 PDT
(In reply to comment #4)
> Any idea how to fix the mac build? it seems WebCore/DNS.h is not found, but I don't see why DNS.h is different than other WebCore headers.

The header might not be marked as "Private" in the Xcode project.  Private means that it can be used by other projects (like WebKit2), unlikely the default value of "Project".
Comment 7 Carlos Garcia Campos 2012-10-18 23:17:56 PDT
(In reply to comment #5)
> (From update of attachment 169365 [details])
> I don't think this is worth it - prefetching DNS is such a small part of the load.

WebKitGTK+ users do this by using the SoupSession directly in WebKit1, for example, Epiphany calls soup_session_prefetch_dns for every hostname in the recent history. This is not possible with WebKit2 because we don't have access to the SoupSession from the UI process. This is something that will only be used on demand, so I think it's harmless. But still, if other ports are not interested, would the same patch be acceptable by adding #if PLATFORM(GTK) everywhere?
Comment 8 Carlos Garcia Campos 2012-10-23 10:26:26 PDT
Created attachment 170187 [details]
Update patch to make it specific to GTK port
Comment 9 Anders Carlsson 2012-10-23 12:48:10 PDT
Comment on attachment 170187 [details]
Update patch to make it specific to GTK port

Still not convinced. Is there a reason why this couldn't be handled all in epiphany by using an injected bundle? Have you measured that prefetching the DNS actually is an improvement?
Comment 10 Carlos Garcia Campos 2012-10-23 23:35:30 PDT
(In reply to comment #9)
> (From update of attachment 170187 [details])
> Still not convinced. Is there a reason why this couldn't be handled all in epiphany by using an injected bundle? Have you measured that prefetching the DNS actually is an improvement?

I don't know, I'm just porting the existing code and need a replacement for soup_session_prefetch_dns(), Xan or Dan should know better than me. And regarding injected bundle, we don't support it yet in our GTK+ API, we are still figuring out how to use and expose it in our APIs.
Comment 11 Carlos Garcia Campos 2013-01-10 05:33:36 PST
Created attachment 182119 [details]
New patch using injected bundle
Comment 12 Sergio Villar Senin 2013-01-18 07:16:30 PST
Comment on attachment 182119 [details]
New patch using injected bundle

LGTM
Comment 13 Martin Robinson 2013-01-18 08:20:52 PST
Looks good to me as well. I guess we need a review from a WebKit2 owner.
Comment 14 Carlos Garcia Campos 2013-01-29 05:23:52 PST
Committed r141102: <http://trac.webkit.org/changeset/141102>