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

chrome/browser/chrome_navigation_browsertest.cc

Index: chrome/browser/chrome_navigation_browsertest.cc
diff --git a/chrome/browser/chrome_navigation_browsertest.cc b/chrome/browser/chrome_navigation_browsertest.cc
index 0d243913e6a425fd033568733e1b86738e2d631c..b2e57ce03dc3a2f0445ac394148ba305ccc2bfe4 100644
--- a/chrome/browser/chrome_navigation_browsertest.cc
+++ b/chrome/browser/chrome_navigation_browsertest.cc
@@ -189,6 +189,119 @@ IN_PROC_BROWSER_TEST_F(ChromeNavigationBrowserTest, TestViewFrameSource) {
             new_web_contents->GetTitle());
 }
 
+// Tests that verify that ctrl-click results 1) open up in a new renderer
+// process (https://crbug.com/23815) and 2) are in a new browsing instance (e.g.
+// cannot find the opener's window by name - https://crbug.com/658386).
+class CtrlClickShouldEndUpInNewProcessTest
+    : public ChromeNavigationBrowserTest {
+ protected:
+  // Simulates ctrl-clicking an anchor with the given name in |main_contents|.
+  // Verifies that the new contents are in a separate process and separate
+  // browsing instance from |main|contents.  Returns contents of the newly

[Charlie Reis, 2017/04/19 20:11:11]

nit: |main_contents|

[Łukasz Anforowicz, 2017/04/20 01:11:29]

Ooops.  Not sure how that happened.  Done.

Also - s/name/id/ (regarding how the anchor is identified).

+  // opened tab.
+  content::WebContents* SimulateCtrlClick(content::WebContents* main_contents,
+                                          const char* id_of_anchor_to_click) {
+    // Ctrl-click the anchor/link in the page.
+    content::WebContents* new_contents = nullptr;
+    {
+      content::WebContentsAddedObserver new_tab_observer;
+#if defined(OS_MACOSX)
+      const char* new_tab_click_script_template =
+          "simulateClick(\"%s\", { metaKey: true });";
+#else
+      const char* new_tab_click_script_template =
+          "simulateClick(\"%s\", { ctrlKey: true });";
+#endif
+      std::string new_tab_click_script = base::StringPrintf(
+          new_tab_click_script_template, id_of_anchor_to_click);
+      EXPECT_TRUE(ExecuteScript(main_contents, new_tab_click_script));
+
+      // Wait for a new tab to appear (the whole point of this test).
+      new_contents = new_tab_observer.GetWebContents();
+    }
+
+    // Verify that the new tab has the right contents and is in the right, new
+    // place in the tab strip.
+    EXPECT_TRUE(WaitForLoadStop(new_contents));
+    int last_tab_index = browser()->tab_strip_model()->count() - 1;
+    EXPECT_LT(1, browser()->tab_strip_model()->count());  // More than 1 tab?
+    EXPECT_EQ(new_contents,
+              browser()->tab_strip_model()->GetWebContentsAt(last_tab_index));
+    GURL expected_url(embedded_test_server()->GetURL("/title1.html"));
+    EXPECT_EQ(expected_url, new_contents->GetLastCommittedURL());
+
+    // Verify that the new tab is in a different process, SiteInstance and
+    // BrowsingInstance from the old contents.
+    EXPECT_NE(main_contents->GetMainFrame()->GetProcess(),
+              new_contents->GetMainFrame()->GetProcess());
+    EXPECT_NE(main_contents->GetMainFrame()->GetSiteInstance(),
+              new_contents->GetMainFrame()->GetSiteInstance());
+    EXPECT_FALSE(main_contents->GetSiteInstance()->IsRelatedSiteInstance(
+        new_contents->GetSiteInstance()));
+
+    // Verify that the new BrowsingInstance can't see windows from the old one.
+    {
+      // Double-check that main_contents has expected window.name set.
+      // This is a sanity check of test setup; this is not a product test.
+      std::string name_of_main_contents_window;
+      EXPECT_TRUE(ExecuteScriptAndExtractString(
+          main_contents, "window.domAutomationController.send(window.name)",
+          &name_of_main_contents_window));
+      EXPECT_EQ("main_contents", name_of_main_contents_window);
+
+      // 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);
+
+      // Verify that the new contents cannot find the old contents via
+      // window.open. (i.e. window.open should open a new window, rather than
+      // returning a reference to main_contents / old window).
+      std::string location_of_opened_window;
+      EXPECT_TRUE(ExecuteScriptAndExtractString(
+          new_contents,
+          "w = window.open('', 'main_contents');"
+          "window.domAutomationController.send(w.location.href);",
+          &location_of_opened_window));
+      EXPECT_EQ(url::kAboutBlankURL, location_of_opened_window);
+    }
+
+    return new_contents;
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(CtrlClickShouldEndUpInNewProcessTest, NoTarget) {
+  // Navigate to the test page.
+  GURL main_url(embedded_test_server()->GetURL(
+      "/frame_tree/anchor_to_same_site_location.html"));
+  ui_test_utils::NavigateToURL(browser(), main_url);
+  const char* kIdOfAnchorToClick = "test-anchor-no-target";
+
+  // Verify that there is only 1 active tab (with the right contents committed).
+  EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
+  content::WebContents* main_contents =
+      browser()->tab_strip_model()->GetWebContentsAt(0);
+  EXPECT_EQ(main_url, main_contents->GetLastCommittedURL());
+
+  // Test what happens after ctrl-click.  SimulateCtrlClick will verify
+  // that |new_contents1| is in a separate process and browsing instance
+  // from |main_contents|.
+  content::WebContents* new_contents1 =
+      SimulateCtrlClick(main_contents, kIdOfAnchorToClick);
+
+  // Test that each subsequent ctrl-click also gets a new process.

[Charlie Reis, 2017/04/19 20:11:11]

Nice-- I like this (especially given the unexpected behavior we saw recently for
subsequent ctrl-clicks)!

+  content::WebContents* new_contents2 =
+      SimulateCtrlClick(main_contents, kIdOfAnchorToClick);
+  EXPECT_NE(new_contents1->GetMainFrame()->GetProcess(),
+            new_contents2->GetMainFrame()->GetProcess());
+  EXPECT_NE(new_contents1->GetMainFrame()->GetSiteInstance(),
+            new_contents2->GetMainFrame()->GetSiteInstance());
+  EXPECT_FALSE(new_contents1->GetSiteInstance()->IsRelatedSiteInstance(
+      new_contents2->GetSiteInstance()));
+}
+
 class ChromeNavigationPortMappedBrowserTest : public InProcessBrowserTest {
  public:
   ChromeNavigationPortMappedBrowserTest() {}