New WebContents created via ctrl-click should be in a new process.

chrome/browser/extensions/api/tabs/tabs_test.cc

 // Copyright (c) 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 <limits.h>
 #include <stddef.h>
 #include <stdint.h>
 
 #include <memory>
 #include <string>
@@ skipping 21 matching lines @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/zoom/chrome_zoom_level_prefs.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/page_zoom.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/test/browser_test_utils.h"
 #include "extensions/browser/api_test_utils.h"
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/browser/app_window/app_window_registry.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/test_util.h"
 #include "extensions/test/extension_test_message_listener.h"
@@ skipping 2057 matching lines @@
   EXPECT_TRUE(
       base::MatchPattern(error, manifest_errors::kCannotAccessChromeUrl));
 
   // chrome.tabs.setZoomSettings().
   error = RunSetZoomSettingsExpectError(tab_id, "manual", "per-tab");
   EXPECT_TRUE(
       base::MatchPattern(error, manifest_errors::kCannotAccessChromeUrl));
 }
 
 // Regression test for crbug.com/660498.
-IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Foo) {
+IN_PROC_BROWSER_TEST_F(ExtensionApiTest, TemporaryAddressSpoof) {
   ASSERT_TRUE(StartEmbeddedTestServer());
   content::WebContents* first_web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_TRUE(first_web_contents);
   chrome::NewTab(browser());
   content::WebContents* second_web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_NE(first_web_contents, second_web_contents);
   GURL url = embedded_test_server()->GetURL(
       "/extensions/api_test/tabs/pdf_extension_test.html");
   content::TestNavigationManager navigation_manager(
       second_web_contents, GURL("http://www.facebook.com:83"));
   ui_test_utils::NavigateToURLWithDisposition(
       browser(), url, WindowOpenDisposition::CURRENT_TAB,
       ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
   EXPECT_TRUE(navigation_manager.WaitForRequestStart());
 
   browser()->tab_strip_model()->ActivateTabAt(0, true);
   EXPECT_EQ(first_web_contents,
             browser()->tab_strip_model()->GetActiveWebContents());
   browser()->tab_strip_model()->ActivateTabAt(1, true);
   EXPECT_EQ(second_web_contents,
             browser()->tab_strip_model()->GetActiveWebContents());
 
   EXPECT_EQ(url, second_web_contents->GetVisibleURL());
 }
 
+// Window created by chrome.windows.create should be in the same SiteInstance
+// and BrowsingInstance as the opener - this is a regression test for
+// hangouts-vs-isolate-extensions-trouble (see also https://crbug.com/597750).
+IN_PROC_BROWSER_TEST_F(ExtensionApiTest, WindowsCreateVsSiteInstance) {
+  const extensions::Extension* extension =
+      LoadExtension(test_data_dir_.AppendASCII("../simple_with_file"));
+  ASSERT_TRUE(extension);
+
+  // Navigate a tab to an extension page.
+  GURL extension_url = extension->GetResourceURL("file.html");
+  ui_test_utils::NavigateToURL(browser(), extension_url);
+  content::WebContents* old_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  // Execute chrome.windows.create and store the new tab in |new_contents|.
+  content::WebContents* new_contents;
+  {
+    content::WebContentsAddedObserver observer;
+    ASSERT_TRUE(content::ExecuteScript(old_contents,
+                                       "window.name = 'test-name';\n"
+                                       "chrome.windows.create({url: '" +
+                                           extension_url.spec() + "'})"));
+    new_contents = observer.GetWebContents();
+  }
+
+  // Verify that the old and new tab are in the same process and SiteInstance.
+  // Note: both test assertions are important - one observed failure mode was
+  // having the same process, but different SiteInstance.
+  EXPECT_EQ(old_contents->GetMainFrame()->GetProcess(),
+            new_contents->GetMainFrame()->GetProcess());
+  EXPECT_EQ(old_contents->GetMainFrame()->GetSiteInstance(),
+            new_contents->GetMainFrame()->GetSiteInstance());
+
+  // Verify that the |new_contents| doesn't have a |window.opener| set.
+  bool window_opener_cast_to_bool;
+  EXPECT_TRUE(ExecuteScriptAndExtractBool(
+      new_contents, "window.domAutomationController.send(!!window.opener)",
+      &window_opener_cast_to_bool));
+  EXPECT_FALSE(window_opener_cast_to_bool);

[Łukasz Anforowicz, 2017/04/20 19:03:57]

I find the current behavior above a bit weird (note that the current CL doesn't
change this behavior - it only adds explicit test to prevent hangouts
regressions).  It is weird that 1) windows can find each other by
window.open('', other_window_name), but 2) window.opener is null.  Maybe things
would be cleaner if chrome.windows.create would populate window.opener (since it
does put the new window in the same BrowsingInstance).  I am guessing that this
might be possibly accomplished by having chrome.windows.create (somehow) set
WebContents::CreateParams::opener_xxx fields, rather than relying on the
(somewhat indirect/magical) influence that
WebContents::CreateParams::site_instance has on
BrowsingInstance/window-findability.

+}
+
 }  // namespace extensions