[webkit-reviews] review denied: [Bug 32792] [Gtk] Make WebKitGtk give full URI to soup_session_prepare_for_uri (patch attached) : [Attachment 45281] Patch for making soup_session_prepare_for_uri receive full URI

[bugzilla-daemon at webkit.org]

Gustavo Noronha (kov) <gns at gnome.org> has denied José Millán Soto
<jmillan at igalia.com>'s request for review:
Bug 32792: [Gtk] Make WebKitGtk give full URI to soup_session_prepare_for_uri
(patch attached)
https://bugs.webkit.org/show_bug.cgi?id=32792

Attachment 45281: Patch for making soup_session_prepare_for_uri receive full
URI
https://bugs.webkit.org/attachment.cgi?id=45281&action=review

------- Additional Comments from Gustavo Noronha (kov) <gns at gnome.org>
> Index: WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp
> ===================================================================
> --- WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp	(revision 52416)
> +++ WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp	(working copy)
> @@ -462,6 +462,7 @@ void ChromeClient::mouseDidMoveOverEleme
>      bool isLink = hit.isLiveLink();
>      if (isLink) {
>	   KURL url = hit.absoluteLinkURL();
> +	   prepareForUri(url);
>	   if (!url.isEmpty() && url != m_hoveredLinkURL) {
>	       TextDirection dir;
>	       CString titleString = hit.title(dir).utf8();

I don't think this is a good solution. If WebCore decides to prefetch DNS in
additional situations we'll miss out. I recommend changing the prefetchDNS
function to take the full URL, and change all other ports implementations to
use the host alone. Should not be too much work, and is much cleaner:

kov at goiaba ~/s/W/WebCore> ack-grep prefetchDNS 
page/Chrome.cpp
330:		prefetchDNS(result.absoluteLinkURL().host());

platform/android/TemporaryLinkStubs.cpp
479:void prefetchDNS(const String&)

platform/network/soup/DNSSoup.cpp
35:void prefetchDNS(const String& hostname)

platform/network/cf/DNSCFNet.cpp
147:void prefetchDNS(const String& hostname)

platform/network/DNS.h
33:    void prefetchDNS(const String& hostname);

platform/network/qt/DnsPrefetchHelper.cpp
27:void prefetchDNS(const String& hostname)

platform/network/chromium/DNSChromium.cpp
33:void prefetchDNS(const String& hostname)
35:    ChromiumBridge::prefetchDNS(hostname);

platform/network/curl/DNSCurl.cpp
33:void prefetchDNS(const String& hostname)

platform/chromium/ChromiumBridge.h
93:	   static void prefetchDNS(const String& hostname);

html/HTMLLinkElement.cpp
186:	    prefetchDNS(m_url.host());

html/HTMLAnchorElement.cpp
282:			prefetchDNS(document()->completeURL(parsedURL).host());


wml/WMLAElement.cpp
64:		   prefetchDNS(document()->completeURL(value).host());