At this moment the DNS prefetching is handled by a prefetchDNS function, which receives the host name which may be resolved. libsoup (HTTP library used by WebKit GTK) has a different approach: a function called soup_session_prepare_for_uri which takes the full URL as an argument, at now it only does DNS prefetching, but it will soon also have proxy resolution and prefetching (https://bugzilla.gnome.org/show_bug.cgi?id=605065 ). By having the full URL as an argument, the function can be expanded in the future (cache, etc.). However, as WebKit DNS prefetching function does only have the host name as a parameter, a fake URI (http://hostname) is sent to soup_session_prepare_for_uri, so the function cannot use the information given by the rest of the URI.
Created attachment 49821 [details] ResourceHandle::prepareForURL This patch adds a new static method to ResourceHandle, prepareForURL, which can be invoked when a URL may be requested. The default implementation of the method executes prefetchDNS. Soup prepareForURL implementation uses soup_session_prepare_for_uri instead. Other ports may also implement their own version of prepareForURL.
Attachment 49821 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/network/soup/ResourceHandleSoup.cpp:152: Use 0 instead of NULL. [readability/null] [4] WebCore/platform/network/ResourceHandle.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 49821 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/321956
Comment on attachment 49821 [details] ResourceHandle::prepareForURL r- since the EWS complains about breakage.
Created attachment 50412 [details] ResourceHandle::prepareForURL New version of the patch. It adds an implementation of ResourceHandle::prepareForURL in platform/network/chromium/DNSChromium.cpp, as Chromium does not use ResourceHandler.cpp
Attachment 50412 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/network/soup/ResourceHandleSoup.cpp:153: Use 0 instead of NULL. [readability/null] [4] WebCore/platform/network/ResourceHandle.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 50412 [details] ResourceHandle::prepareForURL Please fix the style nits. + KURL url(ParsedURLString, "http://"+hostname); Are you sure you want to do this? prefetchDNS is pretty performance sensitive.
Created attachment 50704 [details] ResourceHandle::prepareForURL New version of the patch > Please fix the style nits. The problem caused by using NULL in a comment was fixed, but the problem caused by the alphabetical order in the include statements is there because both ResourceHandle.h and ResourceHandleInternal.h are included as primary headers.
Attachment 50704 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/platform/network/ResourceHandle.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
> If any of these errors are false positives, please file a bug against > check-webkit-style. Ok. Sounds we like should file a bug against check-webkit-style then. What about the performance question?
Comment on attachment 50704 [details] ResourceHandle::prepareForURL Looks like you solved that problem too. I don't know what soup_session_prepare_for_uri does, but you might want to make sure you're not generating a billion network requests.
(In reply to comment #10) > > If any of these errors are false positives, please file a bug against > > check-webkit-style. > > Ok. Sounds we like should file a bug against check-webkit-style then. The coding style guidelines do not consider that a file can have something like #include "ResourceHandleInternal.h", so I'm not sure that opening a bug against check-webkit-style is the best possible solution. > What about the performance question? This version does not change the previous soup implementation of prefetchDNS. However, the objective of this patch is to make ResourceHandle::prepareForURL the suggested way of prefetching DNS resolution.
> The coding style guidelines do not consider that a file can have something like > #include "ResourceHandleInternal.h", so I'm not sure that opening a bug against > check-webkit-style is the best possible solution. The goal for check-webkit-style is to have zero errors in our codebase. That means for every error, we need to either change the codebase or change check-webkit-style. In this case, it looks like check-webkit-style isn't smart enough to understand that ResourceHandle.h and ResourceHandleInternal.h are both primary headers.
(In reply to comment #11) > (From update of attachment 50704 [details]) > Looks like you solved that problem too. I don't know what > soup_session_prepare_for_uri does, but you might want to make sure you're not > generating a billion network requests. soup_session_prepare_for_uri now only does prefetch DNS, but future implementations may handle other network aspects, like proxy resolution (https://bugzilla.gnome.org/show_bug.cgi?id=605065) libsoup handles the network requests to avoid prefetching a domain which was previously resolved.
Great! :)
Created attachment 50723 [details] ResourceHandle::prepareForURL New version of the patch, this one removes Soup implementation of prefetchDNS, as it's now handled by ResourceHandle::prepareForURL,
Attachment 50723 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WARNING: Could not read file. Skipping: "WebCore/platform/network/soup/DNSSoup.cpp" WebCore/platform/network/ResourceHandle.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 50723 [details] ResourceHandle::prepareForURL Ok. + #ifdef HAVE_LIBSOUP_2_29_90 Shouldn't this not be indented?
Comment on attachment 50704 [details] ResourceHandle::prepareForURL Cleared Adam Barth's review+ from obsolete attachment 50704 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Created attachment 50801 [details] ResourceHandle::prepareForURL New version of the patch. This time preprocessor directives are not indented.
Attachment 50801 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WARNING: Could not read file. Skipping: "WebCore/platform/network/soup/DNSSoup.cpp" WebCore/platform/network/ResourceHandle.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 50801 [details] ResourceHandle::prepareForURL + #if !USE(SOUP) Is still indented...
Created attachment 50807 [details] ResourceHandle::prepareForURL Let's hope this time is OK.
Attachment 50807 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WARNING: Could not read file. Skipping: "WebCore/platform/network/soup/DNSSoup.cpp" WebCore/platform/network/ResourceHandle.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 50807 [details] ResourceHandle::prepareForURL Yay! Thanks for fixing the nits.
Comment on attachment 50807 [details] ResourceHandle::prepareForURL Clearing flags on attachment: 50807 Committed r56128: <http://trac.webkit.org/changeset/56128>
All reviewed patches have been landed. Closing bug.
*** Bug 32792 has been marked as a duplicate of this bug. ***