diff options
author | nick <nick@chromium.org> | 2015-07-27 14:51:08 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-07-27 21:51:38 +0000 |
commit | d30fd969388ba0ffe122eb63c22776aa02a8836f (patch) | |
tree | e2117fa68c6350c6b5c1dcc7abed29eca33efe4e | |
parent | 17de7455a3a1d5e4556d1ed18961e42d987e36c2 (diff) | |
download | chromium_src-d30fd969388ba0ffe122eb63c22776aa02a8836f.zip chromium_src-d30fd969388ba0ffe122eb63c22776aa02a8836f.tar.gz chromium_src-d30fd969388ba0ffe122eb63c22776aa02a8836f.tar.bz2 |
Move existing kSitePerProcess checks to a policy-oracle object
Introduces SiteIsolationPolicy, which interprets the kSitePerProcess
switch (and eventually others too), in order to make decisions about
oopifs, oopif-related features, and site isolation policy.
Replace explicit calls to HasSwitch(content::kSitePerProcess) with
calls to appropriate methods of SiteIsolationPolicy,
BrowserPluginGuestMode, or content's browser_test_utils.
SiteIsolationPolicy is content-internal, and I expect it eventually
to become a stateful object.
There are six cases:
1. SiteIsolationPolicy::DoesSiteRequireDedicatedProcess(url) This
anticipates site isolation being launched for a subset of
sites/schemes.
2. BrowserPluginGuestMode::UseCrossProcessFramesForGuests() Tracks
some current feature work that requires out of process iframes and
so piggybacks on --site-per-process. We ought to control this by a
different flag
3. SiteIsolationPolicy::AreCrossProcessFramesPossible() For dchecks
and determining whether to create proxies -- basically it is the
"or" of all of the above functions.
4. SiteIsolationPolicy::UseSubframeNavigationEntries() Tracks some
current feature work related to navigation, that's tied to --site-
per-process. Expected to be shortlived.
5. IsSwappedOutStateForbidden() (on RFHM/RFProxy) Another class of
temporary feature work.
6. content::AreAllSitesIsolatedForTesting() For bailing out of
tests.
BUG=481066
Review URL: https://codereview.chromium.org/1208143002
Cr-Commit-Position: refs/heads/master@{#340570}
50 files changed, 486 insertions, 386 deletions
diff --git a/chrome/browser/chrome_content_browser_client_browsertest.cc b/chrome/browser/chrome_content_browser_client_browsertest.cc index 46bcbd6..5268dcc 100644 --- a/chrome/browser/chrome_content_browser_client_browsertest.cc +++ b/chrome/browser/chrome_content_browser_client_browsertest.cc @@ -11,7 +11,7 @@ #include "content/public/browser/navigation_controller.h" #include "content/public/browser/navigation_entry.h" #include "content/public/browser/web_contents.h" -#include "content/public/common/content_switches.h" +#include "content/public/test/browser_test_utils.h" #include "url/gurl.h" namespace content { @@ -87,8 +87,7 @@ IN_PROC_BROWSER_TEST_F(ChromeContentBrowserClientBrowserTest, // such as http://crbug.com/164223. IN_PROC_BROWSER_TEST_F(ChromeContentBrowserClientBrowserTest, SitePerProcessNavigation) { - base::CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kSitePerProcess); + content::IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess()); ASSERT_TRUE(test_server()->Start()); const GURL url(test_server()->GetURL("files/title1.html")); diff --git a/chrome/browser/chrome_site_per_process_browsertest.cc b/chrome/browser/chrome_site_per_process_browsertest.cc index 10e8783..120ab10 100644 --- a/chrome/browser/chrome_site_per_process_browsertest.cc +++ b/chrome/browser/chrome_site_per_process_browsertest.cc @@ -14,7 +14,6 @@ #include "content/public/browser/render_frame_host.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_observer.h" -#include "content/public/common/content_switches.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/content_browser_test_utils.h" #include "content/public/test/test_utils.h" @@ -28,7 +27,7 @@ class ChromeSitePerProcessTest : public InProcessBrowserTest { ~ChromeSitePerProcessTest() override {} void SetUpCommandLine(base::CommandLine* command_line) override { - command_line->AppendSwitch(switches::kSitePerProcess); + content::IsolateAllSitesForTesting(command_line); } void SetUpOnMainThread() override { diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc index 7514427..9539515 100644 --- a/chrome/browser/extensions/app_process_apitest.cc +++ b/chrome/browser/extensions/app_process_apitest.cc @@ -775,9 +775,7 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, OpenWebPopupFromWebIframe) { active_browser_list->get(1)->tab_strip_model()->GetActiveWebContents(); content::WaitForLoadStop(popup_contents); - bool should_be_in_same_process = - !base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); + bool should_be_in_same_process = !content::AreAllSitesIsolatedForTesting(); content::RenderProcessHost* popup_process = popup_contents->GetRenderProcessHost(); EXPECT_EQ(should_be_in_same_process, process == popup_process); diff --git a/chrome/browser/password_manager/password_manager_browsertest.cc b/chrome/browser/password_manager/password_manager_browsertest.cc index 0d0f9c3..915796b 100644 --- a/chrome/browser/password_manager/password_manager_browsertest.cc +++ b/chrome/browser/password_manager/password_manager_browsertest.cc @@ -1970,8 +1970,7 @@ IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase, IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase, CrossSitePasswordEnforcement) { // The code under test is only active under site isolation. - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - ::switches::kSitePerProcess)) { + if (!content::AreAllSitesIsolatedForTesting()) { return; } diff --git a/chrome/browser/task_manager/task_manager_browsertest.cc b/chrome/browser/task_manager/task_manager_browsertest.cc index 3c27c47..e11ad15 100644 --- a/chrome/browser/task_manager/task_manager_browsertest.cc +++ b/chrome/browser/task_manager/task_manager_browsertest.cc @@ -38,7 +38,6 @@ #include "content/public/browser/notification_service.h" #include "content/public/browser/page_navigator.h" #include "content/public/browser/render_frame_host.h" -#include "content/public/common/content_switches.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/content_browser_test_utils.h" #include "extensions/browser/extension_system.h" @@ -159,12 +158,11 @@ class TaskManagerOOPIFBrowserTest : public TaskManagerBrowserTest, void SetUpCommandLine(base::CommandLine* command_line) override { TaskManagerBrowserTest::SetUpCommandLine(command_line); if (GetParam()) - command_line->AppendSwitch(switches::kSitePerProcess); + content::IsolateAllSitesForTesting(command_line); } bool ShouldExpectSubframes() { - return base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); + return content::AreAllSitesIsolatedForTesting(); } private: diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc index 9413042..29fe11c 100644 --- a/chrome/renderer/chrome_content_renderer_client.cc +++ b/chrome/renderer/chrome_content_renderer_client.cc @@ -1395,6 +1395,8 @@ bool ChromeContentRendererClient::CrossesExtensionExtents( // the type of process. In default Chrome, that's the URL of the opener's // top frame and not the opener frame itself. In --site-per-process, we // can use the opener frame itself. + // TODO(nick): Either wire this up to SiteIsolationPolicy, or to state on + // |opener_frame|/its ancestors. if (base::CommandLine::ForCurrentProcess()->HasSwitch( switches::kSitePerProcess)) old_url = opener_frame->document().url(); diff --git a/components/guest_view/browser/guest_view_base.cc b/components/guest_view/browser/guest_view_base.cc index ef65c87..f0bf741 100644 --- a/components/guest_view/browser/guest_view_base.cc +++ b/components/guest_view/browser/guest_view_base.cc @@ -4,7 +4,6 @@ #include "components/guest_view/browser/guest_view_base.h" -#include "base/command_line.h" #include "base/lazy_instance.h" #include "base/strings/utf_string_conversions.h" #include "components/guest_view/browser/guest_view_event.h" @@ -19,7 +18,7 @@ #include "content/public/browser/render_view_host.h" #include "content/public/browser/render_widget_host_view.h" #include "content/public/browser/web_contents.h" -#include "content/public/common/content_switches.h" +#include "content/public/common/browser_plugin_guest_mode.h" #include "content/public/common/page_zoom.h" #include "content/public/common/url_constants.h" #include "third_party/WebKit/public/web/WebInputEvent.h" @@ -557,8 +556,7 @@ void GuestViewBase::DidNavigateMainFrame( // TODO(lazyboy): This breaks guest visibility in --site-per-process because // we do not take the widget's visibility into account. We need to also // stay hidden during "visibility:none" state. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (content::BrowserPluginGuestMode::UseCrossProcessFramesForGuests()) { web_contents()->WasShown(); } } diff --git a/components/guest_view/renderer/iframe_guest_view_container.cc b/components/guest_view/renderer/iframe_guest_view_container.cc index f267383..36ca0b6 100644 --- a/components/guest_view/renderer/iframe_guest_view_container.cc +++ b/components/guest_view/renderer/iframe_guest_view_container.cc @@ -4,9 +4,8 @@ #include "components/guest_view/renderer/iframe_guest_view_container.h" -#include "base/command_line.h" #include "components/guest_view/common/guest_view_messages.h" -#include "content/public/common/content_switches.h" +#include "content/public/common/browser_plugin_guest_mode.h" #include "content/public/renderer/render_frame.h" namespace guest_view { @@ -14,8 +13,7 @@ namespace guest_view { IframeGuestViewContainer::IframeGuestViewContainer( content::RenderFrame* render_frame) : GuestViewContainer(render_frame) { - CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)); + CHECK(content::BrowserPluginGuestMode::UseCrossProcessFramesForGuests()); // There is no BrowserPluginDelegate to wait for. ready_ = true; } diff --git a/content/browser/browser_plugin/browser_plugin_guest.cc b/content/browser/browser_plugin/browser_plugin_guest.cc index 54c3f5d..1bf8368 100644 --- a/content/browser/browser_plugin/browser_plugin_guest.cc +++ b/content/browser/browser_plugin/browser_plugin_guest.cc @@ -6,7 +6,6 @@ #include <algorithm> -#include "base/command_line.h" #include "base/message_loop/message_loop.h" #include "base/pickle.h" #include "base/strings/utf_string_conversions.h" @@ -38,7 +37,7 @@ #include "content/public/browser/render_widget_host_view.h" #include "content/public/browser/user_metrics.h" #include "content/public/browser/web_contents_observer.h" -#include "content/public/common/content_switches.h" +#include "content/public/common/browser_plugin_guest_mode.h" #include "content/public/common/drop_data.h" #include "ui/gfx/geometry/size_conversions.h" @@ -109,8 +108,7 @@ BrowserPluginGuest::BrowserPluginGuest(bool has_render_view, } int BrowserPluginGuest::GetGuestProxyRoutingID() { - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (BrowserPluginGuestMode::UseCrossProcessFramesForGuests()) { // We don't use the proxy to send postMessage in --site-per-process, since // we use the contentWindow directly from the frame element instead. return MSG_ROUTING_NONE; @@ -278,8 +276,7 @@ void BrowserPluginGuest::InitInternal( if (owner_web_contents_ != owner_web_contents) { WebContentsViewGuest* new_view = nullptr; - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (!BrowserPluginGuestMode::UseCrossProcessFramesForGuests()) { new_view = static_cast<WebContentsViewGuest*>(GetWebContents()->GetView()); } @@ -626,8 +623,7 @@ bool BrowserPluginGuest::OnMessageReceived(const IPC::Message& message) { // TODO(lazyboy): Fix this as part of http://crbug.com/330264. The required // parts of code from this class should be extracted to a separate class for // --site-per-process. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (BrowserPluginGuestMode::UseCrossProcessFramesForGuests()) { return false; } @@ -711,12 +707,12 @@ void BrowserPluginGuest::Attach( void BrowserPluginGuest::OnWillAttachComplete( WebContentsImpl* embedder_web_contents, const BrowserPluginHostMsg_Attach_Params& params) { - bool use_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); + bool use_cross_process_frames = + BrowserPluginGuestMode::UseCrossProcessFramesForGuests(); // If a RenderView has already been created for this new window, then we need // to initialize the browser-side state now so that the RenderFrameHostManager // does not create a new RenderView on navigation. - if (!use_site_per_process && has_render_view_) { + if (!use_cross_process_frames && has_render_view_) { // This will trigger a callback to RenderViewReady after a round-trip IPC. static_cast<RenderViewHostImpl*>( GetWebContents()->GetRenderViewHost())->Init(); @@ -736,7 +732,7 @@ void BrowserPluginGuest::OnWillAttachComplete( delegate_->DidAttach(GetGuestProxyRoutingID()); - if (!use_site_per_process) + if (!use_cross_process_frames) has_render_view_ = true; RecordAction(base::UserMetricsAction("BrowserPlugin.Guest.Attached")); diff --git a/content/browser/child_process_security_policy_impl.cc b/content/browser/child_process_security_policy_impl.cc index 8a67465..7c96e7b 100644 --- a/content/browser/child_process_security_policy_impl.cc +++ b/content/browser/child_process_security_policy_impl.cc @@ -12,11 +12,11 @@ #include "base/strings/string_util.h" #include "content/browser/plugin_process_host.h" #include "content/browser/site_instance_impl.h" +#include "content/common/site_isolation_policy.h" #include "content/public/browser/child_process_data.h" #include "content/public/browser/content_browser_client.h" #include "content/public/browser/render_process_host.h" #include "content/public/common/bindings_policy.h" -#include "content/public/common/content_switches.h" #include "content/public/common/url_constants.h" #include "net/base/filename_util.h" #include "net/url_request/url_request.h" diff --git a/content/browser/cross_site_transfer_browsertest.cc b/content/browser/cross_site_transfer_browsertest.cc index 6b54a51..0ce755c 100644 --- a/content/browser/cross_site_transfer_browsertest.cc +++ b/content/browser/cross_site_transfer_browsertest.cc @@ -9,7 +9,6 @@ #include "content/public/browser/resource_dispatcher_host_delegate.h" #include "content/public/browser/resource_throttle.h" #include "content/public/browser/web_contents.h" -#include "content/public/common/content_switches.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/content_browser_test.h" #include "content/public/test/content_browser_test_utils.h" @@ -209,8 +208,7 @@ class CrossSiteTransferTest : public ContentBrowserTest { } void SetUpCommandLine(base::CommandLine* command_line) override { - // Use --site-per-process to force process swaps for cross-site transfers. - command_line->AppendSwitch(switches::kSitePerProcess); + IsolateAllSitesForTesting(command_line); } void InjectResourceDisptcherHostDelegate() { diff --git a/content/browser/frame_host/frame_tree.cc b/content/browser/frame_host/frame_tree.cc index 2bc147d..544fd5a 100644 --- a/content/browser/frame_host/frame_tree.cc +++ b/content/browser/frame_host/frame_tree.cc @@ -8,7 +8,6 @@ #include "base/bind.h" #include "base/callback.h" -#include "base/command_line.h" #include "base/containers/hash_tables.h" #include "base/lazy_instance.h" #include "content/browser/frame_host/frame_tree_node.h" @@ -18,7 +17,6 @@ #include "content/browser/frame_host/render_frame_proxy_host.h" #include "content/browser/renderer_host/render_view_host_factory.h" #include "content/browser/renderer_host/render_view_host_impl.h" -#include "content/public/common/content_switches.h" #include "third_party/WebKit/public/web/WebSandboxFlags.h" namespace content { diff --git a/content/browser/frame_host/frame_tree_browsertest.cc b/content/browser/frame_host/frame_tree_browsertest.cc index 8f35488..a0450de 100644 --- a/content/browser/frame_host/frame_tree_browsertest.cc +++ b/content/browser/frame_host/frame_tree_browsertest.cc @@ -2,14 +2,12 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/command_line.h" #include "content/browser/frame_host/frame_tree.h" #include "content/browser/frame_host/frame_tree_node.h" #include "content/browser/renderer_host/render_view_host_impl.h" #include "content/browser/web_contents/web_contents_impl.h" #include "content/public/browser/notification_service.h" #include "content/public/browser/notification_types.h" -#include "content/public/common/content_switches.h" #include "content/public/common/url_constants.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/content_browser_test.h" @@ -319,7 +317,7 @@ class CrossProcessFrameTreeBrowserTest : public ContentBrowserTest { CrossProcessFrameTreeBrowserTest() {} void SetUpCommandLine(base::CommandLine* command_line) override { - command_line->AppendSwitch(switches::kSitePerProcess); + IsolateAllSitesForTesting(command_line); } void SetUpOnMainThread() override { diff --git a/content/browser/frame_host/frame_tree_node.cc b/content/browser/frame_host/frame_tree_node.cc index 67d9b83..1f1b9dc 100644 --- a/content/browser/frame_host/frame_tree_node.cc +++ b/content/browser/frame_host/frame_tree_node.cc @@ -15,6 +15,7 @@ #include "content/browser/frame_host/render_frame_host_impl.h" #include "content/browser/renderer_host/render_view_host_impl.h" #include "content/common/frame_messages.h" +#include "content/common/site_isolation_policy.h" #include "content/public/browser/browser_thread.h" #include "content/public/common/content_switches.h" @@ -140,8 +141,8 @@ void FrameTreeNode::AddChild(scoped_ptr<FrameTreeNode> child, // about the new frame. Create a proxy for the child frame in all // SiteInstances that have a proxy for the frame's parent, since all frames // in a frame tree should have the same set of proxies. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) + // TODO(alexmos, nick): We ought to do this for non-oopif too, for openers. + if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) render_manager_.CreateProxiesForChildFrame(child.get()); children_.push_back(child.release()); diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc index 59a07a2..01da9c1 100644 --- a/content/browser/frame_host/navigation_controller_impl.cc +++ b/content/browser/frame_host/navigation_controller_impl.cc @@ -59,6 +59,7 @@ #include "content/browser/renderer_host/render_view_host_impl.h" // Temporary #include "content/browser/site_instance_impl.h" #include "content/common/frame_messages.h" +#include "content/common/site_isolation_policy.h" #include "content/common/ssl_status_serialization.h" #include "content/common/view_messages.h" #include "content/public/browser/browser_context.h" @@ -73,7 +74,6 @@ #include "content/public/browser/user_metrics.h" #include "content/public/common/content_client.h" #include "content/public/common/content_constants.h" -#include "content/public/common/content_switches.h" #include "media/base/mime_util.h" #include "net/base/escape.h" #include "net/base/net_util.h" @@ -739,8 +739,7 @@ void NavigationControllerImpl::LoadURLWithParams(const LoadURLParams& params) { // In --site-per-process, create an identical NavigationEntry with a // new FrameNavigationEntry for the target subframe. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { entry = GetLastCommittedEntry()->Clone(); entry->SetPageID(-1); entry->AddOrUpdateFrameEntry(node, -1, -1, nullptr, params.url, @@ -893,8 +892,7 @@ bool NavigationControllerImpl::RendererDidNavigate( NavigationEntryImpl* active_entry = GetLastCommittedEntry(); active_entry->SetTimestamp(timestamp); active_entry->SetHttpStatusCode(params.http_status_code); - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // Update the frame-specific PageState. FrameNavigationEntry* frame_entry = active_entry->GetFrameEntry(rfh->frame_tree_node()); @@ -1225,8 +1223,7 @@ void NavigationControllerImpl::RendererDidNavigateNewSubframe( << "that a last committed entry exists."; scoped_ptr<NavigationEntryImpl> new_entry; - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // Make sure new_entry takes ownership of frame_entry in a scoped_refptr. FrameNavigationEntry* frame_entry = new FrameNavigationEntry( rfh->frame_tree_node()->frame_tree_node_id(), @@ -1281,8 +1278,7 @@ bool NavigationControllerImpl::RendererDidNavigateAutoSubframe( } } - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // This may be a "new auto" case where we add a new FrameNavigationEntry, or // it may be a "history auto" case where we update an existing one. NavigationEntryImpl* last_committed = GetLastCommittedEntry(); @@ -1746,8 +1742,7 @@ bool NavigationControllerImpl::NavigateToPendingEntryInternal( // In default Chrome, there are no subframe FrameNavigationEntries. Either // navigate the main frame or use the main frame's FrameNavigationEntry to // tell the indicated frame where to go. - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (!SiteIsolationPolicy::UseSubframeNavigationEntries()) { FrameNavigationEntry* frame_entry = GetPendingEntry()->GetFrameEntry(root); FrameTreeNode* frame = root; int ftn_id = GetPendingEntry()->frame_tree_node_id(); diff --git a/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/content/browser/frame_host/navigation_controller_impl_browsertest.cc index 541aaa5..301f27a 100644 --- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc +++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include "base/bind.h" -#include "base/command_line.h" #include "base/strings/stringprintf.h" #include "base/strings/utf_string_conversions.h" #include "content/browser/frame_host/frame_navigation_entry.h" @@ -11,6 +10,7 @@ #include "content/browser/frame_host/navigation_controller_impl.h" #include "content/browser/frame_host/navigation_entry_impl.h" #include "content/browser/web_contents/web_contents_impl.h" +#include "content/common/site_isolation_policy.h" #include "content/public/browser/render_view_host.h" #include "content/public/browser/resource_controller.h" #include "content/public/browser/resource_dispatcher_host.h" @@ -19,7 +19,6 @@ #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/common/bindings_policy.h" -#include "content/public/common/content_switches.h" #include "content/public/common/url_constants.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/content_browser_test.h" @@ -64,9 +63,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, LoadCrossSiteSubframe) { // We should only have swapped processes in --site-per-process. bool cross_process = root->current_frame_host()->GetProcess() != root->child_at(0)->current_frame_host()->GetProcess(); - EXPECT_EQ(base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess), - cross_process); + EXPECT_EQ(AreAllSitesIsolatedForTesting(), cross_process); } IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, LoadDataWithBaseURL) { @@ -1159,9 +1156,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, FrameNavigationEntry* root_entry = entry->root_node()->frame_entry.get(); EXPECT_EQ(main_url, root_entry->url()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should now have one blank subframe FrameNavigationEntry, but // this does not count as committing a real load. ASSERT_EQ(1U, entry->root_node()->children.size()); @@ -1186,9 +1182,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type()); } - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The nested entry should have one blank subframe FrameNavigationEntry, but // this does not count as committing a real load. ASSERT_EQ(1U, entry->root_node()->children[0]->children.size()); @@ -1216,9 +1211,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(1, controller.GetEntryCount()); EXPECT_EQ(entry, controller.GetLastCommittedEntry()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The new entry should have one blank subframe FrameNavigationEntry, but // this does not count as committing a real load. ASSERT_EQ(2U, entry->root_node()->children.size()); @@ -1249,9 +1243,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(1, controller.GetEntryCount()); EXPECT_EQ(entry, controller.GetLastCommittedEntry()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should still have one nested subframe FrameNavigationEntry. ASSERT_EQ(1U, entry->root_node()->children[0]->children.size()); FrameNavigationEntry* frame_entry = @@ -1281,9 +1274,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(1, controller.GetEntryCount()); EXPECT_EQ(entry, controller.GetLastCommittedEntry()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should still have two subframe FrameNavigationEntries. ASSERT_EQ(2U, entry->root_node()->children.size()); FrameNavigationEntry* frame_entry = @@ -1315,9 +1307,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_NE(entry, controller.GetLastCommittedEntry()); NavigationEntryImpl* entry2 = controller.GetLastCommittedEntry(); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { ASSERT_EQ(2U, entry->root_node()->children.size()); FrameNavigationEntry* frame_entry = entry2->root_node()->children[0]->children[0]->frame_entry.get(); @@ -1331,8 +1322,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_TRUE(root->child_at(1)->has_committed_real_load()); // Check the end result of the frame tree. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (AreAllSitesIsolatedForTesting()) { FrameTreeVisualizer visualizer; EXPECT_EQ( " Site A ------------ proxies for B\n" @@ -1381,9 +1371,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(main_url, root_entry->url()); EXPECT_FALSE(controller.GetPendingEntry()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should now have a subframe FrameNavigationEntry. ASSERT_EQ(1U, entry->root_node()->children.size()); FrameNavigationEntry* frame_entry = @@ -1416,9 +1405,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(main_url, root_entry->url()); EXPECT_FALSE(controller.GetPendingEntry()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should now have 2 subframe FrameNavigationEntries. ASSERT_EQ(2U, entry->root_node()->children.size()); FrameNavigationEntry* frame_entry = @@ -1449,9 +1437,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, root_entry = entry->root_node()->frame_entry.get(); EXPECT_EQ(main_url, root_entry->url()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should now have 2 subframe FrameNavigationEntries. ASSERT_EQ(2U, entry->root_node()->children.size()); ASSERT_EQ(1U, entry->root_node()->children[1]->children.size()); @@ -1482,9 +1469,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, root_entry = entry->root_node()->frame_entry.get(); EXPECT_EQ(main_url, root_entry->url()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should now have 3 subframe FrameNavigationEntries. ASSERT_EQ(3U, entry->root_node()->children.size()); FrameNavigationEntry* frame_entry = @@ -1514,9 +1500,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, root_entry = entry->root_node()->frame_entry.get(); EXPECT_EQ(main_url, root_entry->url()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // There should be a corresponding FrameNavigationEntry. ASSERT_EQ(1U, entry->root_node()->children[2]->children.size()); FrameNavigationEntry* frame_entry = @@ -1528,8 +1513,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, } // Check the end result of the frame tree. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (AreAllSitesIsolatedForTesting()) { FrameTreeVisualizer visualizer; EXPECT_EQ( " Site A ------------ proxies for B\n" @@ -1592,9 +1576,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, FrameNavigationEntry* root_entry2 = entry2->root_node()->frame_entry.get(); EXPECT_EQ(main_url, root_entry2->url()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should have a new FrameNavigationEntries for the subframe. ASSERT_EQ(1U, entry2->root_node()->children.size()); EXPECT_EQ(frame_url2, entry2->root_node()->children[0]->frame_entry->url()); @@ -1646,9 +1629,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, FrameNavigationEntry* root_entry3 = entry3->root_node()->frame_entry.get(); EXPECT_EQ(main_url, root_entry3->url()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should still have FrameNavigationEntries for all 3 subframes. ASSERT_EQ(2U, entry3->root_node()->children.size()); EXPECT_EQ(frame_url2, entry3->root_node()->children[0]->frame_entry->url()); @@ -1685,9 +1667,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, FrameNavigationEntry* root_entry4 = entry4->root_node()->frame_entry.get(); EXPECT_EQ(main_url, root_entry4->url()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should still have FrameNavigationEntries for all 3 subframes. ASSERT_EQ(2U, entry4->root_node()->children.size()); EXPECT_EQ(frame_url2, entry4->root_node()->children[0]->frame_entry->url()); @@ -1699,8 +1680,7 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, } // Check the end result of the frame tree. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (AreAllSitesIsolatedForTesting()) { FrameTreeVisualizer visualizer; EXPECT_EQ( " Site A ------------ proxies for B\n" @@ -1775,9 +1755,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(1, controller.GetLastCommittedEntryIndex()); EXPECT_EQ(entry2, controller.GetLastCommittedEntry()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should have a new FrameNavigationEntries for the subframe. ASSERT_EQ(1U, entry2->root_node()->children.size()); EXPECT_EQ(frame_url2, entry2->root_node()->children[0]->frame_entry->url()); @@ -1798,9 +1777,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(0, controller.GetLastCommittedEntryIndex()); EXPECT_EQ(entry1, controller.GetLastCommittedEntry()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should have a new FrameNavigationEntries for the subframe. ASSERT_EQ(1U, entry1->root_node()->children.size()); EXPECT_EQ(frame_url, entry1->root_node()->children[0]->frame_entry->url()); @@ -1821,9 +1799,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(1, controller.GetLastCommittedEntryIndex()); EXPECT_EQ(entry2, controller.GetLastCommittedEntry()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should have a new FrameNavigationEntries for the subframe. ASSERT_EQ(1U, entry2->root_node()->children.size()); EXPECT_EQ(frame_url2, entry2->root_node()->children[0]->frame_entry->url()); @@ -1844,9 +1821,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_EQ(2, controller.GetLastCommittedEntryIndex()); EXPECT_EQ(entry3, controller.GetLastCommittedEntry()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should have a new FrameNavigationEntries for the subframe. ASSERT_EQ(1U, entry3->root_node()->children.size()); EXPECT_EQ(frame_url3, entry3->root_node()->children[0]->frame_entry->url()); @@ -1891,10 +1867,8 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, EXPECT_NE(isn_1, isn_2); EXPECT_EQ(dsn_1, dsn_2); - // Also test subframe sequence numbers, but only in --site-per-proces mode. - // (We do not create subframe FrameNavigationEntries in default mode yet.) - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) + // Test subframe sequence numbers only if enabled, e.g. in --site-per-process. + if (!SiteIsolationPolicy::UseSubframeNavigationEntries()) return; // 3. Add a subframe, which does an AUTO_SUBFRAME navigation. diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc index 5989bb4..d5099408 100644 --- a/content/browser/frame_host/navigation_controller_impl_unittest.cc +++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc @@ -22,6 +22,7 @@ #include "content/browser/site_instance_impl.h" #include "content/browser/web_contents/web_contents_impl.h" #include "content/common/frame_messages.h" +#include "content/common/site_isolation_policy.h" #include "content/common/ssl_status_serialization.h" #include "content/common/view_messages.h" #include "content/public/browser/navigation_details.h" @@ -2083,9 +2084,8 @@ TEST_F(NavigationControllerTest, NewSubframe) { EXPECT_EQ(url1, entry->GetURL()); EXPECT_EQ(params.page_id, entry->GetPageID()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should have a subframe FrameNavigationEntry. ASSERT_EQ(1U, entry->root_node()->children.size()); EXPECT_EQ(url2, entry->root_node()->children[0]->frame_entry->url()); @@ -2141,9 +2141,8 @@ TEST_F(NavigationControllerTest, AutoSubframe) { FrameNavigationEntry* root_entry = entry->root_node()->frame_entry.get(); EXPECT_EQ(url1, root_entry->url()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should now have a subframe FrameNavigationEntry. ASSERT_EQ(1U, entry->root_node()->children.size()); FrameNavigationEntry* frame_entry = @@ -2187,9 +2186,8 @@ TEST_F(NavigationControllerTest, AutoSubframe) { EXPECT_EQ(root_entry, entry->root_node()->frame_entry.get()); EXPECT_EQ(url1, root_entry->url()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should now have 2 subframe FrameNavigationEntries. ASSERT_EQ(2U, entry->root_node()->children.size()); FrameNavigationEntry* new_frame_entry = @@ -2237,9 +2235,8 @@ TEST_F(NavigationControllerTest, AutoSubframe) { EXPECT_EQ(root_entry, entry->root_node()->frame_entry.get()); EXPECT_EQ(url1, root_entry->url()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should now have a nested FrameNavigationEntry. EXPECT_EQ(2U, entry->root_node()->children.size()); ASSERT_EQ(1U, entry->root_node()->children[0]->children.size()); @@ -2311,9 +2308,8 @@ TEST_F(NavigationControllerTest, BackSubframe) { navigation_entry_committed_counter_ = 0; EXPECT_EQ(2, controller.GetEntryCount()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should have a subframe FrameNavigationEntry. ASSERT_EQ(1U, entry2->root_node()->children.size()); EXPECT_EQ(url2, entry2->root_node()->children[0]->frame_entry->url()); @@ -2336,9 +2332,8 @@ TEST_F(NavigationControllerTest, BackSubframe) { EXPECT_EQ(3, controller.GetEntryCount()); EXPECT_EQ(2, controller.GetCurrentEntryIndex()); - // Verify subframe entries if we're in --site-per-process mode. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + // Verify subframe entries if they're enabled (e.g. in --site-per-process). + if (SiteIsolationPolicy::UseSubframeNavigationEntries()) { // The entry should have a subframe FrameNavigationEntry. ASSERT_EQ(1U, entry3->root_node()->children.size()); EXPECT_EQ(url3, entry3->root_node()->children[0]->frame_entry->url()); diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc index 5e54fdf..878c9b1 100644 --- a/content/browser/frame_host/navigator_impl.cc +++ b/content/browser/frame_host/navigator_impl.cc @@ -21,6 +21,7 @@ #include "content/browser/webui/web_ui_impl.h" #include "content/common/frame_messages.h" #include "content/common/navigation_params.h" +#include "content/common/site_isolation_policy.h" #include "content/common/view_messages.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/content_browser_client.h" @@ -71,8 +72,7 @@ FrameMsg_Navigate_Type::Value GetNavigationType( } RenderFrameHostManager* GetRenderManager(RenderFrameHostImpl* rfh) { - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) + if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) return rfh->frame_tree_node()->render_manager(); return rfh->frame_tree_node()->frame_tree()->root()->render_manager(); @@ -364,8 +364,7 @@ void NavigatorImpl::DidNavigate( const FrameHostMsg_DidCommitProvisionalLoad_Params& input_params) { FrameHostMsg_DidCommitProvisionalLoad_Params params(input_params); FrameTree* frame_tree = render_frame_host->frame_tree_node()->frame_tree(); - bool use_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); + bool oopifs_possible = SiteIsolationPolicy::AreCrossProcessFramesPossible(); if (ui::PageTransitionIsMainFrame(params.transition)) { if (delegate_) { @@ -390,7 +389,7 @@ void NavigatorImpl::DidNavigate( delegate_->DidNavigateMainFramePreCommit(is_navigation_within_page); } - if (!use_site_per_process) + if (!oopifs_possible) frame_tree->root()->render_manager()->DidNavigateFrame( render_frame_host, params.gesture == NavigationGestureUser); } @@ -405,7 +404,7 @@ void NavigatorImpl::DidNavigate( // When using --site-per-process, we notify the RFHM for all navigations, // not just main frame navigations. - if (use_site_per_process) { + if (oopifs_possible) { FrameTreeNode* frame = render_frame_host->frame_tree_node(); frame->render_manager()->DidNavigateFrame( render_frame_host, params.gesture == NavigationGestureUser); @@ -554,10 +553,8 @@ void NavigatorImpl::RequestTransferURL( // Send the navigation to the current FrameTreeNode if it's destined for a // subframe in the current tab. We'll assume it's for the main frame // (possibly of a new or different WebContents) otherwise. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess) && - disposition == CURRENT_TAB && - render_frame_host->GetParent()) { + if (SiteIsolationPolicy::AreCrossProcessFramesPossible() && + disposition == CURRENT_TAB && render_frame_host->GetParent()) { frame_tree_node_id = render_frame_host->frame_tree_node()->frame_tree_node_id(); } diff --git a/content/browser/frame_host/navigator_impl_unittest.cc b/content/browser/frame_host/navigator_impl_unittest.cc index a600dbf..e03f62a 100644 --- a/content/browser/frame_host/navigator_impl_unittest.cc +++ b/content/browser/frame_host/navigator_impl_unittest.cc @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/command_line.h" #include "base/macros.h" #include "base/time/time.h" #include "content/browser/frame_host/navigation_controller_impl.h" @@ -17,9 +16,9 @@ #include "content/common/frame_messages.h" #include "content/common/navigation_params.h" #include "content/public/browser/stream_handle.h" -#include "content/public/common/content_switches.h" #include "content/public/common/url_constants.h" #include "content/public/common/url_utils.h" +#include "content/public/test/browser_test_utils.h" #include "content/public/test/mock_render_process_host.h" #include "content/test/browser_side_navigation_test_utils.h" #include "content/test/test_navigation_url_loader.h" @@ -318,8 +317,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, BeginNavigation) { // Subframe navigations should never create a speculative RenderFrameHost, // unless site-per-process is enabled. In that case, as the subframe // navigation is to a different site and is still ongoing, it should have one. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (AreAllSitesIsolatedForTesting()) { EXPECT_TRUE(GetSpeculativeRenderFrameHost(subframe_node)); } else { EXPECT_FALSE(GetSpeculativeRenderFrameHost(subframe_node)); @@ -353,8 +351,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, BeginNavigation) { // As the main frame hasn't yet committed the subframe still exists. Thus, the // above situation regarding subframe navigations is valid here. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (AreAllSitesIsolatedForTesting()) { EXPECT_TRUE(GetSpeculativeRenderFrameHost(subframe_node)); } else { EXPECT_FALSE(GetSpeculativeRenderFrameHost(subframe_node)); diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index 4377622..4835e88 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc @@ -46,6 +46,7 @@ #include "content/common/inter_process_time_ticks_converter.h" #include "content/common/navigation_params.h" #include "content/common/render_frame_setup.mojom.h" +#include "content/common/site_isolation_policy.h" #include "content/common/swapped_out_messages.h" #include "content/public/browser/ax_event_notification_details.h" #include "content/public/browser/browser_accessibility_state.h" @@ -1353,8 +1354,7 @@ void RenderFrameHostImpl::OnBeginNavigation( } void RenderFrameHostImpl::OnDispatchLoad() { - CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)); + CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible()); // Only frames with an out-of-process parent frame should be sending this // message. RenderFrameProxyHost* proxy = diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc index d02466c..1889033 100644 --- a/content/browser/frame_host/render_frame_host_manager.cc +++ b/content/browser/frame_host/render_frame_host_manager.cc @@ -31,6 +31,7 @@ #include "content/browser/webui/web_ui_controller_factory_registry.h" #include "content/browser/webui/web_ui_impl.h" #include "content/common/frame_messages.h" +#include "content/common/site_isolation_policy.h" #include "content/common/view_messages.h" #include "content/public/browser/content_browser_client.h" #include "content/public/browser/render_process_host_observer.h" @@ -38,6 +39,7 @@ #include "content/public/browser/render_widget_host_view.h" #include "content/public/browser/user_metrics.h" #include "content/public/browser/web_ui_controller.h" +#include "content/public/common/browser_plugin_guest_mode.h" #include "content/public/common/content_switches.h" #include "content/public/common/referrer.h" #include "content/public/common/url_constants.h" @@ -168,8 +170,7 @@ bool RenderFrameHostManager::ClearRFHsPendingShutdown(FrameTreeNode* node) { // static bool RenderFrameHostManager::IsSwappedOutStateForbidden() { - return base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); + return SiteIsolationPolicy::AreCrossProcessFramesPossible(); } RenderFrameHostManager::RenderFrameHostManager( @@ -908,12 +909,11 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation( // TODO(carlosk): Have renderer-initated main frame navigations swap processes // if needed when it no longer breaks OAuth popups (see // https://crbug.com/440266). - bool site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); bool is_main_frame = frame_tree_node_->IsMainFrame(); if (current_site_instance == dest_site_instance.get() || (!request.browser_initiated() && is_main_frame) || - (!is_main_frame && !site_per_process)) { + (!is_main_frame && !dest_site_instance->RequiresDedicatedProcess() && + !current_site_instance->RequiresDedicatedProcess())) { // Reuse the current RFH if its SiteInstance matches the the navigation's // or if this is a subframe navigation. We only swap RFHs for subframes when // --site-per-process is enabled. @@ -1026,8 +1026,9 @@ void RenderFrameHostManager::OnDidUpdateName(const std::string& name) { // The window.name message may be sent outside of --site-per-process when // report_frame_name_changes renderer preference is set (used by // WebView). Don't send the update to proxies in those cases. - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) + // TODO(nick,nasko): Should this be IsSwappedOutStateForbidden, to match + // OnDidUpdateOrigin? + if (!SiteIsolationPolicy::AreCrossProcessFramesPossible()) return; for (const auto& pair : *proxy_hosts_) { @@ -1092,16 +1093,23 @@ bool RenderFrameHostManager::ResetProxiesInSiteInstance(int32 site_instance_id, } bool RenderFrameHostManager::ShouldTransitionCrossSite() { - // True for --site-per-process, which overrides both kSingleProcess and - // kProcessPerTab. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) + // The logic below is weaker than "are all sites isolated" -- it asks instead, + // "is any site isolated". That's appropriate here since we're just trying to + // figure out if we're in any kind of site isolated mode, and in which case, + // we ignore the kSingleProcess and kProcessPerTab settings. + // + // TODO(nick): Move all handling of kSingleProcess/kProcessPerTab into + // SiteIsolationPolicy so we have a consistent behavior around the interaction + // of the process model flags. + if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) return true; // False in the single-process mode, as it makes RVHs to accumulate // in swapped_out_hosts_. // True if we are using process-per-site-instance (default) or // process-per-site (kProcessPerSite). + // TODO(nick): Move handling of kSingleProcess and kProcessPerTab into + // SiteIsolationPolicy. return !base::CommandLine::ForCurrentProcess()->HasSwitch( switches::kSingleProcess) && !base::CommandLine::ForCurrentProcess()->HasSwitch( @@ -1446,8 +1454,7 @@ const GURL& RenderFrameHostManager::GetCurrentURLForSiteInstance( // TODO(creis): Remove this when we can check the FrameNavigationEntry's url. // See http://crbug.com/369654 if (!frame_tree_node_->IsMainFrame() && - base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) + SiteIsolationPolicy::AreCrossProcessFramesPossible()) return frame_tree_node_->current_url(); // If there is no last non-interstitial entry (and current_instance already @@ -1501,8 +1508,7 @@ void RenderFrameHostManager::CreateProxiesForNewRenderFrameHost( // Only create opener proxies if they are in the same BrowsingInstance. if (new_instance->IsRelatedSiteInstance(old_instance)) CreateOpenerProxiesIfNeeded(new_instance); - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) { // Ensure that the frame tree has RenderFrameProxyHosts for the new // SiteInstance in all nodes except the current one. We do this for all // frames in the tree, whether they are in the same BrowsingInstance or not. @@ -1521,10 +1527,7 @@ void RenderFrameHostManager::CreateProxiesForNewRenderFrameHost( } void RenderFrameHostManager::CreateProxiesForNewNamedFrame() { - // TODO(alexmos): use SiteIsolationPolicy::AreCrossProcessFramesPossible once - // https://codereview.chromium.org/1208143002/ lands. - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) + if (!SiteIsolationPolicy::AreCrossProcessFramesPossible()) return; DCHECK(!frame_tree_node_->frame_name().empty()); @@ -1631,12 +1634,11 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame( int* view_routing_id_ptr) { bool swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT); bool swapped_out_forbidden = IsSwappedOutStateForbidden(); - bool is_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); CHECK(instance); CHECK_IMPLIES(swapped_out_forbidden, !swapped_out); - CHECK_IMPLIES(!is_site_per_process, frame_tree_node_->IsMainFrame()); + CHECK_IMPLIES(!SiteIsolationPolicy::AreCrossProcessFramesPossible(), + frame_tree_node_->IsMainFrame()); // Swapped out views should always be hidden. DCHECK_IMPLIES(swapped_out, (flags & CREATE_RF_HIDDEN)); @@ -1814,8 +1816,7 @@ void RenderFrameHostManager::EnsureRenderViewInitialized( void RenderFrameHostManager::CreateOuterDelegateProxy( SiteInstance* outer_contents_site_instance, RenderFrameHostImpl* render_frame_host) { - CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)); + CHECK(BrowserPluginGuestMode::UseCrossProcessFramesForGuests()); RenderFrameProxyHost* proxy = new RenderFrameProxyHost( outer_contents_site_instance, nullptr, frame_tree_node_); proxy_hosts_->Add(outer_contents_site_instance->GetId(), @@ -2085,18 +2086,17 @@ void RenderFrameHostManager::CommitPending() { proxy_hosts_->Remove(render_frame_host_->GetSiteInstance()->GetId()); } - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { - // If this is a subframe, it should have a CrossProcessFrameConnector - // created already. Use it to link the new RFH's view to the proxy that - // belongs to the parent frame's SiteInstance. If this navigation causes - // an out-of-process frame to return to the same process as its parent, the - // proxy would have been removed from proxy_hosts_ above. - // Note: We do this after swapping out the old RFH because that may create - // the proxy we're looking for. - RenderFrameProxyHost* proxy_to_parent = GetProxyToParent(); - if (proxy_to_parent) - proxy_to_parent->SetChildRWHView(render_frame_host_->GetView()); + // If this is a subframe, it should have a CrossProcessFrameConnector + // created already. Use it to link the new RFH's view to the proxy that + // belongs to the parent frame's SiteInstance. If this navigation causes + // an out-of-process frame to return to the same process as its parent, the + // proxy would have been removed from proxy_hosts_ above. + // Note: We do this after swapping out the old RFH because that may create + // the proxy we're looking for. + RenderFrameProxyHost* proxy_to_parent = GetProxyToParent(); + if (proxy_to_parent) { + CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible()); + proxy_to_parent->SetChildRWHView(render_frame_host_->GetView()); } // After all is done, there must never be a proxy in the list which has the @@ -2156,9 +2156,9 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate( // Don't swap for subframes unless we are in --site-per-process. We can get // here in tests for subframes (e.g., NavigateFrameToURL). if (!frame_tree_node_->IsMainFrame() && - !base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) + !SiteIsolationPolicy::AreCrossProcessFramesPossible()) { return render_frame_host_.get(); + } // If we are currently navigating cross-process, we want to get back to normal // and then navigate as usual. diff --git a/content/browser/frame_host/render_frame_host_manager_browsertest.cc b/content/browser/frame_host/render_frame_host_manager_browsertest.cc index 19e81a0..7ad6055 100644 --- a/content/browser/frame_host/render_frame_host_manager_browsertest.cc +++ b/content/browser/frame_host/render_frame_host_manager_browsertest.cc @@ -328,11 +328,10 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostManagerTest, // Should have the same SiteInstance unless we're in site-per-process mode. scoped_refptr<SiteInstance> blank_site_instance( new_shell->web_contents()->GetSiteInstance()); - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) - EXPECT_EQ(orig_site_instance, blank_site_instance); - else + if (AreAllSitesIsolatedForTesting()) EXPECT_NE(orig_site_instance, blank_site_instance); + else + EXPECT_EQ(orig_site_instance, blank_site_instance); } // Test for crbug.com/24447. Following a cross-site link with rel=noreferrer @@ -373,11 +372,10 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostManagerTest, // Should have the same SiteInstance unless we're in site-per-process mode. scoped_refptr<SiteInstance> noref_site_instance( shell()->web_contents()->GetSiteInstance()); - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) - EXPECT_EQ(orig_site_instance, noref_site_instance); - else + if (AreAllSitesIsolatedForTesting()) EXPECT_NE(orig_site_instance, noref_site_instance); + else + EXPECT_EQ(orig_site_instance, noref_site_instance); } // Test for crbug.com/116192. Targeted links should still work after the diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc index 46bbf97..193ca58 100644 --- a/content/browser/frame_host/render_frame_host_manager_unittest.cc +++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc @@ -18,6 +18,7 @@ #include "content/browser/site_instance_impl.h" #include "content/browser/webui/web_ui_controller_factory_registry.h" #include "content/common/frame_messages.h" +#include "content/common/site_isolation_policy.h" #include "content/common/view_messages.h" #include "content/public/browser/notification_details.h" #include "content/public/browser/notification_service.h" @@ -2035,8 +2036,7 @@ TEST_F(RenderFrameHostManagerTest, // deleted when the node is detached. Motivated by http://crbug.com/441357 and // http://crbug.com/444955. TEST_F(RenderFrameHostManagerTest, DetachPendingChild) { - base::CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kSitePerProcess); + IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess()); const GURL kUrlA("http://www.google.com/"); const GURL kUrlB("http://webkit.org/"); @@ -2159,8 +2159,7 @@ TEST_F(RenderFrameHostManagerTest, DetachPendingChild) { // tab navigates away without reloading. The second tab's navigation shouldn't // mess with the first tab's content. Motivated by http://crbug.com/473714. TEST_F(RenderFrameHostManagerTest, TwoTabsCrashOneReloadsOneLeaves) { - base::CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kSitePerProcess); + IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess()); const GURL kUrl1("http://www.google.com/"); const GURL kUrl2("http://webkit.org/"); diff --git a/content/browser/frame_host/render_widget_host_view_child_frame.cc b/content/browser/frame_host/render_widget_host_view_child_frame.cc index 30591a8..c8de21e 100644 --- a/content/browser/frame_host/render_widget_host_view_child_frame.cc +++ b/content/browser/frame_host/render_widget_host_view_child_frame.cc @@ -4,7 +4,6 @@ #include "content/browser/frame_host/render_widget_host_view_child_frame.h" -#include "base/command_line.h" #include "cc/surfaces/surface.h" #include "cc/surfaces/surface_factory.h" #include "cc/surfaces/surface_manager.h" @@ -19,7 +18,7 @@ #include "content/common/gpu/gpu_messages.h" #include "content/common/view_messages.h" #include "content/public/browser/render_process_host.h" -#include "content/public/common/content_switches.h" +#include "content/public/common/browser_plugin_guest_mode.h" namespace content { @@ -160,8 +159,7 @@ void RenderWidgetHostViewChildFrame::SetIsLoading(bool is_loading) { // is a RenderWidgetHostViewChildFrame. In contrast, when there is no // inner/outer WebContents, only subframe's RenderWidgetHostView can be a // RenderWidgetHostViewChildFrame which do not get a SetIsLoading() call. - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess) && + if (BrowserPluginGuestMode::UseCrossProcessFramesForGuests() && BrowserPluginGuest::IsGuest( static_cast<RenderViewHostImpl*>(RenderViewHost::From(host_)))) { return; diff --git a/content/browser/frame_host/render_widget_host_view_child_frame_browsertest.cc b/content/browser/frame_host/render_widget_host_view_child_frame_browsertest.cc index 3e36908..e560382 100644 --- a/content/browser/frame_host/render_widget_host_view_child_frame_browsertest.cc +++ b/content/browser/frame_host/render_widget_host_view_child_frame_browsertest.cc @@ -2,11 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "base/command_line.h" #include "content/browser/web_contents/web_contents_impl.h" #include "content/public/browser/render_frame_host.h" #include "content/public/browser/web_contents.h" -#include "content/public/common/content_switches.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/content_browser_test.h" #include "content/public/test/content_browser_test_utils.h" @@ -24,7 +22,7 @@ class RenderWidgetHostViewChildFrameTest : public ContentBrowserTest { RenderWidgetHostViewChildFrameTest() {} void SetUpCommandLine(base::CommandLine* command_line) override { - command_line->AppendSwitch(switches::kSitePerProcess); + IsolateAllSitesForTesting(command_line); } void SetUpOnMainThread() override { diff --git a/content/browser/loader/cross_site_resource_handler.cc b/content/browser/loader/cross_site_resource_handler.cc index cb344d4..ea69b318 100644 --- a/content/browser/loader/cross_site_resource_handler.cc +++ b/content/browser/loader/cross_site_resource_handler.cc @@ -17,6 +17,7 @@ #include "content/browser/loader/resource_request_info_impl.h" #include "content/browser/site_instance_impl.h" #include "content/browser/web_contents/web_contents_impl.h" +#include "content/common/site_isolation_policy.h" #include "content/public/browser/browser_thread.h" #include "content/public/browser/content_browser_client.h" #include "content/public/browser/global_request_id.h" @@ -72,8 +73,7 @@ void OnCrossSiteResponseHelper(const CrossSiteResponseParams& params) { // We should only swap processes for subframes in --site-per-process mode. // CrossSiteResourceHandler is not installed on subframe requests in // default Chrome. - CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)); + CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible()); } rfh->OnCrossSiteResponse( params.global_request_id, cross_site_transferring_request.Pass(), @@ -88,8 +88,7 @@ void OnCrossSiteResponseHelper(const CrossSiteResponseParams& params) { // Returns whether a transfer is needed by doing a check on the UI thread. bool CheckNavigationPolicyOnUI(GURL url, int process_id, int render_frame_id) { - CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)); + CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible()); RenderFrameHostImpl* rfh = RenderFrameHostImpl::FromID(process_id, render_frame_id); if (!rfh) @@ -106,12 +105,18 @@ bool CheckNavigationPolicyOnUI(GURL url, int process_id, int render_frame_id) { if (web_contents->GetBrowserPluginGuest()) return false; - // TODO(nasko): This check is very simplistic and is used temporarily only - // for --site-per-process. It should be updated to match the check performed - // by RenderFrameHostManager::UpdateStateForNavigate. - return !SiteInstance::IsSameWebSite( - rfh->GetSiteInstance()->GetBrowserContext(), - rfh->GetSiteInstance()->GetSiteURL(), url); + // TODO(nasko, nick): These following --site-per-process checks are + // overly simplistic. Update them to match all the cases + // considered by RenderFrameHostManager::DetermineSiteInstanceForURL. + if (SiteInstance::IsSameWebSite(rfh->GetSiteInstance()->GetBrowserContext(), + rfh->GetSiteInstance()->GetSiteURL(), url)) { + return false; // The same site, no transition needed. + } + + // The sites differ. If either one requires a dedicated process, + // then a transfer is needed. + return rfh->GetSiteInstance()->RequiresDedicatedProcess() || + SiteIsolationPolicy::DoesSiteRequireDedicatedProcess(url); } } // namespace @@ -166,12 +171,12 @@ bool CrossSiteResourceHandler::OnNormalResponseStarted( ResourceRequestInfoImpl* info = GetRequestInfo(); - // We only need to pause the response if a transfer to a different process is - // required. Other cross-process navigations can proceed immediately, since - // we run the unload handler at commit time. - // Note that a process swap may no longer be necessary if we transferred back - // into the original process due to a redirect. - bool should_transfer = + // The content embedder can decide that a transfer to a different process is + // required for this URL. If so, pause the response now. Other cross process + // navigations can proceed immediately, since we run the unload handler at + // commit time. Note that a process swap may no longer be necessary if we + // transferred back into the original process due to a redirect. + bool definitely_transfer = GetContentClient()->browser()->ShouldSwapProcessesForRedirect( info->GetContext(), request()->original_url(), request()->url()); @@ -196,30 +201,33 @@ bool CrossSiteResourceHandler::OnNormalResponseStarted( return next_handler_->OnResponseStarted(response, defer); } - // When the --site-per-process flag is passed, we transfer processes for - // cross-site navigations. This is skipped if a transfer is already required - // or for WebUI processes for now, since pages like the NTP host multiple - // cross-site WebUI iframes. - if (!should_transfer && - base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess) && + if (definitely_transfer) { + // Now that we know a transfer is needed and we have something to commit, we + // pause to let the UI thread set up the transfer. + StartCrossSiteTransition(response); + + // Defer loading until after the new renderer process has issued a + // corresponding request. + *defer = true; + OnDidDefer(); + return true; + } + + // In the site-per-process model, we may also decide (independently from the + // content embedder's ShouldSwapProcessesForRedirect decision above) that a + // process transfer is needed. For that we need to consult the navigation + // policy on the UI thread, so pause the response. Process transfers are + // skipped for WebUI processes for now, since e.g. chrome://settings has + // multiple "cross-site" chrome:// frames, and that doesn't yet work cross- + // process. + if (SiteIsolationPolicy::AreCrossProcessFramesPossible() && !ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings( info->GetChildID())) { return DeferForNavigationPolicyCheck(info, response, defer); } - if (!should_transfer) - return next_handler_->OnResponseStarted(response, defer); - - // Now that we know a transfer is needed and we have something to commit, we - // pause to let the UI thread set up the transfer. - StartCrossSiteTransition(response); - - // Defer loading until after the new renderer process has issued a - // corresponding request. - *defer = true; - OnDidDefer(); - return true; + // No deferral needed. Pass the response through. + return next_handler_->OnResponseStarted(response, defer); } void CrossSiteResourceHandler::ResumeOrTransfer(bool is_transfer) { diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc index 5912444..5600a23 100644 --- a/content/browser/loader/resource_dispatcher_host_impl.cc +++ b/content/browser/loader/resource_dispatcher_host_impl.cc @@ -61,6 +61,7 @@ #include "content/common/appcache_interfaces.h" #include "content/common/navigation_params.h" #include "content/common/resource_messages.h" +#include "content/common/site_isolation_policy.h" #include "content/common/ssl_status_serialization.h" #include "content/common/view_messages.h" #include "content/public/browser/browser_thread.h" @@ -1386,10 +1387,11 @@ scoped_ptr<ResourceHandler> ResourceDispatcherHostImpl::CreateResourceHandler( // to drive the transfer. bool is_swappable_navigation = request_data.resource_type == RESOURCE_TYPE_MAIN_FRAME; - // If we are using --site-per-process, install it for subframes as well. + // If out-of-process iframes are possible, then all subframe requests need + // to go through the CrossSiteResourceHandler to enforce the site isolation + // policy. if (!is_swappable_navigation && - base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + SiteIsolationPolicy::AreCrossProcessFramesPossible()) { is_swappable_navigation = request_data.resource_type == RESOURCE_TYPE_SUB_FRAME; } diff --git a/content/browser/renderer_host/render_message_filter_browsertest.cc b/content/browser/renderer_host/render_message_filter_browsertest.cc index f6ae7ae..ee42f40 100644 --- a/content/browser/renderer_host/render_message_filter_browsertest.cc +++ b/content/browser/renderer_host/render_message_filter_browsertest.cc @@ -106,8 +106,7 @@ IN_PROC_BROWSER_TEST_F(RenderMessageFilterBrowserTest, Cookies) { IN_PROC_BROWSER_TEST_F(RenderMessageFilterBrowserTest, CrossSiteCookieSecurityEnforcement) { // The code under test is only active under site isolation. - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (!AreAllSitesIsolatedForTesting()) { return; } diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc index ddb311a..4666d75 100644 --- a/content/browser/renderer_host/render_process_host_impl.cc +++ b/content/browser/renderer_host/render_process_host_impl.cc @@ -122,6 +122,7 @@ #include "content/common/mojo/channel_init.h" #include "content/common/mojo/mojo_messages.h" #include "content/common/resource_messages.h" +#include "content/common/site_isolation_policy.h" #include "content/common/view_messages.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/content_browser_client.h" @@ -1891,12 +1892,13 @@ bool RenderProcessHostImpl::IsSuitableHost( // Check whether the given host and the intended site_url will be using the // same StoragePartition, since a RenderProcessHost can only support a single - // StoragePartition. This is relevant for packaged apps and isolated sites. + // StoragePartition. This is relevant for packaged apps. StoragePartition* dest_partition = BrowserContext::GetStoragePartitionForSite(browser_context, site_url); if (!host->InSameStoragePartition(dest_partition)) return false; + // TODO(nick): Consult the SiteIsolationPolicy here. https://crbug.com/513036 if (ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings( host->GetID()) != WebUIControllerFactoryRegistry::GetInstance()->UseWebUIBindingsForURL( @@ -1948,13 +1950,12 @@ RenderProcessHost* RenderProcessHost::FromID(int render_process_id) { bool RenderProcessHost::ShouldTryToUseExistingProcessHost( BrowserContext* browser_context, const GURL& url) { // If --site-per-process is enabled, do not try to reuse renderer processes - // when over the limit. (We could allow pages from the same site to share, if - // we knew what the given process was dedicated to. Allowing no sharing is - // simpler for now.) This may cause resource exhaustion issues if too many - // sites are open at once. - const base::CommandLine& command_line = - *base::CommandLine::ForCurrentProcess(); - if (command_line.HasSwitch(switches::kSitePerProcess)) + // when over the limit. + // TODO(nick): This is overly conservative and isn't launchable. Move this + // logic into IsSuitableHost, and check |url| against the URL the process is + // dedicated to. This will allow pages from the same site to share, and will + // also allow non-isolated sites to share processes. https://crbug.com/513036 + if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) return false; if (run_renderer_in_process()) diff --git a/content/browser/renderer_host/render_process_host_unittest.cc b/content/browser/renderer_host/render_process_host_unittest.cc index 2b1517e..834c878 100644 --- a/content/browser/renderer_host/render_process_host_unittest.cc +++ b/content/browser/renderer_host/render_process_host_unittest.cc @@ -4,9 +4,8 @@ #include <limits> -#include "base/command_line.h" #include "content/public/common/content_constants.h" -#include "content/public/common/content_switches.h" +#include "content/public/test/browser_test_utils.h" #include "content/public/test/mock_render_process_host.h" #include "content/test/test_render_view_host.h" @@ -35,9 +34,7 @@ TEST_F(RenderProcessHostUnitTest, GuestsAreNotSuitableHosts) { TEST_F(RenderProcessHostUnitTest, RendererProcessLimit) { // This test shouldn't run with --site-per-process mode, which prohibits // the renderer process reuse this test explicitly exercises. - const base::CommandLine& command_line = - *base::CommandLine::ForCurrentProcess(); - if (command_line.HasSwitch(switches::kSitePerProcess)) + if (AreAllSitesIsolatedForTesting()) return; // Disable any overrides. diff --git a/content/browser/site_instance_impl.cc b/content/browser/site_instance_impl.cc index ad335f5..77f54bc 100644 --- a/content/browser/site_instance_impl.cc +++ b/content/browser/site_instance_impl.cc @@ -4,16 +4,15 @@ #include "content/browser/site_instance_impl.h" -#include "base/command_line.h" #include "content/browser/browsing_instance.h" #include "content/browser/child_process_security_policy_impl.h" #include "content/browser/frame_host/debug_urls.h" #include "content/browser/renderer_host/render_process_host_impl.h" #include "content/browser/storage_partition_impl.h" +#include "content/common/site_isolation_policy.h" #include "content/public/browser/content_browser_client.h" #include "content/public/browser/render_process_host_factory.h" #include "content/public/browser/web_ui_controller_factory.h" -#include "content/public/common/content_switches.h" #include "content/public/common/url_constants.h" #include "net/base/registry_controlled_domains/registry_controlled_domain.h" @@ -211,6 +210,12 @@ bool SiteInstanceImpl::HasWrongProcessForURL(const GURL& url) { GetProcess(), browsing_instance_->browser_context(), site_url); } +bool SiteInstanceImpl::RequiresDedicatedProcess() { + if (!has_site_) + return false; + return SiteIsolationPolicy::DoesSiteRequireDedicatedProcess(site_); +} + void SiteInstanceImpl::IncrementRelatedActiveContentsCount() { browsing_instance_->increment_active_contents_count(); } @@ -341,11 +346,11 @@ void SiteInstanceImpl::RenderProcessHostDestroyed(RenderProcessHost* host) { } void SiteInstanceImpl::LockToOrigin() { - // We currently only restrict this process to a particular site if --site-per- - // process flag is present. - const base::CommandLine& command_line = - *base::CommandLine::ForCurrentProcess(); - if (command_line.HasSwitch(switches::kSitePerProcess)) { + // TODO(nick): When all sites are isolated, this operation provides strong + // protection. If only some sites are isolated, we need additional logic to + // prevent the non-isolated sites from requesting resources for isolated + // sites. https://crbug.com/509125 + if (SiteIsolationPolicy::DoesSiteRequireDedicatedProcess(site_)) { // Guest processes cannot be locked to its site because guests always have // a fixed SiteInstance. The site of GURLs a guest loads doesn't match that // SiteInstance. So we skip locking the guest process to the site. diff --git a/content/browser/site_instance_impl.h b/content/browser/site_instance_impl.h index 2c55688..3bbaf33 100644 --- a/content/browser/site_instance_impl.h +++ b/content/browser/site_instance_impl.h @@ -27,6 +27,7 @@ class CONTENT_EXPORT SiteInstanceImpl : public SiteInstance, SiteInstance* GetRelatedSiteInstance(const GURL& url) override; bool IsRelatedSiteInstance(const SiteInstance* instance) override; size_t GetRelatedActiveContentsCount() override; + bool RequiresDedicatedProcess() override; // Set the web site that this SiteInstance is rendering pages for. // This includes the scheme and registered domain, but not the port. If the diff --git a/content/browser/site_instance_impl_unittest.cc b/content/browser/site_instance_impl_unittest.cc index 7267449e..40540d5 100644 --- a/content/browser/site_instance_impl_unittest.cc +++ b/content/browser/site_instance_impl_unittest.cc @@ -21,6 +21,7 @@ #include "content/public/common/content_switches.h" #include "content/public/common/url_constants.h" #include "content/public/common/url_utils.h" +#include "content/public/test/browser_test_utils.h" #include "content/public/test/mock_render_process_host.h" #include "content/public/test/test_browser_context.h" #include "content/public/test/test_browser_thread.h" @@ -569,9 +570,7 @@ static SiteInstanceImpl* CreateSiteInstance(BrowserContext* browser_context, TEST_F(SiteInstanceTest, ProcessSharingByType) { // This test shouldn't run with --site-per-process mode, which prohibits // the renderer process reuse this test explicitly exercises. - const base::CommandLine& command_line = - *base::CommandLine::ForCurrentProcess(); - if (command_line.HasSwitch(switches::kSitePerProcess)) + if (AreAllSitesIsolatedForTesting()) return; // On Android by default the number of renderer hosts is unlimited and process @@ -692,8 +691,7 @@ TEST_F(SiteInstanceTest, HasWrongProcessForURL) { // Test to ensure that HasWrongProcessForURL behaves properly even when // --site-per-process is used (http://crbug.com/160671). TEST_F(SiteInstanceTest, HasWrongProcessForURLInSitePerProcess) { - base::CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kSitePerProcess); + IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess()); scoped_ptr<TestBrowserContext> browser_context(new TestBrowserContext()); scoped_ptr<RenderProcessHost> host; diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc index 9271948..7b7de8c 100644 --- a/content/browser/site_per_process_browsertest.cc +++ b/content/browser/site_per_process_browsertest.cc @@ -291,7 +291,7 @@ void SitePerProcessBrowserTest::StartFrameAtDataURL() { void SitePerProcessBrowserTest::SetUpCommandLine( base::CommandLine* command_line) { - command_line->AppendSwitch(switches::kSitePerProcess); + IsolateAllSitesForTesting(command_line); }; void SitePerProcessBrowserTest::SetUpOnMainThread() { diff --git a/content/browser/web_contents/opened_by_dom_browsertest.cc b/content/browser/web_contents/opened_by_dom_browsertest.cc index f505afc..efca34f 100644 --- a/content/browser/web_contents/opened_by_dom_browsertest.cc +++ b/content/browser/web_contents/opened_by_dom_browsertest.cc @@ -6,7 +6,6 @@ #include "base/strings/stringprintf.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_delegate.h" -#include "content/public/common/content_switches.h" #include "content/public/test/browser_test_utils.h" #include "content/public/test/content_browser_test.h" #include "content/public/test/content_browser_test_utils.h" @@ -44,7 +43,7 @@ class OpenedByDOMTest : public ContentBrowserTest { protected: void SetUpCommandLine(base::CommandLine* command_line) override { // Use --site-per-process to force process swaps on cross-site navigations. - command_line->AppendSwitch(switches::kSitePerProcess); + IsolateAllSitesForTesting(command_line); } bool AttemptCloseFromJavaScript(WebContents* web_contents) { diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc index 951652a..41e49dd 100644 --- a/content/browser/web_contents/web_contents_impl.cc +++ b/content/browser/web_contents/web_contents_impl.cc @@ -88,6 +88,7 @@ #include "content/public/browser/web_contents_delegate.h" #include "content/public/browser/web_contents_observer.h" #include "content/public/common/bindings_policy.h" +#include "content/public/common/browser_plugin_guest_mode.h" #include "content/public/common/content_constants.h" #include "content/public/common/content_switches.h" #include "content/public/common/page_zoom.h" @@ -1238,8 +1239,7 @@ void WebContentsImpl::DispatchBeforeUnload(bool for_cross_site_transition) { void WebContentsImpl::AttachToOuterWebContentsFrame( WebContents* outer_web_contents, RenderFrameHost* outer_contents_frame) { - CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)); + CHECK(BrowserPluginGuestMode::UseCrossProcessFramesForGuests()); // Create a link to our outer WebContents. node_.reset(new WebContentsTreeNode()); node_->ConnectToOuterWebContents( @@ -1333,8 +1333,7 @@ void WebContentsImpl::Init(const WebContents::CreateParams& params) { GetContentClient()->browser()->GetWebContentsViewDelegate(this); if (browser_plugin_guest_ && - !base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + !BrowserPluginGuestMode::UseCrossProcessFramesForGuests()) { scoped_ptr<WebContentsView> platform_view(CreateWebContentsView( this, delegate, &render_view_host_delegate_view_)); @@ -1635,9 +1634,7 @@ void WebContentsImpl::CreateNewWindow( // SiteInstance in its own BrowsingInstance. bool is_guest = BrowserPluginGuest::IsGuest(this); - if (is_guest && - base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (is_guest && BrowserPluginGuestMode::UseCrossProcessFramesForGuests()) { // TODO(lazyboy): CreateNewWindow doesn't work for OOPIF-based <webview> // yet. NOTREACHED(); @@ -4051,8 +4048,7 @@ void WebContentsImpl::EnsureOpenerProxiesExist(RenderFrameHost* source_rfh) { // then we should not create a RenderView. AttachToOuterWebContentsFrame() // already created a RenderFrameProxyHost for that purpose. if (GetBrowserPluginEmbedder() && - base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + BrowserPluginGuestMode::UseCrossProcessFramesForGuests()) { return; } @@ -4277,8 +4273,7 @@ bool WebContentsImpl::CreateRenderViewForRenderManager( // frame RWHVs are unique in that they do not have their own WebContents. bool is_guest_in_site_per_process = !!browser_plugin_guest_.get() && - base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); + BrowserPluginGuestMode::UseCrossProcessFramesForGuests(); if (!for_main_frame_navigation || is_guest_in_site_per_process) { RenderWidgetHostViewChildFrame* rwh_view_child = new RenderWidgetHostViewChildFrame(render_view_host); diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc index d2300e9..380e4f2 100644 --- a/content/browser/web_contents/web_contents_impl_unittest.cc +++ b/content/browser/web_contents/web_contents_impl_unittest.cc @@ -32,6 +32,7 @@ #include "content/public/common/content_switches.h" #include "content/public/common/url_constants.h" #include "content/public/common/url_utils.h" +#include "content/public/test/browser_test_utils.h" #include "content/public/test/mock_render_process_host.h" #include "content/public/test/test_utils.h" #include "content/test/test_content_browser_client.h" @@ -2886,8 +2887,7 @@ TEST_F(WebContentsImplTest, StartStopEventsBalance) { // The bug manifests itself in regular mode as well, but browser-initiated // navigation of subframes is only possible in --site-per-process mode within // unit tests. - base::CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kSitePerProcess); + IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess()); const GURL initial_url("about:blank"); const GURL main_url("http://www.chromium.org"); const GURL foo_url("http://foo.chromium.org"); diff --git a/content/common/site_isolation_policy.cc b/content/common/site_isolation_policy.cc new file mode 100644 index 0000000..0aae959 --- /dev/null +++ b/content/common/site_isolation_policy.cc @@ -0,0 +1,30 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/common/site_isolation_policy.h" + +#include "base/command_line.h" +#include "content/public/common/content_switches.h" + +namespace content { + +// static +bool SiteIsolationPolicy::AreCrossProcessFramesPossible() { + return base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess); +} + +// static +bool SiteIsolationPolicy::DoesSiteRequireDedicatedProcess(const GURL& gurl) { + return base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess); +} + +// static +bool SiteIsolationPolicy::UseSubframeNavigationEntries() { + return base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess); +} + +} // namespace content diff --git a/content/common/site_isolation_policy.h b/content/common/site_isolation_policy.h new file mode 100644 index 0000000..07061dc --- /dev/null +++ b/content/common/site_isolation_policy.h @@ -0,0 +1,64 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CONTENT_COMMON_SITE_ISOLATION_POLICY_H_ +#define CONTENT_COMMON_SITE_ISOLATION_POLICY_H_ + +#include "base/basictypes.h" +#include "content/common/content_export.h" +#include "url/gurl.h" + +namespace content { + +// A centralized place for making policy decisions about out-of-process iframes, +// site isolation, --site-per-process, and related features. +// +// This is currently static because all these modes are controlled by command- +// line flags. +// +// These methods can be called from any thread. +class CONTENT_EXPORT SiteIsolationPolicy { + public: + // Returns true if the current process model might allow the use of cross- + // process iframes. This should typically used to avoid executing codepaths + // that only matter for cross-process iframes, to protect the default + // behavior. + // + // Note: Since cross-process frames will soon be possible by default (e.g. for + // <iframe src="http://..."> in an extension process), usage should be limited + // to temporary stop-gaps. + // + // Instead of calling this method, prefer to examine object state to see + // whether a particular frame happens to have a cross-process relationship + // with another, or to consult DoesSiteRequireDedicatedProcess() to see if a + // particular site merits protection. + static bool AreCrossProcessFramesPossible(); + + // Returns true if pages loaded from |url|'s site ought to be handled only by + // a renderer process isolated from other sites. If --site-per-process is on + // the command line, this is true for all sites. + // + // Eventually, this function will be made to return true for only some schemes + // (e.g. extensions) or a whitelist of sites that we should protect for this + // user. + // + // Although |url| is currently ignored, callers can assume for now that they + // can pass a full URL here -- they needn't canonicalize it to a site. + static bool DoesSiteRequireDedicatedProcess(const GURL& url); + + // Returns true if navigation and history code should maintain per-frame + // navigation entries. This is an in-progress feature related to site + // isolation, so the return value is currently tied to --site-per-process. + // TODO(creis, avi): Make this the default, and eliminate this. + static bool UseSubframeNavigationEntries(); + + private: + SiteIsolationPolicy(); // Not instantiable. + + DISALLOW_COPY_AND_ASSIGN(SiteIsolationPolicy); +}; + +} // namespace content + +#endif // CONTENT_COMMON_SITE_ISOLATION_POLICY_H_ diff --git a/content/content_common.gypi b/content/content_common.gypi index 296dc28..8b5a374 100644 --- a/content/content_common.gypi +++ b/content/content_common.gypi @@ -39,6 +39,8 @@ 'public_common_sources': [ 'public/common/appcache_info.h', 'public/common/bindings_policy.h', + 'public/common/browser_plugin_guest_mode.cc', + 'public/common/browser_plugin_guest_mode.h', 'public/common/child_process_host.h', 'public/common/child_process_host_delegate.cc', 'public/common/child_process_host_delegate.h', @@ -360,6 +362,8 @@ 'common/host_discardable_shared_memory_manager.h', 'common/host_shared_bitmap_manager.cc', 'common/host_shared_bitmap_manager.h', + 'common/in_process_child_thread_params.cc', + 'common/in_process_child_thread_params.h', 'common/indexed_db/indexed_db_constants.h', 'common/indexed_db/indexed_db_key.cc', 'common/indexed_db/indexed_db_key.h', @@ -407,8 +411,6 @@ 'common/input_messages.h', 'common/inter_process_time_ticks_converter.cc', 'common/inter_process_time_ticks_converter.h', - 'common/in_process_child_thread_params.cc', - 'common/in_process_child_thread_params.h', 'common/mac/attributed_string_coder.h', 'common/mac/attributed_string_coder.mm', 'common/mac/font_descriptor.h', @@ -454,8 +456,8 @@ 'common/net/url_request_user_data.h', 'common/one_writer_seqlock.cc', 'common/one_writer_seqlock.h', - 'common/p2p_messages.h', 'common/origin_util.cc', + 'common/p2p_messages.h', 'common/page_state_serialization.cc', 'common/page_state_serialization.h', 'common/page_zoom.cc', @@ -533,6 +535,8 @@ 'common/set_process_title.h', 'common/set_process_title_linux.cc', 'common/set_process_title_linux.h', + 'common/site_isolation_policy.cc', + 'common/site_isolation_policy.h', 'common/speech_recognition_messages.h', 'common/ssl_status_serialization.cc', 'common/ssl_status_serialization.h', diff --git a/content/public/browser/site_instance.h b/content/public/browser/site_instance.h index a828d29..7a30518 100644 --- a/content/public/browser/site_instance.h +++ b/content/public/browser/site_instance.h @@ -17,43 +17,52 @@ class RenderProcessHost; /////////////////////////////////////////////////////////////////////////////// // SiteInstance interface. // -// A SiteInstance represents a group of web pages that may be able to -// synchronously script each other, and thus must live in the same renderer -// process. +// A SiteInstance represents a group of web pages that must live in the same +// renderer process. Pages able to synchronously script each other will always +// be placed in the same SiteInstance. Pages unable to synchronously script +// each other may also be placed in the same SiteInstance, as determined by the +// process model. // -// We identify this group using a combination of where the page comes from -// (the site) and which tabs have references to each other (the instance). -// Here, a "site" is similar to the page's origin, but it only includes the -// registered domain name and scheme, not the port or subdomains. This accounts -// for the fact that changes to document.domain allow similar origin pages with -// different ports or subdomains to script each other. An "instance" includes -// all tabs that might be able to script each other because of how they were -// created (e.g., window.open or targeted links). We represent instances using -// the BrowsingInstance class. +// A page's SiteInstance is determined by a combination of where the page comes +// from (the site) and which frames have references to each other (the +// instance). Here, a "site" is similar to the page's origin but includes only +// the registered domain name and scheme, not the port or subdomains. This +// accounts for the fact that changes to document.domain allow similar origin +// pages with different ports or subdomains to script each other. An "instance" +// includes all frames that might be able to script each other because of how +// they were created (e.g., window.open or targeted links). We represent +// instances using the BrowsingInstance class. // -// Process models: +// Four process models are currently supported: // -// In process-per-site-instance (the current default process model), -// SiteInstances are created (1) when the user manually creates a new tab -// (which also creates a new BrowsingInstance), and (2) when the user navigates -// across site boundaries (which uses the same BrowsingInstance). If the user -// navigates within a site, the same SiteInstance is used. -// (Caveat: we currently allow renderer-initiated cross-site navigations to -// stay in the same SiteInstance, to preserve compatibility in cases like -// cross-site iframes that open popups.) +// PROCESS PER SITE INSTANCE (the current default): SiteInstances are created +// (1) when the user manually creates a new tab (which also creates a new +// BrowsingInstance), and (2) when the user navigates across site boundaries +// (which uses the same BrowsingInstance). If the user navigates within a site, +// the same SiteInstance is used. Caveat: we currently allow renderer-initiated +// cross-site navigations to stay in the same SiteInstance, to preserve +// compatibility in cases like cross-site iframes that open popups. // -// In --process-per-tab, SiteInstances are created when the user manually -// creates a new tab, but not when navigating across site boundaries (unless -// a process swap is required for security reasons, such as navigating from -// a privileged WebUI page to a normal web page). This corresponds to one -// process per BrowsingInstance. +// SITE PER PROCESS (currently experimental): is the most granular process +// model and is made possible by our support for out-of-process iframes. A +// subframe will be given a different SiteInstance if its site differs from the +// containing document. Cross-site navigation of top-level frames or subframes +// will trigger a change of SiteInstances, even if the navigation is renderer +// initiated. In this model, each process can be dedicated to documents from +// just one site, allowing the same origin policy to be enforced by the sandbox. // -// In --process-per-site, we consolidate all SiteInstances for a given site into -// the same process, throughout the entire browser context. This ensures that -// only one process will be used for each site. +// PROCESS PER TAB: SiteInstances are created when the user manually creates a +// new tab, but not when navigating across site boundaries (unless a process +// swap is required for security reasons, such as navigating from a privileged +// WebUI page to a normal web page). This corresponds to one process per +// BrowsingInstance. +// +// PROCESS PER SITE: We consolidate all SiteInstances for a given site into the +// same process, throughout the entire browser context. This ensures that only +// one process will be used for each site. // // Each NavigationEntry for a WebContents points to the SiteInstance that -// rendered it. Each RenderViewHost also points to the SiteInstance that it is +// rendered it. Each RenderFrameHost also points to the SiteInstance that it is // associated with. A SiteInstance keeps track of the number of these // references and deletes itself when the count goes to zero. This means that // a SiteInstance is only live as long as it is accessible, either from new @@ -110,12 +119,16 @@ class CONTENT_EXPORT SiteInstance : public base::RefCounted<SiteInstance> { // related SiteInstances in the same BrowsingInstance. virtual size_t GetRelatedActiveContentsCount() = 0; + // Returns true if this SiteInstance is for a site that requires a dedicated + // process. This only returns true under the "site per process" process model. + virtual bool RequiresDedicatedProcess() = 0; + // Factory method to create a new SiteInstance. This will create a new // new BrowsingInstance, so it should only be used when creating a new tab // from scratch (or similar circumstances). Callers should ensure that // this SiteInstance becomes ref counted, by storing it in a scoped_refptr. // - // The render process host factory may be nullptr. See SiteInstance + // The render process host factory may be nullptr. See SiteInstance // constructor. // // TODO(creis): This may be an argument to build a pass_refptr<T> class, as diff --git a/content/public/common/browser_plugin_guest_mode.cc b/content/public/common/browser_plugin_guest_mode.cc new file mode 100644 index 0000000..f9181c4 --- /dev/null +++ b/content/public/common/browser_plugin_guest_mode.cc @@ -0,0 +1,18 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "content/public/common/browser_plugin_guest_mode.h" + +#include "base/command_line.h" +#include "content/public/common/content_switches.h" + +namespace content { + +// static +bool BrowserPluginGuestMode::UseCrossProcessFramesForGuests() { + return base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess); +} + +} // namespace content diff --git a/content/public/common/browser_plugin_guest_mode.h b/content/public/common/browser_plugin_guest_mode.h new file mode 100644 index 0000000..dd16ac3 --- /dev/null +++ b/content/public/common/browser_plugin_guest_mode.h @@ -0,0 +1,28 @@ +// Copyright 2015 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CONTENT_PUBLIC_COMMON_BROWSER_PLUGIN_GUEST_MODE_H_ +#define CONTENT_PUBLIC_COMMON_BROWSER_PLUGIN_GUEST_MODE_H_ + +#include "base/basictypes.h" +#include "content/common/content_export.h" + +namespace content { + +class CONTENT_EXPORT BrowserPluginGuestMode { + public: + // Returns true if inner WebContents should be implemented in terms of cross- + // process iframes. TODO(lazyboy, nick): This should probably be a command + // line flag separate from full site isolation (--site-per-process). + static bool UseCrossProcessFramesForGuests(); + + private: + BrowserPluginGuestMode(); // Not instantiable + + DISALLOW_COPY_AND_ASSIGN(BrowserPluginGuestMode); +}; + +} // namespace + +#endif // CONTENT_PUBLIC_COMMON_BROWSER_PLUGIN_GUEST_MODE_H_ diff --git a/content/public/test/browser_test_utils.cc b/content/public/test/browser_test_utils.cc index e1ddc2d..2cd42b5 100644 --- a/content/public/test/browser_test_utils.cc +++ b/content/public/test/browser_test_utils.cc @@ -32,6 +32,7 @@ #include "content/public/browser/render_view_host.h" #include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents_observer.h" +#include "content/public/common/content_switches.h" #include "content/public/test/test_utils.h" #include "net/base/filename_util.h" #include "net/cookies/cookie_store.h" @@ -742,6 +743,15 @@ void RunTaskAndWaitForInterstitialDetach(content::WebContents* web_contents, loop_runner->Run(); } +bool AreAllSitesIsolatedForTesting() { + return base::CommandLine::ForCurrentProcess()->HasSwitch( + switches::kSitePerProcess); +} + +void IsolateAllSitesForTesting(base::CommandLine* command_line) { + command_line->AppendSwitch(switches::kSitePerProcess); +} + bool WaitForRenderFrameReady(RenderFrameHost* rfh) { if (!rfh) return false; diff --git a/content/public/test/browser_test_utils.h b/content/public/test/browser_test_utils.h index bd5cddc..812cb35 100644 --- a/content/public/test/browser_test_utils.h +++ b/content/public/test/browser_test_utils.h @@ -10,6 +10,7 @@ #include <vector> #include "base/callback.h" +#include "base/command_line.h" #include "base/compiler_specific.h" #include "base/files/scoped_temp_dir.h" #include "base/memory/ref_counted.h" @@ -246,6 +247,19 @@ void WaitForInterstitialDetach(content::WebContents* web_contents); void RunTaskAndWaitForInterstitialDetach(content::WebContents* web_contents, const base::Closure& task); +// Returns true if all sites are isolated. Typically used to bail from a test +// that is incompatible with --site-per-process. +bool AreAllSitesIsolatedForTesting(); + +// Appends --site-per-process to the command line, enabling tests to exercise +// site isolation and cross-process iframes. +// +// TODO(nick): In some places this method is called from the top of a test +// body. That's not strictly safe (it's setting a command line after it +// already may have been read). We should try make that pattern safer, as it +// makes browser tests easier to write. +void IsolateAllSitesForTesting(base::CommandLine* command_line); + // Waits until all resources have loaded in the given RenderFrameHost. // When the load completes, this function sends a "pageLoadComplete" message // via domAutomationController. The caller should make sure this extra diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index 50f70da..f6a4bac 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -41,6 +41,7 @@ #include "content/common/input_messages.h" #include "content/common/navigation_params.h" #include "content/common/service_worker/service_worker_types.h" +#include "content/common/site_isolation_policy.h" #include "content/common/swapped_out_messages.h" #include "content/common/view_messages.h" #include "content/public/common/bindings_policy.h" @@ -515,11 +516,6 @@ bool IsReload(FrameMsg_Navigate_Type::Value navigation_type) { navigation_type == FrameMsg_Navigate_Type::RELOAD_ORIGINAL_REQUEST_URL; } -bool IsSwappedOutStateForbidden() { - return base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); -} - RenderFrameImpl::CreateRenderFrameImplFunction g_create_render_frame_impl = nullptr; @@ -599,8 +595,7 @@ void RenderFrameImpl::CreateFrame( CHECK_IMPLIES(parent_routing_id == MSG_ROUTING_NONE, !web_frame->parent()); if (widget_params.routing_id != MSG_ROUTING_NONE) { - CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)); + CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible()); render_frame->render_widget_ = RenderWidget::CreateForFrame( widget_params.routing_id, widget_params.surface_id, widget_params.hidden, render_frame->render_view_->screen_info(), @@ -707,18 +702,15 @@ RenderFrameImpl::~RenderFrameImpl() { #endif if (!is_subframe_) { - if (!IsSwappedOutStateForbidden()) { - // When using swapped out frames, RenderFrameProxy is "owned" by - // RenderFrameImpl in the case it is the main frame. Ensure it is deleted - // along with this object. - if (render_frame_proxy_ && - !base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { - // The following method calls back into this object and clears - // |render_frame_proxy_|. - render_frame_proxy_->frameDetached( - blink::WebRemoteFrameClient::DetachType::Remove); - } + // When using swapped out frames, RenderFrameProxy is owned by + // RenderFrameImpl in the case it is the main frame. Ensure it is deleted + // along with this object. + if (render_frame_proxy_ && + !RenderFrameProxy::IsSwappedOutStateForbidden()) { + // The following method calls back into this object and clears + // |render_frame_proxy_|. + render_frame_proxy_->frameDetached( + blink::WebRemoteFrameClient::DetachType::Remove); } // Ensure the RenderView doesn't point to this object, once it is destroyed. @@ -1133,12 +1125,12 @@ void RenderFrameImpl::OnSwapOut( const FrameReplicationState& replicated_frame_state) { TRACE_EVENT1("navigation", "RenderFrameImpl::OnSwapOut", "id", routing_id_); RenderFrameProxy* proxy = NULL; - bool is_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); + bool swapped_out_forbidden = RenderFrameProxy::IsSwappedOutStateForbidden(); bool is_main_frame = !frame_->parent(); // This codepath should only be hit for subframes when in --site-per-process. - CHECK_IMPLIES(!is_main_frame, is_site_per_process); + CHECK_IMPLIES(!is_main_frame, + SiteIsolationPolicy::AreCrossProcessFramesPossible()); // Only run unload if we're not swapped out yet, but send the ack either way. if (!is_swapped_out_) { @@ -1180,7 +1172,7 @@ void RenderFrameImpl::OnSwapOut( // TODO(creis): Should we be stopping all frames here and using // StopAltErrorPageFetcher with RenderView::OnStop, or just stopping this // frame? - if (!IsSwappedOutStateForbidden()) + if (!swapped_out_forbidden) OnStop(); // Transfer settings such as initial drawing parameters to the remote frame, @@ -1192,7 +1184,7 @@ void RenderFrameImpl::OnSwapOut( // run a second time, thanks to a check in FrameLoader::stopLoading. // TODO(creis): Need to add a better way to do this that avoids running the // beforeunload handler. For now, we just run it a second time silently. - if (!IsSwappedOutStateForbidden()) + if (!swapped_out_forbidden) NavigateToSwappedOutURL(); // Let WebKit know that this view is hidden so it can drop resources and @@ -1215,7 +1207,7 @@ void RenderFrameImpl::OnSwapOut( // Now that all of the cleanup is complete and the browser side is notified, // start using the RenderFrameProxy, if one is created. - if (proxy && IsSwappedOutStateForbidden()) { + if (proxy && swapped_out_forbidden) { frame_->swap(proxy->web_frame()); if (is_loading) @@ -1230,7 +1222,7 @@ void RenderFrameImpl::OnSwapOut( // in proxy->web_frame(), the RemoteFrame will not exist for main frames. // When we do an unconditional swap for all frames, we can remove // !is_main_frame below. - if (proxy && IsSwappedOutStateForbidden()) + if (proxy && swapped_out_forbidden) proxy->SetReplicatedState(replicated_frame_state); // Safe to exit if no one else is using the process. @@ -2267,9 +2259,7 @@ void RenderFrameImpl::didChangeName(blink::WebLocalFrame* frame, // can be optimized further by only sending the update if there are any // remote frames in the frame tree, or delaying and batching up IPCs if // updates are happening too frequently. - bool is_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); - if (is_site_per_process || + if (SiteIsolationPolicy::AreCrossProcessFramesPossible() || render_view_->renderer_preferences_.report_frame_name_changes) { Send(new FrameHostMsg_DidChangeName( routing_id_, base::UTF16ToUTF8(base::StringPiece16(name)))); @@ -2539,9 +2529,8 @@ void RenderFrameImpl::didStartProvisionalLoad(blink::WebLocalFrame* frame, DocumentState* document_state = DocumentState::FromDataSource(ds); // We should only navigate to swappedout:// when is_swapped_out_ is true. - CHECK((ds->request().url() != GURL(kSwappedOutURL)) || - is_swapped_out_) << - "Heard swappedout:// when not swapped out."; + CHECK_IMPLIES(ds->request().url() == GURL(kSwappedOutURL), is_swapped_out_) + << "Heard swappedout:// when not swapped out."; // Update the request time if WebKit has better knowledge of it. if (document_state->request_time().is_null() && @@ -3954,8 +3943,7 @@ void RenderFrameImpl::SendDidCommitProvisionalLoad( // Make navigation state a part of the DidCommitProvisionalLoad message so // that committed entry has it at all times. HistoryEntry* entry = render_view_->history_controller()->GetCurrentEntry(); - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (!SiteIsolationPolicy::UseSubframeNavigationEntries()) { if (entry) params.page_state = HistoryEntryToPageState(entry); else @@ -4194,10 +4182,9 @@ WebNavigationPolicy RenderFrameImpl::DecidePolicyForNavigation( const NavigationPolicyInfo& info) { Referrer referrer(RenderViewImpl::GetReferrerFromRequest(info.frame, info.urlRequest)); - const base::CommandLine& command_line = - *base::CommandLine::ForCurrentProcess(); - if (command_line.HasSwitch(switches::kSitePerProcess) && is_subframe_) { + // TODO(nick): Is consulting |is_subframe_| here correct? + if (RenderFrameProxy::IsSwappedOutStateForbidden() && is_subframe_) { // There's no reason to ignore navigations on subframes, since the swap out // logic no longer applies. } else { @@ -4504,8 +4491,7 @@ void RenderFrameImpl::NavigateInternal( if (!browser_side_navigation) { scoped_ptr<NavigationParams> navigation_params( new NavigationParams(*pending_navigation_params_.get())); - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) { + if (!SiteIsolationPolicy::UseSubframeNavigationEntries()) { // By default, tell the HistoryController to go the deserialized // HistoryEntry. This only works if all frames are in the same // process. diff --git a/content/renderer/render_frame_impl_browsertest.cc b/content/renderer/render_frame_impl_browsertest.cc index 7305512..1a0e02c 100644 --- a/content/renderer/render_frame_impl_browsertest.cc +++ b/content/renderer/render_frame_impl_browsertest.cc @@ -6,7 +6,7 @@ #include "base/debug/leak_annotations.h" #include "content/common/frame_messages.h" #include "content/common/view_messages.h" -#include "content/public/common/content_switches.h" +#include "content/public/test/browser_test_utils.h" #include "content/public/test/frame_load_waiter.h" #include "content/public/test/render_view_test.h" #include "content/renderer/render_frame_impl.h" @@ -42,8 +42,7 @@ class RenderFrameImplTest : public RenderViewTest { widget_params.surface_id = kSubframeSurfaceId; widget_params.hidden = false; - base::CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kSitePerProcess); + IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess()); LoadHTML("Parent frame <iframe name='frame'></iframe>"); diff --git a/content/renderer/render_frame_proxy.cc b/content/renderer/render_frame_proxy.cc index 7de79b3..d4946b7 100644 --- a/content/renderer/render_frame_proxy.cc +++ b/content/renderer/render_frame_proxy.cc @@ -11,9 +11,9 @@ #include "content/child/webmessageportchannel_impl.h" #include "content/common/frame_messages.h" #include "content/common/frame_replication_state.h" +#include "content/common/site_isolation_policy.h" #include "content/common/swapped_out_messages.h" #include "content/common/view_messages.h" -#include "content/public/common/content_switches.h" #include "content/renderer/child_frame_compositing_helper.h" #include "content/renderer/render_frame_impl.h" #include "content/renderer/render_thread_impl.h" @@ -119,8 +119,7 @@ RenderFrameProxy* RenderFrameProxy::FromWebFrame(blink::WebFrame* web_frame) { // static bool RenderFrameProxy::IsSwappedOutStateForbidden() { - return base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess); + return SiteIsolationPolicy::AreCrossProcessFramesPossible(); } RenderFrameProxy::RenderFrameProxy(int routing_id, int frame_routing_id) diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc index a5b060b..3af6bcf 100644 --- a/content/renderer/render_view_browsertest.cc +++ b/content/renderer/render_view_browsertest.cc @@ -765,8 +765,7 @@ TEST_F(RenderViewImplTest, ReloadWhileSwappedOut) { TEST_F(RenderViewImplTest, OriginReplicationForSwapOut) { // This test should only run with --site-per-process, since origin // replication only happens in that mode. - if (!base::CommandLine::ForCurrentProcess()->HasSwitch( - switches::kSitePerProcess)) + if (!AreAllSitesIsolatedForTesting()) return; LoadHTML( diff --git a/extensions/renderer/dispatcher.cc b/extensions/renderer/dispatcher.cc index 4abecfe..8f7494a 100644 --- a/extensions/renderer/dispatcher.cc +++ b/extensions/renderer/dispatcher.cc @@ -19,6 +19,7 @@ #include "base/values.h" #include "content/grit/content_resources.h" #include "content/public/child/v8_value_converter.h" +#include "content/public/common/browser_plugin_guest_mode.h" #include "content/public/common/content_switches.h" #include "content/public/common/url_constants.h" #include "content/public/renderer/render_frame.h" @@ -472,8 +473,7 @@ std::vector<std::pair<std::string, int> > Dispatcher::GetJsResources() { resources.push_back(std::make_pair("guestViewEvents", IDR_GUEST_VIEW_EVENTS_JS)); - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - ::switches::kSitePerProcess)) { + if (content::BrowserPluginGuestMode::UseCrossProcessFramesForGuests()) { resources.push_back(std::make_pair("guestViewIframe", IDR_GUEST_VIEW_IFRAME_JS)); resources.push_back(std::make_pair("guestViewIframeContainer", @@ -517,8 +517,7 @@ std::vector<std::pair<std::string, int> > Dispatcher::GetJsResources() { resources.push_back(std::make_pair("webViewEvents", IDR_WEB_VIEW_EVENTS_JS)); resources.push_back(std::make_pair("webViewInternal", IDR_WEB_VIEW_INTERNAL_CUSTOM_BINDINGS_JS)); - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - ::switches::kSitePerProcess)) { + if (content::BrowserPluginGuestMode::UseCrossProcessFramesForGuests()) { resources.push_back(std::make_pair("webViewIframe", IDR_WEB_VIEW_IFRAME_JS)); } @@ -1436,8 +1435,7 @@ void Dispatcher::RequireGuestViewModules(ScriptContext* context) { module_system->Require("webViewApiMethods"); module_system->Require("webViewAttributes"); - if (base::CommandLine::ForCurrentProcess()->HasSwitch( - ::switches::kSitePerProcess)) { + if (content::BrowserPluginGuestMode::UseCrossProcessFramesForGuests()) { module_system->Require("webViewIframe"); } } |