summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorcreis@google.com <creis@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2009-10-22 17:53:25 +0000
committercreis@google.com <creis@google.com@0039d316-1c4b-4281-b951-d872f2087c98>2009-10-22 17:53:25 +0000
commit2b74c093728f3bf35a0dc6b444bde69074f99f5d (patch)
tree8878e66d5b532d77a72c78579f744a3f45e12890
parent2461cf0a67fec9bf61c5c80ca64a9a234c0d0e8d (diff)
downloadchromium_src-2b74c093728f3bf35a0dc6b444bde69074f99f5d.zip
chromium_src-2b74c093728f3bf35a0dc6b444bde69074f99f5d.tar.gz
chromium_src-2b74c093728f3bf35a0dc6b444bde69074f99f5d.tar.bz2
Swaps renderer processes on links with rel=noreferrer and target=_blank.
We now detect these types of navigations in RenderView and allow the browser process to handle them. This allows us to swap process if the navigation is cross-site. Requires exposing suppressOpenerInNewFrame in WebFrame to accurately detect these links. BUG=24447 TEST=RenderViewHostManagerTest.SwapProcessOnRelNoreferrerWithTargetBlank Review URL: http://codereview.chromium.org/284015 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@29783 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/renderer_host/test/render_view_host_manager_browsertest.cc92
-rw-r--r--chrome/renderer/render_view.cc37
-rw-r--r--chrome/renderer/render_view.h5
-rw-r--r--chrome/test/data/click-noreferrer-links.html37
-rw-r--r--webkit/api/public/WebFrame.h4
-rw-r--r--webkit/glue/webframe_impl.cc4
-rw-r--r--webkit/glue/webframe_impl.h1
7 files changed, 178 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..6e713cc 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,95 @@ 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. Links with either
+// rel=noreferrer or target=_blank (but not both) should not create a new
+// SiteInstance.
+IN_PROC_BROWSER_TEST_F(RenderViewHostManagerTest,
+ SwapProcessOnRelNoreferrerWithTargetBlank) {
+ // 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);
+
+ // 1. 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);
+ // Opens in new tab.
+ EXPECT_EQ(2, browser()->tab_count());
+ EXPECT_EQ(1, browser()->selected_index());
+
+ // Wait for the cross-site transition to finish.
+ ui_test_utils::WaitForLoadStop(
+ &(browser()->GetSelectedTabContents()->controller()));
+
+ // Should have a new SiteInstance.
+ scoped_refptr<SiteInstance> noref_blank_site_instance(
+ browser()->GetSelectedTabContents()->GetSiteInstance());
+ EXPECT_NE(orig_site_instance, noref_blank_site_instance);
+
+ // Close the tab to try another link.
+ browser()->CloseTab();
+
+ // 2. Test clicking a target=blank link.
+ success = false;
+ EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+ browser()->GetSelectedTabContents()->render_view_host(), L"",
+ L"window.domAutomationController.send(clickTargetBlankLink());",
+ &success));
+ EXPECT_TRUE(success);
+ // Opens in new tab.
+ EXPECT_EQ(2, browser()->tab_count());
+ EXPECT_EQ(1, browser()->selected_index());
+
+ // Wait for the cross-site transition to finish.
+ ui_test_utils::WaitForLoadStop(
+ &(browser()->GetSelectedTabContents()->controller()));
+
+ // Should have the same SiteInstance.
+ scoped_refptr<SiteInstance> blank_site_instance(
+ browser()->GetSelectedTabContents()->GetSiteInstance());
+ EXPECT_EQ(orig_site_instance, blank_site_instance);
+
+ // Close the tab to try another link.
+ browser()->CloseTab();
+
+ // 3. Test clicking a rel=noreferrer link.
+ success = false;
+ EXPECT_TRUE(ui_test_utils::ExecuteJavaScriptAndExtractBool(
+ browser()->GetSelectedTabContents()->render_view_host(), L"",
+ L"window.domAutomationController.send(clickNoRefLink());",
+ &success));
+ EXPECT_TRUE(success);
+ // Opens in same tab.
+ EXPECT_EQ(1, browser()->tab_count());
+ EXPECT_EQ(0, browser()->selected_index());
+
+ // Wait for the cross-site transition to finish.
+ ui_test_utils::WaitForLoadStop(
+ &(browser()->GetSelectedTabContents()->controller()));
+
+ // 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 d25f439..4e37403 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),
@@ -1293,6 +1294,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));
@@ -1308,6 +1310,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())
@@ -1886,6 +1891,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) &&
@@ -1903,7 +1911,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 c63ca9f..e75eb32 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..a066355
--- /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/title1.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/title1.html" id="tblank_link"
+ target="_blank">target=_blank</a><br>
+<a href="https://127.0.0.1:9443/files/title1.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 08b7281..de4f772 100644
--- a/webkit/glue/webframe_impl.cc
+++ b/webkit/glue/webframe_impl.cc
@@ -880,6 +880,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 8ec1f9f..d2ee953 100644
--- a/webkit/glue/webframe_impl.h
+++ b/webkit/glue/webframe_impl.h
@@ -130,6 +130,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(