Fix WebContentsDelegate::ShouldCreateWebContents implementations.

chrome/browser/ui/browser.cc

 // Copyright 2012 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/ui/browser.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <string>
@@ skipping 1562 matching lines @@
   DCHECK(source);
   chrome::ViewSource(this, source, frame_url, frame_page_state);
 }
 
 void Browser::ShowRepostFormWarningDialog(WebContents* source) {
   TabModalConfirmDialog::Create(new RepostFormWarningController(source),
                                 source);
 }
 
 bool Browser::ShouldCreateWebContents(
-    WebContents* web_contents,
+    content::WebContents* web_contents,
+    content::SiteInstance* source_site_instance,
     int32_t route_id,
     int32_t main_frame_route_id,
     int32_t main_frame_widget_route_id,
     WindowContainerType window_container_type,
+    const GURL& opener_url,
     const std::string& frame_name,
     const GURL& target_url,
     const std::string& partition_id,
     content::SessionStorageNamespace* session_storage_namespace) {
   if (window_container_type == WINDOW_CONTAINER_TYPE_BACKGROUND) {
     // If a BackgroundContents is created, suppress the normal WebContents.
     return !MaybeCreateBackgroundContents(
-        route_id, main_frame_route_id, main_frame_widget_route_id, web_contents,
-        frame_name, target_url, partition_id, session_storage_namespace);
+        source_site_instance, opener_url, route_id, main_frame_route_id,
+        main_frame_widget_route_id, frame_name, target_url, partition_id,
+        session_storage_namespace);
   }
 
   return true;
 }
 
 void Browser::WebContentsCreated(WebContents* source_contents,
                                  int opener_render_process_id,
                                  int opener_render_frame_id,
                                  const std::string& frame_name,
                                  const GURL& target_url,
@@ skipping 905 matching lines @@
   // Windows and GTK remove the browser controls in fullscreen, but Mac and Ash
   // keep the controls in a slide-down panel.
   return window_ && window_->ShouldHideUIForFullscreen();
 }
 
 bool Browser::ShouldStartShutdown() const {
   return BrowserList::GetInstance()->size() <= 1;
 }
 
 bool Browser::MaybeCreateBackgroundContents(
+    content::SiteInstance* site_instance,
+    const GURL& opener_url,
     int32_t route_id,
     int32_t main_frame_route_id,
     int32_t main_frame_widget_route_id,
-    WebContents* opener_web_contents,
     const std::string& frame_name,
     const GURL& target_url,
     const std::string& partition_id,
     content::SessionStorageNamespace* session_storage_namespace) {
-  GURL opener_url = opener_web_contents->GetURL();

[Charlie Reis, 2016/11/23 06:59:48]

Yuck!  Thanks for fixing this.

[ncarter (slow), 2016/11/24 00:00:05]

Acknowledged.

   ExtensionService* extensions_service =
       extensions::ExtensionSystem::Get(profile_)->extension_service();
 
-  if (!opener_url.is_valid() ||
-      frame_name.empty() ||
-      !extensions_service ||
+  if (!opener_url.is_valid() || frame_name.empty() || !extensions_service ||
       !extensions_service->is_ready())
     return false;
 
   // Only hosted apps have web extents, so this ensures that only hosted apps
   // can create BackgroundContents. We don't have to check for background
   // permission as that is checked in RenderMessageFilter when the CreateWindow
   // message is processed.
   const Extension* extension = extensions::ExtensionRegistry::Get(profile_)
                                    ->enabled_extensions()
                                    .GetHostedAppByURL(opener_url);
   if (!extension)
     return false;
 
   // No BackgroundContents allowed if BackgroundContentsService doesn't exist.
   BackgroundContentsService* service =
       BackgroundContentsServiceFactory::GetForProfile(profile_);
   if (!service)
     return false;
 
   // Ensure that we're trying to open this from the extension's process.
-  SiteInstance* opener_site_instance = opener_web_contents->GetSiteInstance();

[Charlie Reis, 2016/11/23 06:59:48]

And this!

[ncarter (slow), 2016/11/24 00:00:05]

Done.

   extensions::ProcessMap* process_map = extensions::ProcessMap::Get(profile_);
-  if (!opener_site_instance->GetProcess() ||
-      !process_map->Contains(
-          extension->id(), opener_site_instance->GetProcess()->GetID())) {
+  if (!site_instance->GetProcess() ||
+      !process_map->Contains(extension->id(),
+                             site_instance->GetProcess()->GetID())) {
     return false;
   }
 
   // Only allow a single background contents per app.
   bool allow_js_access = extensions::BackgroundInfo::AllowJSAccess(extension);
   BackgroundContents* existing =
       service->GetAppBackgroundContents(base::ASCIIToUTF16(extension->id()));
   if (existing) {
     // For non-scriptable background contents, ignore the request altogether,
     // (returning true, so that a regular WebContents isn't created either).
     if (!allow_js_access)
       return true;
     // For scriptable background pages, if one already exists, close it (even
     // if it was specified in the manifest).
     delete existing;
   }
 
-  // If script access is not allowed, create the the background contents in a
-  // new SiteInstance, so that a separate process is used.
-  scoped_refptr<content::SiteInstance> site_instance =
-      allow_js_access ?
-      opener_site_instance :
-      content::SiteInstance::Create(opener_web_contents->GetBrowserContext());
+  // Passed all the checks, so this should be created as a BackgroundContents.
+  BackgroundContents* contents = nullptr;
+  if (allow_js_access) {
+    contents = service->CreateBackgroundContents(
+        site_instance, route_id, main_frame_route_id,
+        main_frame_widget_route_id, profile_, frame_name,
+        base::ASCIIToUTF16(extension->id()), partition_id,
+        session_storage_namespace);
+  } else {
+    // If script access is not allowed, create the the background contents in a
+    // new SiteInstance, so that a separate process is used. We must not use any
+    // of the passed-in routing IDs, as they are objects in the opener's
+    // process.
+    contents = service->CreateBackgroundContents(
+        content::SiteInstance::Create(site_instance->GetBrowserContext()),
+        MSG_ROUTING_NONE, MSG_ROUTING_NONE, MSG_ROUTING_NONE, profile_,
+        frame_name, base::ASCIIToUTF16(extension->id()), partition_id,
+        session_storage_namespace);
 
-  // Passed all the checks, so this should be created as a BackgroundContents.
-  BackgroundContents* contents = service->CreateBackgroundContents(
-      site_instance.get(), route_id, main_frame_route_id,
-      main_frame_widget_route_id, profile_, frame_name,
-      base::ASCIIToUTF16(extension->id()), partition_id,
-      session_storage_namespace);
-
-  // When a separate process is used, the original renderer cannot access the
-  // new window later, thus we need to navigate the window now.
-  if (contents && !allow_js_access) {
-    contents->web_contents()->GetController().LoadURL(
-        target_url,
-        content::Referrer(),
-        ui::PAGE_TRANSITION_LINK,
-        std::string());  // No extra headers.
+    // When a separate process is used, the original renderer cannot access the
+    // new window later, thus we need to navigate the window now.
+    if (contents) {
+      contents->web_contents()->GetController().LoadURL(
+          target_url, content::Referrer(), ui::PAGE_TRANSITION_LINK,
+          std::string());  // No extra headers.
+    }
   }
 
   return contents != NULL;
 }