diff options
author | creis@google.com <creis@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-22 17:53:25 +0000 |
---|---|---|
committer | creis@google.com <creis@google.com@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-22 17:53:25 +0000 |
commit | 2b74c093728f3bf35a0dc6b444bde69074f99f5d (patch) | |
tree | 8878e66d5b532d77a72c78579f744a3f45e12890 | |
parent | 2461cf0a67fec9bf61c5c80ca64a9a234c0d0e8d (diff) | |
download | chromium_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.cc | 92 | ||||
-rw-r--r-- | chrome/renderer/render_view.cc | 37 | ||||
-rw-r--r-- | chrome/renderer/render_view.h | 5 | ||||
-rw-r--r-- | chrome/test/data/click-noreferrer-links.html | 37 | ||||
-rw-r--r-- | webkit/api/public/WebFrame.h | 4 | ||||
-rw-r--r-- | webkit/glue/webframe_impl.cc | 4 | ||||
-rw-r--r-- | webkit/glue/webframe_impl.h | 1 |
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( |