Avoid GURL allocations/copies/frees in RenderFrameImpl::willSendRequest

chrome/renderer/chrome_content_renderer_client.cc

Index: chrome/renderer/chrome_content_renderer_client.cc
diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index 0a1c0741bb5e38e48247310aa1e13420113e08d8..1373dd2aff8b966b44fdbd6869e6559bd5733571 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -1121,11 +1121,10 @@ bool ChromeContentRendererClient::ShouldFork(WebLocalFrame* frame,
 bool ChromeContentRendererClient::WillSendRequest(
     WebFrame* frame,
     ui::PageTransition transition_type,
-    const GURL& url,
-    const GURL& first_party_for_cookies,
+    const blink::WebURL& url,
     GURL* new_url) {
-  // Check whether the request should be allowed. If not allowed, we reset the
-  // URL to something invalid to prevent the request and cause an error.
+// Check whether the request should be allowed. If not allowed, we reset the
+// URL to something invalid to prevent the request and cause an error.
 #if defined(ENABLE_EXTENSIONS)
   if (ChromeExtensionsRendererClient::GetInstance()->WillSendRequest(
           frame, transition_type, url, new_url)) {
@@ -1133,18 +1132,23 @@ bool ChromeContentRendererClient::WillSendRequest(
   }
 #endif
 
+  if (!url.protocolIs(chrome::kChromeSearchScheme))
+    return false;
+
   const content::RenderView* render_view =
       content::RenderView::FromWebView(frame->view());
   SearchBox* search_box = SearchBox::Get(render_view);
-  if (search_box && url.SchemeIs(chrome::kChromeSearchScheme)) {
+  if (search_box) {
+    // Note: this GURL copy could be avoided if host() were added to WebURL.
+    GURL gurl(url);
     SearchBox::ImageSourceType type = SearchBox::NONE;
-    if (url.host() == chrome::kChromeUIFaviconHost)
+    if (gurl.host() == chrome::kChromeUIFaviconHost)

[Devlin, 2016/10/31 16:00:58]

Because this is a cl targeting cleaning up unneeded copies...

GURL::host() actually makes a new std::string with each invocation (separately -
I wonder if we can fix that, at least making a base::StringPiece() variant -
there's no reason it *needs* to do the allocation).  Maybe cache std::string
host at line 1144 so we can just use that?

[Charlie Harrison, 2016/10/31 16:37:34]

Good catch! We actually have GURL::host_piece which does just what you want.
Possibly we should switch the default to use StringPiece.

[Devlin, 2016/10/31 16:57:42]

...I swear I looked for *just that*.  I was on the verge of filing a bug for
creating GURL::host_piece().  Not sure how I missed it.  Thanks! :)

       type = SearchBox::FAVICON;
-    else if (url.host() == chrome::kChromeUILargeIconHost)
+    else if (gurl.host() == chrome::kChromeUILargeIconHost)
       type = SearchBox::LARGE_ICON;
-    else if (url.host() == chrome::kChromeUIFallbackIconHost)
+    else if (gurl.host() == chrome::kChromeUIFallbackIconHost)
       type = SearchBox::FALLBACK_ICON;
-    else if (url.host() == chrome::kChromeUIThumbnailHost)
+    else if (gurl.host() == chrome::kChromeUIThumbnailHost)
       type = SearchBox::THUMB;
 
     if (type != SearchBox::NONE)