Create window in a new BrowsingInstance when opening a link in a new process.

This allows the rel=noreferrer process trick to work even for same-site links.

BUG=69267
TEST=Same-site link to rel=noreferrer target=_blank link gets new process.

Review URL: https://chromiumcodereview.appspot.com/9325082

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@125180 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 123 insertions, 56 deletions

diff --git a/content/browser/renderer_host/render_view_host_manager_browsertest.cc b/content/browser/renderer_host/render_view_host_manager_browsertest.cc
index cc84f6d..ebe8aed 100644
--- a/content/browser/renderer_host/render_view_host_manager_browsertest.cc
+++ b/content/browser/renderer_host/render_view_host_manager_browsertest.cc
@@ -98,6 +98,64 @@ IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
   EXPECT_NE(orig_site_instance, noref_blank_site_instance);
 }
 
+// As of crbug.com/69267, we create a new BrowsingInstance (and SiteInstance)
+// for rel=noreferrer links in new windows, even to same site pages and named
+// targets.
+IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
+                       SwapProcessWithSameSiteRelNoreferrer) {
+  // Start two servers with different sites.
+  ASSERT_TRUE(test_server()->Start());
+  net::TestServer https_server(
+      net::TestServer::TYPE_HTTPS,
+      net::TestServer::kLocalhost,
+      FilePath(FILE_PATH_LITERAL("chrome/test/data")));
+  ASSERT_TRUE(https_server.Start());
+
+  // Load a page with links that open in a new window.
+  std::string replacement_path;
+  ASSERT_TRUE(GetFilePathWithHostAndPortReplacement(
+      "files/click-noreferrer-links.html",
+      https_server.host_port_pair(),
+      &replacement_path));
+  ui_test_utils::NavigateToURL(browser(),
+                               test_server()->GetURL(replacement_path));
+
+  // Get the original SiteInstance for later comparison.
+  scoped_refptr<SiteInstance> orig_site_instance(
+      browser()->GetSelectedWebContents()->GetSiteInstance());
+  EXPECT_TRUE(orig_site_instance != NULL);
+
+  // Test clicking a same-site rel=noreferrer + target=foo link.
+  bool success = false;
+  EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+      browser()->GetSelectedWebContents()->GetRenderViewHost(), L"",
+      L"window.domAutomationController.send(clickSameSiteNoRefTargetedLink());",
+      &success));
+  EXPECT_TRUE(success);
+
+  // Wait for the tab to open.
+  if (browser()->tab_count() < 2)
+    ui_test_utils::WaitForNewTab(browser());
+
+  // Opens in new tab.
+  EXPECT_EQ(2, browser()->tab_count());
+  EXPECT_EQ(1, browser()->active_index());
+  EXPECT_EQ("/files/title2.html",
+            browser()->GetSelectedWebContents()->GetURL().path());
+
+  // Wait for the cross-site transition in the new tab to finish.
+  ui_test_utils::WaitForLoadStop(browser()->GetSelectedWebContents());
+  TabContents* tab_contents = static_cast<TabContents*>(
+      browser()->GetSelectedWebContents());
+  EXPECT_FALSE(tab_contents->GetRenderManagerForTesting()->
+      pending_render_view_host());
+
+  // Should have a new SiteInstance (in a new BrowsingInstance).
+  scoped_refptr<SiteInstance> noref_blank_site_instance(
+      browser()->GetSelectedWebContents()->GetSiteInstance());
+  EXPECT_NE(orig_site_instance, noref_blank_site_instance);
+}
+
 // Test for crbug.com/24447.  Following a cross-site link with just
 // target=_blank should not create a new SiteInstance.
 IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
