Second attempt to swap processes on rel=noreferrer, target=blank links.

The test timed out on one of the bots last time.  This change tries to fix that, and it breaks down the test into multiple tests to isolate the problem in case it isn't fixed.

No changes to code outside the test and the test HTML file.  Previous review at http://codereview.chromium.org/284015.

BUG=24447
TEST=RenderViewHostManagerTest.*SwapProcess*

Review URL: http://codereview.chromium.org/328017

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

7 files changed, 218 insertions, 2 deletions

diff --git a/chrome/browser/renderer_host/test/render_view_host_manager_browsertest.cc b/chrome/browser/renderer_host/test/render_view_host_manager_browsertest.cc
index 4fe70cd..dd6546c 100644
--- a/chrome/browser/renderer_host/test/render_view_host_manager_browsertest.cc
+++ b/chrome/browser/renderer_host/test/render_view_host_manager_browsertest.cc
@@ -3,9 +3,10 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/browser.h"
-#include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/renderer_host/site_instance.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/notification_details.h"
@@ -25,6 +26,135 @@ class RenderViewHostManagerTest : public InProcessBrowserTest {
   }
 };
 
+// Test for crbug.com/24447.  Following a cross-site link with rel=noreferrer
+// and target=_blank should create a new SiteInstance.
+IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
+                       SwapProcessWithRelNoreferrerAndTargetBlank) {
+  // Start two servers with different sites.
+  const wchar_t kDocRoot[] = L"chrome/test/data";
+  scoped_refptr<HTTPTestServer> http_server =
+      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  scoped_refptr<HTTPSTestServer> https_server =
+      HTTPSTestServer::CreateGoodServer(kDocRoot);
+
+  // Load a page with links that open in a new window.
+  ui_test_utils::NavigateToURL(browser(), http_server->TestServerPageW(
+      L"files/click-noreferrer-links.html"));
+
+  // Get the original SiteInstance for later comparison.
+  scoped_refptr<SiteInstance> orig_site_instance(
+      browser()->GetSelectedTabContents()->GetSiteInstance());
+  EXPECT_TRUE(orig_site_instance != NULL);
+
+  // Test clicking a rel=noreferrer + target=blank link.
+  bool success = false;
+  EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+      browser()->GetSelectedTabContents()->render_view_host(), L"",
+      L"window.domAutomationController.send(clickNoRefTargetBlankLink());",
+      &success));
+  EXPECT_TRUE(success);
+  // Wait for the cross-site transition to finish.
+  ui_test_utils::WaitForLoadStop(
+      &(browser()->GetSelectedTabContents()->controller()));
+
+  // Opens in new tab.
+  EXPECT_EQ(2, browser()->tab_count());
+  EXPECT_EQ(1, browser()->selected_index());
+  EXPECT_EQ(L"Title Of Awesomeness",
+      browser()->GetSelectedTabContents()->GetTitle());
+
+  // Should have a new SiteInstance.
+  scoped_refptr<SiteInstance> noref_blank_site_instance(
+      browser()->GetSelectedTabContents()->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,
+                       DontSwapProcessWithOnlyTargetBlank) {
+  // Start two servers with different sites.
+  const wchar_t kDocRoot[] = L"chrome/test/data";
+  scoped_refptr<HTTPTestServer> http_server =
+      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  scoped_refptr<HTTPSTestServer> https_server =
+      HTTPSTestServer::CreateGoodServer(kDocRoot);
+
+  // Load a page with links that open in a new window.
+  ui_test_utils::NavigateToURL(browser(), http_server->TestServerPageW(
+      L"files/click-noreferrer-links.html"));
+
+  // Get the original SiteInstance for later comparison.
+  scoped_refptr<SiteInstance> orig_site_instance(
+      browser()->GetSelectedTabContents()->GetSiteInstance());
+  EXPECT_TRUE(orig_site_instance != NULL);
+
+  // Test clicking a target=blank link.
+  bool success = false;
+  EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+      browser()->GetSelectedTabContents()->render_view_host(), L"",
+      L"window.domAutomationController.send(clickTargetBlankLink());",
+      &success));
+  EXPECT_TRUE(success);
+  // Wait for the cross-site transition to finish.
+  ui_test_utils::WaitForLoadStop(
+      &(browser()->GetSelectedTabContents()->controller()));
+
+  // Opens in new tab.
+  EXPECT_EQ(2, browser()->tab_count());
+  EXPECT_EQ(1, browser()->selected_index());
+  EXPECT_EQ(L"Title Of Awesomeness",
+      browser()->GetSelectedTabContents()->GetTitle());
+
+  // Should have the same SiteInstance.
+  scoped_refptr<SiteInstance> blank_site_instance(
+      browser()->GetSelectedTabContents()->GetSiteInstance());
+  EXPECT_EQ(orig_site_instance, blank_site_instance);
+}
+
+// Test for crbug.com/24447.  Following a cross-site link with rel=noreferrer
+// and no target=_blank should not create a new SiteInstance.
+IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
+                       DontSwapProcessWithOnlyRelNoreferrer) {
+  // Start two servers with different sites.
+  const wchar_t kDocRoot[] = L"chrome/test/data";
+  scoped_refptr<HTTPTestServer> http_server =
+      HTTPTestServer::CreateServer(kDocRoot, NULL);
+  scoped_refptr<HTTPSTestServer> https_server =
+      HTTPSTestServer::CreateGoodServer(kDocRoot);
+
+  // Load a page with links that open in a new window.
+  ui_test_utils::NavigateToURL(browser(), http_server->TestServerPageW(
+      L"files/click-noreferrer-links.html"));
+
+  // Get the original SiteInstance for later comparison.
+  scoped_refptr<SiteInstance> orig_site_instance(
+      browser()->GetSelectedTabContents()->GetSiteInstance());
+  EXPECT_TRUE(orig_site_instance != NULL);
+
+  // Test clicking a rel=noreferrer link.
+  bool success = false;
+  EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+      browser()->GetSelectedTabContents()->render_view_host(), L"",
+      L"window.domAutomationController.send(clickNoRefLink());",
+      &success));
+  EXPECT_TRUE(success);
+  // Wait for the cross-site transition to finish.
+  ui_test_utils::WaitForLoadStop(
+      &(browser()->GetSelectedTabContents()->controller()));
+
+  // Opens in same tab.
+  EXPECT_EQ(1, browser()->tab_count());
+  EXPECT_EQ(0, browser()->selected_index());
+  EXPECT_EQ(L"Title Of Awesomeness",
+      browser()->GetSelectedTabContents()->GetTitle());
+
+  // Should have the same SiteInstance.
+  scoped_refptr<SiteInstance> noref_site_instance(
+      browser()->GetSelectedTabContents()->GetSiteInstance());
+  EXPECT_EQ(orig_site_instance, noref_site_instance);
+}
+
 // Test for crbug.com/14505. This tests that chrome:// urls are still functional
 // after download of a file while viewing another chrome://.
 IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 3bde88e..2cc7ad1 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -217,6 +217,7 @@ RenderView::RenderView(RenderThreadBase* render_thread,
       last_page_id_sent_to_browser_(-1),
       last_indexed_page_id_(-1),
       opened_by_user_gesture_(true),
+      opener_suppressed_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
       devtools_agent_(NULL),
       devtools_client_(NULL),
@@ -1289,6 +1290,7 @@ WebView* RenderView::createView(WebFrame* creator) {
 
   int32 routing_id = MSG_ROUTING_NONE;
   bool user_gesture = creator->isProcessingUserGesture();
+  bool opener_suppressed = creator->willSuppressOpenerInNewFrame();
 
   render_thread_->Send(
       new ViewHostMsg_CreateWindow(routing_id_, user_gesture, &routing_id));
@@ -1304,6 +1306,9 @@ WebView* RenderView::createView(WebFrame* creator) {
                                         routing_id);
   view->opened_by_user_gesture_ = user_gesture;
 
+  // Record whether the creator frame is trying to suppress the opener field.
+  view->opener_suppressed_ = opener_suppressed;
+
   // Record the security origin of the creator.
   GURL creator_url(creator->securityOrigin().toString().utf8());
   if (!creator_url.is_valid() || !creator_url.IsStandard())
@@ -1887,6 +1892,9 @@ WebNavigationPolicy RenderView::decidePolicyForNavigation(
   // The parent page must open a new tab to about:blank, set the new tab's
   // window.opener to null, and then redirect the tab to a cross-site URL using
   // JavaScript.
+  //
+  // TODO(creis): Deprecate this logic once we can rely on rel=noreferrer
+  // (see below).
   bool is_fork =
       // Must start from a tab showing about:blank, which is later redirected.
       GURL(frame->url()) == GURL(chrome::kAboutBlankURL) &&
@@ -1904,7 +1912,34 @@ WebNavigationPolicy RenderView::decidePolicyForNavigation(
       default_policy == WebKit::WebNavigationPolicyCurrentTab &&
       // Must be a JavaScript navigation, which appears as "other".
       type == WebKit::WebNavigationTypeOther;
-  if (is_fork) {
+
+  // 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->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) {
     // Open the URL via the browser, not via WebKit.
     OpenURL(url, GURL(), default_policy);
     return WebKit::WebNavigationPolicyIgnore;
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index b77dc2a..790bfd5 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -813,6 +813,11 @@ class RenderView : public RenderWidget,
   // The alternate error page URL, if one exists.
   GURL alternate_error_page_url_;
 
+  // Whether this RenderView was created by a frame that was suppressing its
+  // opener. If so, we may want to load pages in a separate process.  See
+  // decidePolicyForNavigation for details.
+  bool opener_suppressed_;
+
   ScopedRunnableMethodFactory<RenderView> method_factory_;
 
   // Timer used to delay the updating of nav state (see SyncNavigationState).
diff --git a/chrome/test/data/click-noreferrer-links.html b/chrome/test/data/click-noreferrer-links.html
new file mode 100644
index 0000000..f6ae295
--- /dev/null
+++ b/chrome/test/data/click-noreferrer-links.html
@@ -0,0 +1,37 @@
+<html>
+
+ <head><title>Click noreferrer links</title>
+ <script>
+  function simulateClick(target) {
+    var evt = document.createEvent("MouseEvents");
+    evt.initMouseEvent("click", true, true, window,
+                       0, 0, 0, 0, 0, false, false,
+                       false, false, 0, null);
+
+    return target.dispatchEvent(evt);
+  }
+
+  function clickNoRefTargetBlankLink() {
+    return simulateClick(document.getElementById("noref_and_tblank_link"));
+  }
+
+  function clickTargetBlankLink() {
+    return simulateClick(document.getElementById("tblank_link"));
+  }
+
+  function clickNoRefLink() {
+    return simulateClick(document.getElementById("noref_link"));
+  }
+
+ </script>
+ </head>
+
+<a href="https://127.0.0.1:9443/files/title2.html" id="noref_and_tblank_link"
+   rel="noreferrer" target="_blank">rel=noreferrer and target=_blank</a><br>
+<a href="https://127.0.0.1:9443/files/title2.html" id="tblank_link"
+   target="_blank">target=_blank</a><br>
+<a href="https://127.0.0.1:9443/files/title2.html" id="noref_link"
+   rel="noreferrer">rel=noreferrer</a><br>
+
+</html>
+
diff --git a/webkit/api/public/WebFrame.h b/webkit/api/public/WebFrame.h
index 04c862d..965a510 100644
--- a/webkit/api/public/WebFrame.h
+++ b/webkit/api/public/WebFrame.h
@@ -303,6 +303,10 @@ namespace WebKit {
         // Returns true if a user gesture is currently being processed.
         virtual bool isProcessingUserGesture() const = 0;
 
+        // Returns true if this frame is in the process of opening a new frame
+        // with a suppressed opener.
+        virtual bool willSuppressOpenerInNewFrame() const = 0;
+
 
         // Editing -------------------------------------------------------------
 
diff --git a/webkit/glue/webframe_impl.cc b/webkit/glue/webframe_impl.cc
index 29a2339..6eab6a1 100644
--- a/webkit/glue/webframe_impl.cc
+++ b/webkit/glue/webframe_impl.cc
@@ -914,6 +914,10 @@ bool WebFrameImpl::isProcessingUserGesture() const {
   return frame()->loader()->isProcessingUserGesture();
 }
 
+bool WebFrameImpl::willSuppressOpenerInNewFrame() const {
+  return frame()->loader()->suppressOpenerInNewFrame();
+}
+
 void WebFrameImpl::replaceSelection(const WebString& wtext) {
   String text = webkit_glue::WebStringToString(wtext);
   RefPtr<DocumentFragment> fragment = createFragmentFromText(
diff --git a/webkit/glue/webframe_impl.h b/webkit/glue/webframe_impl.h
index f15b3a1..f1d0548 100644
--- a/webkit/glue/webframe_impl.h
+++ b/webkit/glue/webframe_impl.h
@@ -126,6 +126,7 @@ class WebFrameImpl : public WebKit::WebFrame, public RefCounted<WebFrameImpl> {
   virtual void commitDocumentData(const char* data, size_t length);
   virtual unsigned unloadListenerCount() const;
   virtual bool isProcessingUserGesture() const;
+  virtual bool willSuppressOpenerInNewFrame() const;
   virtual void replaceSelection(const WebKit::WebString& text);
   virtual void insertText(const WebKit::WebString& text);
   virtual void setMarkedText(