Summary: | [GTK] Add API to prefetch DNS of a given hostname to WebKit2 GTK+ API | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, andersca, danw, gustavo, gyuyoung.kim, joone, mrobinson, rakuco, sam, svillar, webkit.review.bot, xan.lopez | ||||||||
Priority: | P2 | Keywords: | Gtk | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2012-10-18 01:01:44 PDT
Created attachment 169365 [details]
Patch
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 on attachment 169365 [details] Patch Attachment 169365 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14391939 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 on attachment 169365 [details]
Patch
I don't think this is worth it - prefetching DNS is such a small part of the load.
(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". (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? Created attachment 170187 [details]
Update patch to make it specific to GTK port
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?
(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. Created attachment 182119 [details]
New patch using injected bundle
Comment on attachment 182119 [details]
New patch using injected bundle
LGTM
Looks good to me as well. I guess we need a review from a WebKit2 owner. Committed r141102: <http://trac.webkit.org/changeset/141102> |