diff --git a/content/browser/renderer_host/render_widget_helper.cc b/content/browser/renderer_host/render_widget_helper.cc
index 93b931a..f230b9c 100644
--- a/content/browser/renderer_host/render_widget_helper.cc
+++ b/content/browser/renderer_host/render_widget_helper.cc
@@ -215,13 +215,23 @@ void RenderWidgetHelper::CreateNewWindow(
     base::ProcessHandle render_process,
     int* route_id,
     int* surface_id) {
-  *route_id = GetNextRoutingID();
-  *surface_id = GpuSurfaceTracker::Get()->AddSurfaceForRenderer(
-      render_process_id_, *route_id);
-  // Block resource requests until the view is created, since the HWND might be
-  // needed if a response ends up creating a plugin.
-  resource_dispatcher_host_->BlockRequestsForRoute(
-      render_process_id_, *route_id);
+  if (params.opener_suppressed) {
+    // If the opener is supppressed, we should open the window in a new
+    // BrowsingInstance, and thus a new process.  That means the current
+    // renderer process will not be able to route messages to it.  Because of
+    // this, we will immediately show and navigate the window in
+    // OnCreateWindowOnUI, using the params provided here.
+    *route_id = MSG_ROUTING_NONE;
+    *surface_id = 0;
+  } else {
+    *route_id = GetNextRoutingID();
+    *surface_id = GpuSurfaceTracker::Get()->AddSurfaceForRenderer(
+        render_process_id_, *route_id);
+    // Block resource requests until the view is created, since the HWND might
+    // be needed if a response ends up creating a plugin.
+    resource_dispatcher_host_->BlockRequestsForRoute(
+        render_process_id_, *route_id);
+  }
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
@@ -237,9 +247,13 @@ void RenderWidgetHelper::OnCreateWindowOnUI(
   if (host)
     host->CreateNewWindow(route_id, params);
 
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&RenderWidgetHelper::OnCreateWindowOnIO, this, route_id));
+  // We only need to resume blocked requests if we used a valid route_id.
+  // See CreateNewWindow.
+  if (route_id != MSG_ROUTING_NONE) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&RenderWidgetHelper::OnCreateWindowOnIO, this, route_id));
+  }
 }
 
 void RenderWidgetHelper::OnCreateWindowOnIO(int route_id) {
diff --git a/content/browser/tab_contents/tab_contents_view_helper.cc b/content/browser/tab_contents/tab_contents_view_helper.cc
index d21a8bd..98817e6 100644
--- a/content/browser/tab_contents/tab_contents_view_helper.cc
+++ b/content/browser/tab_contents/tab_contents_view_helper.cc
@@ -11,6 +11,7 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/notification_types.h"
+#include "content/public/browser/site_instance.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/browser/web_contents_view.h"
@@ -58,16 +59,43 @@ TabContents* TabContentsViewHelper::CreateNewWindow(
   if (!should_create)
     return NULL;
 
+  // We usually create the new window in the same BrowsingInstance (group of
+  // script-related windows), by passing in the current SiteInstance.  However,
+  // if the opener is being suppressed, we create a new SiteInstance in its own
+  // BrowsingInstance.
+  scoped_refptr<content::SiteInstance> site_instance =
+      params.opener_suppressed ?
+      content::SiteInstance::Create(web_contents->GetBrowserContext()) :
+      web_contents->GetSiteInstance();
+
   // Create the new web contents. This will automatically create the new
   // WebContentsView. In the future, we may want to create the view separately.
   TabContents* new_contents =
       new TabContents(web_contents->GetBrowserContext(),
-                      web_contents->GetSiteInstance(),
+                      site_instance,
                       route_id,
                       static_cast<TabContents*>(web_contents),
                       NULL);
   new_contents->set_opener_web_ui_type(
       web_contents->GetWebUITypeForCurrentState());
+
+  if (params.opener_suppressed) {
+    // When the opener is suppressed, the original renderer cannot access the
+    // new window.  As a result, we need to show and navigate the window here.
+    gfx::Rect initial_pos;
+    web_contents->AddNewContents(new_contents,
+                                 params.disposition,
+                                 initial_pos,
+                                 params.user_gesture);
+    content::OpenURLParams open_params(params.target_url, content::Referrer(),
+                                       CURRENT_TAB,
+                                       content::PAGE_TRANSITION_LINK,
+                                       true /* is_renderer_initiated */);
+    WebContents* opened_contents = new_contents->OpenURL(open_params);
+    DCHECK_EQ(new_contents, opened_contents);
+    return new_contents;
+  }
+
   content::WebContentsView* new_view = new_contents->GetView();
 
   // TODO(brettw): It seems bogus that we have to call this function on the
@@ -75,6 +103,7 @@ TabContents* TabContentsViewHelper::CreateNewWindow(
   new_view->CreateViewForWidget(new_contents->GetRenderViewHost());
 
   // Save the created window associated with the route so we can show it later.
+  DCHECK_NE(MSG_ROUTING_NONE, route_id);
   pending_contents_[route_id] = new_contents;
 
   if (web_contents->GetDelegate())
diff --git a/content/common/view_messages.h b/content/common/view_messages.h
index 9ce789f..2ef5024 100644
--- a/content/common/view_messages.h
+++ b/content/common/view_messages.h
@@ -370,6 +370,13 @@ IPC_STRUCT_BEGIN(ViewHostMsg_CreateWindow_Params)
   // The security origin of the frame initiating the open.
   IPC_STRUCT_MEMBER(std::string, opener_security_origin)
 
+  // Whether the opener will be suppressed in the new window, in which case
+  // scripting the new window is not allowed.
+  IPC_STRUCT_MEMBER(bool, opener_suppressed)
+
+  // Whether the window should be opened in the foreground, background, etc.
+  IPC_STRUCT_MEMBER(WindowOpenDisposition, disposition)
+
   // The URL that will be loaded in the new window (empty if none has been
   // sepcified).
   IPC_STRUCT_MEMBER(GURL, target_url)
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 59a0c46..9f34b6c 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -1427,22 +1427,12 @@ bool RenderViewImpl::SendAndRunNestedMessageLoop(IPC::SyncMessage* message) {
 
 // WebKit::WebViewClient ------------------------------------------------------
 
-// TODO(creis): New contract for createView temporarily redirects to the old
-// contract.  Remove the old one as part of http://crbug.com/69267.
 WebView* RenderViewImpl::createView(
     WebFrame* creator,
     const WebURLRequest& request,
     const WebWindowFeatures& features,
     const WebString& frame_name,
     WebNavigationPolicy policy) {
-  return createView(creator, request, features, frame_name);
-}
-
-WebView* RenderViewImpl::createView(
-    WebFrame* creator,
-    const WebURLRequest& request,
-    const WebWindowFeatures& features,
-    const WebString& frame_name) {
   // Check to make sure we aren't overloading on popups.
   if (shared_popup_counter_->data > kMaximumNumberOfUnacknowledgedPopups)
     return NULL;
@@ -1457,13 +1447,14 @@ WebView* RenderViewImpl::createView(
   params.opener_url = creator->document().url();
   params.opener_security_origin =
       creator->document().securityOrigin().toString().utf8();
+  params.opener_suppressed = creator->willSuppressOpenerInNewFrame();
+  params.disposition = NavigationPolicyToDisposition(policy);
   if (!request.isNull())
     params.target_url = request.url();
 
   int32 routing_id = MSG_ROUTING_NONE;
   int32 surface_id = 0;
   int64 cloned_session_storage_namespace_id;
-  bool opener_suppressed = creator->willSuppressOpenerInNewFrame();
 
   RenderThread::Get()->Send(
       new ViewHostMsg_CreateWindow(params,
@@ -1488,7 +1479,7 @@ WebView* RenderViewImpl::createView(
   view->opened_by_user_gesture_ = params.user_gesture;
 
   // Record whether the creator frame is trying to suppress the opener field.
-  view->opener_suppressed_ = opener_suppressed;
+  view->opener_suppressed_ = params.opener_suppressed;
 
   // Record the security origin of the creator.
   GURL creator_url(creator->document().securityOrigin().toString().utf8());
@@ -2356,33 +2347,7 @@ WebNavigationPolicy RenderViewImpl::decidePolicyForNavigation(
       // Must be a JavaScript navigation, which appears as "other".
       type == WebKit::WebNavigationTypeOther;
 
-  // Recognize if this navigation is from a link with rel=noreferrer and
-  // target=_blank attributes, in which case the opener will be suppressed. If
-  // so, it is safe to load cross-site pages in a separate process, so we
-  // should let the browser handle it.
-  bool is_noreferrer_and_blank_target =
-      // Frame should be top level and not yet navigated.
-      frame->parent() == NULL &&
-      frame->document().url().isEmpty() &&
-      historyBackListCount() < 1 &&
-      historyForwardListCount() < 1 &&
-      // Links with rel=noreferrer will have no Referer field, and their
-      // resulting frame will have its window.opener suppressed.
-      // TODO(creis): should add a request.httpReferrer() method to help avoid
-      // typos on the unusual spelling of Referer.
-      request.httpHeaderField(WebString::fromUTF8("Referer")).isNull() &&
-      opener_suppressed_ &&
-      frame->opener() == NULL &&
-      // Links with target=_blank will have no name.
-      frame->name().isNull() &&
-      // Another frame (with a non-empty creator) should have initiated the
-      // request, targeted at this frame.
-      !creator_url_.is_empty() &&
-      is_content_initiated &&
-      default_policy == WebKit::WebNavigationPolicyCurrentTab &&
-      type == WebKit::WebNavigationTypeOther;
-
-  if (is_fork || is_noreferrer_and_blank_target) {
+  if (is_fork) {
     // Open the URL via the browser, not via WebKit.
     OpenURL(frame, url, Referrer(), default_policy);
     return WebKit::WebNavigationPolicyIgnore;
diff --git a/content/renderer/render_view_impl.h b/content/renderer/render_view_impl.h
index 8117125..a7c893b 100644
--- a/content/renderer/render_view_impl.h
+++ b/content/renderer/render_view_impl.h
@@ -340,12 +340,6 @@ class RenderViewImpl : public RenderWidget,
       const WebKit::WebWindowFeatures& features,
       const WebKit::WebString& frame_name,
       WebKit::WebNavigationPolicy policy);
-  // TODO(creis): Remove as part of http://crbug.com/69267.
-  virtual WebKit::WebView* createView(
-      WebKit::WebFrame* creator,
-      const WebKit::WebURLRequest& request,
-      const WebKit::WebWindowFeatures& features,
-      const WebKit::WebString& frame_name);
   virtual WebKit::WebWidget* createPopupMenu(WebKit::WebPopupType popup_type);
   virtual WebKit::WebWidget* createPopupMenu(
       const WebKit::WebPopupMenuInfo& info);