Stop initializing url::Origin in extensions OnBeforeURLRequest when not needed

chrome/browser/net/chrome_extensions_network_delegate.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/net/chrome_extensions_network_delegate.h"
 
 #include <stdint.h>
 
 #include "base/macros.h"
 #include "extensions/features/features.h"
@@ skipping 155 matching lines @@
     net::URLRequest* request) {
   ForwardRequestStatus(REQUEST_DONE, request, profile_);
 }
 
 int ChromeExtensionsNetworkDelegateImpl::OnBeforeURLRequest(
     net::URLRequest* request,
     const net::CompletionCallback& callback,
     GURL* new_url) {
   const content::ResourceRequestInfo* info =
       content::ResourceRequestInfo::ForRequest(request);
-  GURL url(request->url());
+  const GURL& url(request->url());
 
   // Block top-level navigations to blob: or filesystem: URLs with extension
   // origin from non-extension processes.  See https://crbug.com/645028.
   //
   // TODO(alexmos): This check is redundant with the one in
   // ExtensionNavigationThrottle::WillStartRequest, which was introduced in
   // M56. This check is reintroduced temporarily to tighten this blocking for
   // apps with a "webview" permission on M55/54 (see https://crbug.com/656752).
   // It will be removed after it's merged.  Unlike the check in
   // ExtensionNavigationThrottle, this check is incompatible with PlzNavigate
   // and is disabled for that mode.
   bool is_nested_url = url.SchemeIsFileSystem() || url.SchemeIsBlob();
   bool is_navigation =
       info && content::IsResourceTypeFrame(info->GetResourceType());
-  url::Origin origin(url);
+  url::Origin origin;
   if (is_nested_url && is_navigation && info->IsMainFrame() &&
-      origin.scheme() == extensions::kExtensionScheme &&
+      (origin = url::Origin(url)).scheme() == extensions::kExtensionScheme &&

[Randy Smith (Not in Mondays), 2016/12/06 16:09:20]

Suggestion: I understand why you're doing this, and I feel the pull of terseness
myself, but I think looking at this code it both takes a moment to understand
what's actually happening and is actively hard to understand why it was written
this way (so that the origin copy doesn't happen unless the previous elements of
the conditional).  I'd suggest making this a doubly nested conditional with the
origin declaration (and thus assignment) in between the two conditionals.

Alternatively, you could explore turning this into an "early return" function by
hoisting the case at the end of it up to this point at which point you don't
need to worry about nesting conditionals at all.

[Charlie Harrison, 2016/12/06 18:23:38]

Moved it to a nested conditional and added a justification comment.

       !extension_info_map_->process_map().Contains(info->GetChildID()) &&
       !content::IsBrowserSideNavigationEnabled()) {
     // Relax this restriction for apps that use <webview>.  See
     // https://crbug.com/652077.
     const extensions::Extension* extension =
         extension_info_map_->extensions().GetByID(origin.host());
     bool has_webview_permission =
         extension &&
         extension->permissions_data()->HasAPIPermission(
             extensions::APIPermission::kWebView);
@@ skipping 197 matching lines @@
 }
 
 net::NetworkDelegate::AuthRequiredResponse
 ChromeExtensionsNetworkDelegate::OnAuthRequired(
     net::URLRequest* request,
     const net::AuthChallengeInfo& auth_info,
     const AuthCallback& callback,
     net::AuthCredentials* credentials) {
   return net::NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION;
 }