diff options
47 files changed, 753 insertions, 213 deletions
diff --git a/chrome/browser/extensions/app_process_apitest.cc b/chrome/browser/extensions/app_process_apitest.cc index c5be9c2..4172c0c 100644 --- a/chrome/browser/extensions/app_process_apitest.cc +++ b/chrome/browser/extensions/app_process_apitest.cc @@ -105,12 +105,9 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcess) { base_url.Resolve("path1/empty.html"), true); WindowOpenHelper(browser(), host, base_url.Resolve("path2/empty.html"), true); - // TODO(creis): This should open in a new process (i.e., false for the last - // argument), but we temporarily avoid swapping processes away from an app - // until we're able to restore window.opener if the page later returns to an - // in-app URL. See crbug.com/65953. + // This should open in a new process (i.e., false for the last argument). WindowOpenHelper(browser(), host, - base_url.Resolve("path3/empty.html"), true); + base_url.Resolve("path3/empty.html"), false); // Now let's have these pages navigate, into or out of the extension web // extent. They should switch processes. @@ -118,10 +115,7 @@ IN_PROC_BROWSER_TEST_F(AppApiTest, AppProcess) { const GURL& non_app_url(base_url.Resolve("path3/empty.html")); NavigateTabHelper(browser()->GetTabContentsAt(2), non_app_url); NavigateTabHelper(browser()->GetTabContentsAt(3), app_url); - // TODO(creis): This should swap out of the app's process (i.e., EXPECT_NE), - // but we temporarily avoid swapping away from an app in case it needs to - // communicate with window.opener later. See crbug.com/65953. - EXPECT_EQ(host->process(), + EXPECT_NE(host->process(), browser()->GetTabContentsAt(2)->render_view_host()->process()); EXPECT_EQ(host->process(), browser()->GetTabContentsAt(3)->render_view_host()->process()); diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc index 7badebf..af3eda6 100644 --- a/chrome/browser/extensions/extension_host.cc +++ b/chrome/browser/extensions/extension_host.cc @@ -427,7 +427,8 @@ void ExtensionHost::DocumentOnLoadCompletedInMainFrame(RenderViewHost* rvh, } } -void ExtensionHost::RunJavaScriptMessage(const std::wstring& message, +void ExtensionHost::RunJavaScriptMessage(const RenderViewHost* rvh, + const std::wstring& message, const std::wstring& default_prompt, const GURL& frame_url, const int flags, diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h index 311975d..116e37d 100644 --- a/chrome/browser/extensions/extension_host.h +++ b/chrome/browser/extensions/extension_host.h @@ -129,7 +129,8 @@ class ExtensionHost : public RenderViewHostDelegate, // RenderViewHostDelegate implementation. virtual RenderViewHostDelegate::View* GetViewDelegate(); virtual WebPreferences GetWebkitPrefs(); - virtual void RunJavaScriptMessage(const std::wstring& message, + virtual void RunJavaScriptMessage(const RenderViewHost* rvh, + const std::wstring& message, const std::wstring& default_prompt, const GURL& frame_url, const int flags, diff --git a/chrome/browser/tab_contents/background_contents.cc b/chrome/browser/tab_contents/background_contents.cc index e5f98b8..2d0dd69 100644 --- a/chrome/browser/tab_contents/background_contents.cc +++ b/chrome/browser/tab_contents/background_contents.cc @@ -103,6 +103,7 @@ void BackgroundContents::DidNavigate( } void BackgroundContents::RunJavaScriptMessage( + const RenderViewHost* rvh, const std::wstring& message, const std::wstring& default_prompt, const GURL& frame_url, diff --git a/chrome/browser/tab_contents/background_contents.h b/chrome/browser/tab_contents/background_contents.h index f4e8874..9cd4f81 100644 --- a/chrome/browser/tab_contents/background_contents.h +++ b/chrome/browser/tab_contents/background_contents.h @@ -67,7 +67,8 @@ class BackgroundContents : public RenderViewHostDelegate, virtual void DidNavigate(RenderViewHost* render_view_host, const ViewHostMsg_FrameNavigate_Params& params); virtual WebPreferences GetWebkitPrefs(); - virtual void RunJavaScriptMessage(const std::wstring& message, + virtual void RunJavaScriptMessage(const RenderViewHost* rvh, + const std::wstring& message, const std::wstring& default_prompt, const GURL& frame_url, const int flags, diff --git a/chrome/browser/tab_contents/web_contents_unittest.cc b/chrome/browser/tab_contents/web_contents_unittest.cc index 76f0ef0..411e693 100644 --- a/chrome/browser/tab_contents/web_contents_unittest.cc +++ b/chrome/browser/tab_contents/web_contents_unittest.cc @@ -300,6 +300,11 @@ TEST_F(TabContentsTest, CrossSiteBoundaries) { int pending_rvh_delete_count = 0; pending_rvh->set_delete_counter(&pending_rvh_delete_count); + // Navigations should be suspended in pending_rvh until ShouldCloseACK. + EXPECT_TRUE(pending_rvh->are_navigations_suspended()); + orig_rvh->SendShouldCloseACK(true); + EXPECT_FALSE(pending_rvh->are_navigations_suspended()); + // DidNavigate from the pending page ViewHostMsg_FrameNavigate_Params params2; InitNavigateParams(¶ms2, 1, url2, PageTransition::TYPED); @@ -310,20 +315,36 @@ TEST_F(TabContentsTest, CrossSiteBoundaries) { EXPECT_EQ(pending_rvh, contents()->render_view_host()); EXPECT_NE(instance1, instance2); EXPECT_TRUE(contents()->pending_rvh() == NULL); - EXPECT_EQ(orig_rvh_delete_count, 1); + // We keep the original RVH around, swapped out. + EXPECT_TRUE(contents()->render_manager()->IsSwappedOut(orig_rvh)); + EXPECT_EQ(orig_rvh_delete_count, 0); // Going back should switch SiteInstances again. The first SiteInstance is // stored in the NavigationEntry, so it should be the same as at the start. + // We should use the same RVH as before, swapping it back in. controller().GoBack(); TestRenderViewHost* goback_rvh = contents()->pending_rvh(); + EXPECT_EQ(orig_rvh, goback_rvh); EXPECT_TRUE(contents()->cross_navigation_pending()); + // Navigations should be suspended in goback_rvh until ShouldCloseACK. + EXPECT_TRUE(goback_rvh->are_navigations_suspended()); + pending_rvh->SendShouldCloseACK(true); + EXPECT_FALSE(goback_rvh->are_navigations_suspended()); + // DidNavigate from the back action contents()->TestDidNavigate(goback_rvh, params1); EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_EQ(goback_rvh, contents()->render_view_host()); - EXPECT_EQ(pending_rvh_delete_count, 1); EXPECT_EQ(instance1, contents()->GetSiteInstance()); + // The pending RVH should now be swapped out, not deleted. + EXPECT_TRUE(contents()->render_manager()->IsSwappedOut(pending_rvh)); + EXPECT_EQ(pending_rvh_delete_count, 0); + + // Close tab and ensure RVHs are deleted. + DeleteContents(); + EXPECT_EQ(orig_rvh_delete_count, 1); + EXPECT_EQ(pending_rvh_delete_count, 1); } // Test that navigating across a site boundary after a crash creates a new @@ -355,6 +376,7 @@ TEST_F(TabContentsTest, CrossSiteBoundariesAfterCrash) { EXPECT_FALSE(contents()->cross_navigation_pending()); EXPECT_TRUE(contents()->pending_rvh() == NULL); EXPECT_NE(orig_rvh, new_rvh); + EXPECT_FALSE(contents()->render_manager()->IsSwappedOut(orig_rvh)); EXPECT_EQ(orig_rvh_delete_count, 1); // DidNavigate from the new page @@ -367,6 +389,10 @@ TEST_F(TabContentsTest, CrossSiteBoundariesAfterCrash) { EXPECT_EQ(new_rvh, rvh()); EXPECT_NE(instance1, instance2); EXPECT_TRUE(contents()->pending_rvh() == NULL); + + // Close tab and ensure RVHs are deleted. + DeleteContents(); + EXPECT_EQ(orig_rvh_delete_count, 1); } // Test that opening a new tab in the same SiteInstance and then navigating @@ -395,6 +421,7 @@ TEST_F(TabContentsTest, NavigateTwoTabsCrossSite) { // Navigate first tab to a new site const GURL url2a("http://www.yahoo.com"); controller().LoadURL(url2a, GURL(), PageTransition::TYPED); + orig_rvh->SendShouldCloseACK(true); TestRenderViewHost* pending_rvh_a = contents()->pending_rvh(); ViewHostMsg_FrameNavigate_Params params2a; InitNavigateParams(¶ms2a, 1, url2a, PageTransition::TYPED); @@ -405,6 +432,9 @@ TEST_F(TabContentsTest, NavigateTwoTabsCrossSite) { // Navigate second tab to the same site as the first tab const GURL url2b("http://mail.yahoo.com"); contents2.controller().LoadURL(url2b, GURL(), PageTransition::TYPED); + TestRenderViewHost* rvh2 = + static_cast<TestRenderViewHost*>(contents2.render_view_host()); + rvh2->SendShouldCloseACK(true); TestRenderViewHost* pending_rvh_b = contents2.pending_rvh(); EXPECT_TRUE(pending_rvh_b != NULL); EXPECT_TRUE(contents2.cross_navigation_pending()); @@ -1602,7 +1632,8 @@ TEST_F(TabContentsTest, NoJSMessageOnInterstitials) { // attempting to show a JS message. IPC::Message* dummy_message = new IPC::Message; bool did_suppress_message = false; - contents()->RunJavaScriptMessage(L"This is an informative message", L"OK", + contents()->RunJavaScriptMessage(contents()->render_view_host(), + L"This is an informative message", L"OK", kGURL, ui::MessageBoxFlags::kIsJavascriptAlert, dummy_message, &did_suppress_message); EXPECT_TRUE(did_suppress_message); diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc index 2095fcf..240bcf0 100644 --- a/chrome/browser/ui/browser.cc +++ b/chrome/browser/ui/browser.cc @@ -4201,7 +4201,7 @@ void Browser::ProcessPendingTabs() { // Null check render_view_host here as this gets called on a PostTask and // the tab's render_view_host may have been nulled out. if (tab->render_view_host()) { - tab->render_view_host()->ClosePage(false, -1, -1); + tab->render_view_host()->ClosePage(); } else { ClearUnloadState(tab, true); } diff --git a/chrome/browser/ui/webui/web_ui_unittest.cc b/chrome/browser/ui/webui/web_ui_unittest.cc index 0d4543c..a37b5cf 100644 --- a/chrome/browser/ui/webui/web_ui_unittest.cc +++ b/chrome/browser/ui/webui/web_ui_unittest.cc @@ -182,19 +182,28 @@ TEST_F(WebUITest, FocusOnNavigate) { GURL next_url("http://google.com/"); int next_page_id = page_id + 1; controller().LoadURL(next_url, GURL(), PageTransition::LINK); + TestRenderViewHost* old_rvh = rvh(); + old_rvh->SendShouldCloseACK(true); pending_rvh()->SendNavigate(next_page_id, next_url); + old_rvh->OnSwapOutACK(); // Navigate back. Should focus the location bar. int focus_called = tc->focus_called(); ASSERT_TRUE(controller().CanGoBack()); controller().GoBack(); + old_rvh = rvh(); + old_rvh->SendShouldCloseACK(true); pending_rvh()->SendNavigate(page_id, new_tab_url); + old_rvh->OnSwapOutACK(); EXPECT_LT(focus_called, tc->focus_called()); // Navigate forward. Shouldn't focus the location bar. focus_called = tc->focus_called(); ASSERT_TRUE(controller().CanGoForward()); controller().GoForward(); + old_rvh = rvh(); + old_rvh->SendShouldCloseACK(true); pending_rvh()->SendNavigate(next_page_id, next_url); + old_rvh->OnSwapOutACK(); EXPECT_EQ(focus_called, tc->focus_called()); } diff --git a/chrome/browser/visitedlink/visitedlink_unittest.cc b/chrome/browser/visitedlink/visitedlink_unittest.cc index 6f58ad7..f5f1ace 100644 --- a/chrome/browser/visitedlink/visitedlink_unittest.cc +++ b/chrome/browser/visitedlink/visitedlink_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -512,7 +512,7 @@ class VisitRelayingRenderProcessHost : public BrowserRenderProcessHost { virtual void CancelResourceRequests(int render_widget_id) { } - virtual void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params) { + virtual void CrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params) { } virtual bool WaitForPaintMsg(int render_widget_id, diff --git a/chrome/common/chrome_content_client.cc b/chrome/common/chrome_content_client.cc index 84b5e5d..f5d0e90 100644 --- a/chrome/common/chrome_content_client.cc +++ b/chrome/common/chrome_content_client.cc @@ -12,6 +12,7 @@ #include "chrome/common/child_process_logging.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" +#include "chrome/common/render_messages.h" #include "content/common/pepper_plugin_registry.h" #include "remoting/client/plugin/pepper_entrypoints.h" @@ -189,4 +190,30 @@ void ChromeContentClient::AddPepperPlugins( #endif } +bool ChromeContentClient::CanSendWhileSwappedOut(const IPC::Message* msg) { + // Any Chrome-specific messages that must be allowed to be sent from swapped + // out renderers. + switch (msg->type()) { + case ViewHostMsg_DomOperationResponse::ID: + return true; + default: + break; + } + return false; +} + +bool ChromeContentClient::CanHandleWhileSwappedOut( + const IPC::Message& msg) { + // Any Chrome-specific messages (apart from those listed in + // CanSendWhileSwappedOut) that must be handled by the browser when sent from + // swapped out renderers. + switch (msg.type()) { + case ViewHostMsg_Snapshot::ID: + return true; + default: + break; + } + return false; +} + } // namespace chrome diff --git a/chrome/common/chrome_content_client.h b/chrome/common/chrome_content_client.h index bd8d78c..b7316b6 100644 --- a/chrome/common/chrome_content_client.h +++ b/chrome/common/chrome_content_client.h @@ -17,6 +17,8 @@ class ChromeContentClient : public content::ContentClient { virtual void SetActiveURL(const GURL& url); virtual void SetGpuInfo(const GPUInfo& gpu_info); virtual void AddPepperPlugins(std::vector<PepperPluginInfo>* plugins); + virtual bool CanSendWhileSwappedOut(const IPC::Message* msg); + virtual bool CanHandleWhileSwappedOut(const IPC::Message& msg); }; } // namespace chrome diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h index 3681e40..80534d8 100644 --- a/chrome/common/render_messages.h +++ b/chrome/common/render_messages.h @@ -29,13 +29,14 @@ #include "ipc/ipc_message_macros.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebCache.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebConsoleMessage.h" -#include "third_party/skia/include/core/SkBitmap.h" #include "ui/gfx/rect.h" // Singly-included section for enums and custom IPC traits. #ifndef CHROME_COMMON_RENDER_MESSAGES_H_ #define CHROME_COMMON_RENDER_MESSAGES_H_ +class SkBitmap; + // Command values for the cmd parameter of the // ViewHost_JavaScriptStressTestControl message. For each command the parameter // passed has a different meaning: diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc index f01769c..4d38a15 100644 --- a/chrome/renderer/chrome_content_renderer_client.cc +++ b/chrome/renderer/chrome_content_renderer_client.cc @@ -461,9 +461,6 @@ bool ChromeContentRendererClient::ShouldFork(WebFrame* frame, // TODO(erikkay) This is happening inside of a check to is_content_initiated // which means that things like the back button won't trigger it. Is that // OK? - // TODO(creis): For hosted apps, we currently only swap processes to enter - // the app and not exit it, since we currently lose context (e.g., - // window.opener) if the window navigates back. See crbug.com/65953. if (!CrossesExtensionExtents(frame, url)) return false; @@ -559,10 +556,7 @@ bool ChromeContentRendererClient::CrossesExtensionExtents(WebFrame* frame, if (old_url.is_empty() && frame->opener()) old_url = frame->opener()->url(); - bool old_url_is_hosted_app = extensions->GetByURL(old_url) && - !extensions->GetByURL(old_url)->web_extent().is_empty(); - return !extensions->InSameExtent(old_url, new_url) && - !old_url_is_hosted_app; + return !extensions->InSameExtent(old_url, new_url); } } // namespace chrome diff --git a/chrome/renderer/chrome_content_renderer_client.h b/chrome/renderer/chrome_content_renderer_client.h index d408bab..26e4ccb 100644 --- a/chrome/renderer/chrome_content_renderer_client.h +++ b/chrome/renderer/chrome_content_renderer_client.h @@ -83,11 +83,6 @@ class ChromeContentRendererClient : public content::ContentRendererClient { // Returns true if the frame is navigating to an URL either into or out of an // extension app's extent. - // TODO(creis): Temporary workaround for crbug.com/65953: Only return true if - // we would enter an extension app's extent from a non-app, or if we leave an - // extension with no web extent. We avoid swapping processes to exit a hosted - // app with a web extent for now, since we do not yet restore context (such - // as window.opener) if the window navigates back. bool CrossesExtensionExtents(WebKit::WebFrame* frame, const GURL& new_url); scoped_ptr<ChromeRenderProcessObserver> chrome_observer_; diff --git a/content/browser/renderer_host/browser_render_process_host.cc b/content/browser/renderer_host/browser_render_process_host.cc index aa5808f..ef90cd65 100644 --- a/content/browser/renderer_host/browser_render_process_host.cc +++ b/content/browser/renderer_host/browser_render_process_host.cc @@ -412,9 +412,9 @@ void BrowserRenderProcessHost::CancelResourceRequests(int render_widget_id) { widget_helper_->CancelResourceRequests(render_widget_id); } -void BrowserRenderProcessHost::CrossSiteClosePageACK( - const ViewMsg_ClosePage_Params& params) { - widget_helper_->CrossSiteClosePageACK(params); +void BrowserRenderProcessHost::CrossSiteSwapOutACK( + const ViewMsg_SwapOut_Params& params) { + widget_helper_->CrossSiteSwapOutACK(params); } bool BrowserRenderProcessHost::WaitForUpdateMsg( diff --git a/content/browser/renderer_host/browser_render_process_host.h b/content/browser/renderer_host/browser_render_process_host.h index 3f7a23c..ca21e1e 100644 --- a/content/browser/renderer_host/browser_render_process_host.h +++ b/content/browser/renderer_host/browser_render_process_host.h @@ -50,7 +50,7 @@ class BrowserRenderProcessHost : public RenderProcessHost, virtual bool Init(bool is_accessibility_enabled, bool is_extensions_process); virtual int GetNextRoutingID(); virtual void CancelResourceRequests(int render_widget_id); - virtual void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params); + virtual void CrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params); virtual bool WaitForUpdateMsg(int render_widget_id, const base::TimeDelta& max_delay, IPC::Message* msg); diff --git a/content/browser/renderer_host/mock_render_process_host.cc b/content/browser/renderer_host/mock_render_process_host.cc index 104395f..b4c9a9c 100644 --- a/content/browser/renderer_host/mock_render_process_host.cc +++ b/content/browser/renderer_host/mock_render_process_host.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -36,8 +36,8 @@ int MockRenderProcessHost::GetNextRoutingID() { void MockRenderProcessHost::CancelResourceRequests(int render_widget_id) { } -void MockRenderProcessHost::CrossSiteClosePageACK( - const ViewMsg_ClosePage_Params& params) { +void MockRenderProcessHost::CrossSiteSwapOutACK( + const ViewMsg_SwapOut_Params& params) { } bool MockRenderProcessHost::WaitForUpdateMsg(int render_widget_id, diff --git a/content/browser/renderer_host/mock_render_process_host.h b/content/browser/renderer_host/mock_render_process_host.h index 0e7656c..8faabfb 100644 --- a/content/browser/renderer_host/mock_render_process_host.h +++ b/content/browser/renderer_host/mock_render_process_host.h @@ -39,7 +39,7 @@ class MockRenderProcessHost : public RenderProcessHost { virtual bool Init(bool is_accessibility_enabled, bool is_extensions_process); virtual int GetNextRoutingID(); virtual void CancelResourceRequests(int render_widget_id); - virtual void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params); + virtual void CrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params); virtual bool WaitForUpdateMsg(int render_widget_id, const base::TimeDelta& max_delay, IPC::Message* msg); diff --git a/content/browser/renderer_host/render_process_host.h b/content/browser/renderer_host/render_process_host.h index b43fbe2..66c3200 100644 --- a/content/browser/renderer_host/render_process_host.h +++ b/content/browser/renderer_host/render_process_host.h @@ -17,7 +17,7 @@ #include "ui/gfx/surface/transport_dib.h" class Profile; -struct ViewMsg_ClosePage_Params; +struct ViewMsg_SwapOut_Params; namespace base { class SharedMemory; @@ -173,12 +173,12 @@ class RenderProcessHost : public IPC::Channel::Sender, // the specified render widget. virtual void CancelResourceRequests(int render_widget_id) = 0; - // Called on the UI thread to simulate a ClosePage_ACK message to the + // Called on the UI thread to simulate a SwapOut_ACK message to the // ResourceDispatcherHost. Necessary for a cross-site request, in the case // that the original RenderViewHost is not live and thus cannot run an - // onunload handler. - virtual void CrossSiteClosePageACK( - const ViewMsg_ClosePage_Params& params) = 0; + // unload handler. + virtual void CrossSiteSwapOutACK( + const ViewMsg_SwapOut_Params& params) = 0; // Called on the UI thread to wait for the next UpdateRect message for the // specified render widget. Returns true if successful, and the msg out- diff --git a/content/browser/renderer_host/render_view_host.cc b/content/browser/renderer_host/render_view_host.cc index 47e5457..a51b567 100644 --- a/content/browser/renderer_host/render_view_host.cc +++ b/content/browser/renderer_host/render_view_host.cc @@ -39,6 +39,7 @@ #include "content/common/notification_service.h" #include "content/common/notification_type.h" #include "content/common/result_codes.h" +#include "content/common/swapped_out_messages.h" #include "content/common/url_constants.h" #include "content/common/view_messages.h" #include "net/base/net_util.h" @@ -92,9 +93,10 @@ RenderViewHost::RenderViewHost(SiteInstance* instance, delegate_(delegate), waiting_for_drag_context_response_(false), enabled_bindings_(0), - pending_request_id_(0), + pending_request_id_(-1), navigations_suspended_(false), suspended_nav_message_(NULL), + is_swapped_out_(false), run_modal_reply_msg_(NULL), is_waiting_for_beforeunload_ack_(false), is_waiting_for_unload_ack_(false), @@ -258,11 +260,20 @@ void RenderViewHost::SetNavigationsSuspended(bool suspend) { navigations_suspended_ = suspend; if (!suspend && suspended_nav_message_.get()) { // There's a navigation message waiting to be sent. Now that we're not - // suspended anymore, resume navigation by sending it. + // suspended anymore, resume navigation by sending it. If we were swapped + // out, we should also stop filtering out the IPC messages now. + is_swapped_out_ = false; Send(suspended_nav_message_.release()); } } +void RenderViewHost::CancelSuspendedNavigations() { + // Clear any state if a pending navigation is canceled or pre-empted. + if (suspended_nav_message_.get()) + suspended_nav_message_.reset(); + navigations_suspended_ = false; +} + void RenderViewHost::FirePageBeforeUnload(bool for_cross_site_transition) { if (!IsRenderViewLive()) { // This RenderViewHost doesn't have a live renderer, so just skip running @@ -295,45 +306,66 @@ void RenderViewHost::FirePageBeforeUnload(bool for_cross_site_transition) { } } -void RenderViewHost::ClosePage(bool for_cross_site_transition, - int new_render_process_host_id, - int new_request_id) { - // This will be set back to false in OnClosePageACK, just before we close the - // tab or replace it with a pending RVH. There are some cases (such as 204 - // errors) where we'll continue to show this RVH. +void RenderViewHost::SwapOut(int new_render_process_host_id, + int new_request_id) { + // Start filtering IPC messages to avoid confusing the delegate. This will + // prevent any dialogs from appearing during unload handlers, but we've + // already decided to silence them in crbug.com/68780. We will set it back + // to false in SetNavigationsSuspended if we swap back in. + is_swapped_out_ = true; + + // This will be set back to false in OnSwapOutACK, just before we replace + // this RVH with the pending RVH. is_waiting_for_unload_ack_ = true; // Start the hang monitor in case the renderer hangs in the unload handler. StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS)); - ViewMsg_ClosePage_Params params; + ViewMsg_SwapOut_Params params; params.closing_process_id = process()->id(); params.closing_route_id = routing_id(); - params.for_cross_site_transition = for_cross_site_transition; params.new_render_process_host_id = new_render_process_host_id; params.new_request_id = new_request_id; if (IsRenderViewLive()) { - NotificationService::current()->Notify( - NotificationType::RENDER_VIEW_HOST_WILL_CLOSE_RENDER_VIEW, - Source<RenderViewHost>(this), - NotificationService::NoDetails()); - - Send(new ViewMsg_ClosePage(routing_id(), params)); + Send(new ViewMsg_SwapOut(routing_id(), params)); } else { - // This RenderViewHost doesn't have a live renderer, so just skip closing - // the page. We must notify the ResourceDispatcherHost on the IO thread, + // This RenderViewHost doesn't have a live renderer, so just skip the unload + // event. We must notify the ResourceDispatcherHost on the IO thread, // which we will do through the RenderProcessHost's widget helper. - process()->CrossSiteClosePageACK(params); + process()->CrossSiteSwapOutACK(params); } } -void RenderViewHost::OnClosePageACK(bool for_cross_site_transition) { +void RenderViewHost::OnSwapOutACK() { + // Stop the hang monitor now that the unload handler has finished. StopHangMonitorTimeout(); is_waiting_for_unload_ack_ = false; +} + +void RenderViewHost::WasSwappedOut() { + // Don't bother reporting hung state anymore. + StopHangMonitorTimeout(); - // If this ClosePageACK is not for a cross-site transition, then it is for an - // attempt to close the tab. We have now finished the unload handler and can - // proceed with closing the tab. - if (!for_cross_site_transition) { + // Inform the renderer that it can exit if no one else is using it. + Send(new ViewMsg_WasSwappedOut(routing_id())); +} + +void RenderViewHost::ClosePage() { + // Start the hang monitor in case the renderer hangs in the unload handler. + is_waiting_for_unload_ack_ = true; + StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS)); + + if (IsRenderViewLive()) { + // TODO(creis): Should this be moved to Shutdown? It may not be called for + // RenderViewHosts that have been swapped out. + NotificationService::current()->Notify( + NotificationType::RENDER_VIEW_HOST_WILL_CLOSE_RENDER_VIEW, + Source<RenderViewHost>(this), + NotificationService::NoDetails()); + + Send(new ViewMsg_ClosePage(routing_id())); + } else { + // This RenderViewHost doesn't have a live renderer, so just skip the unload + // event and close the page. ClosePageIgnoringUnloadEvents(); } } @@ -671,6 +703,12 @@ bool RenderViewHost::OnMessageReceived(const IPC::Message& msg) { if (!BrowserMessageFilter::CheckCanDispatchOnUI(msg, this)) return true; + // Filter out most IPC messages if this renderer is swapped out. + // We still want to certain ACKs to keep our state consistent. + if (is_swapped_out_) + if (!content::SwappedOutMessages::CanHandleWhileSwappedOut(msg)) + return true; + { // delegate_->OnMessageReceived can end up deleting |this|, in which case // the destructor for ObserverListBase::Iterator would access the deleted @@ -728,6 +766,7 @@ bool RenderViewHost::OnMessageReceived(const IPC::Message& msg) { IPC_MESSAGE_HANDLER(ViewHostMsg_TakeFocus, OnTakeFocus) IPC_MESSAGE_HANDLER(ViewHostMsg_AddMessageToConsole, OnAddMessageToConsole) IPC_MESSAGE_HANDLER(ViewHostMsg_ShouldClose_ACK, OnMsgShouldCloseACK) + IPC_MESSAGE_HANDLER(ViewHostMsg_ClosePage_ACK, OnMsgClosePageACK) IPC_MESSAGE_HANDLER(ViewHostMsg_SelectionChanged, OnMsgSelectionChanged) IPC_MESSAGE_HANDLER(ViewHostMsg_AccessibilityNotifications, OnAccessibilityNotifications) @@ -797,7 +836,8 @@ void RenderViewHost::OnMsgShowView(int route_id, bool user_gesture) { RenderViewHostDelegate::View* view = delegate_->GetViewDelegate(); if (view) { - view->ShowCreatedWindow(route_id, disposition, initial_pos, user_gesture); + if (!is_swapped_out_) + view->ShowCreatedWindow(route_id, disposition, initial_pos, user_gesture); Send(new ViewMsg_Move_ACK(route_id)); } } @@ -806,7 +846,8 @@ void RenderViewHost::OnMsgShowWidget(int route_id, const gfx::Rect& initial_pos) { RenderViewHostDelegate::View* view = delegate_->GetViewDelegate(); if (view) { - view->ShowCreatedWidget(route_id, initial_pos); + if (!is_swapped_out_) + view->ShowCreatedWidget(route_id, initial_pos); Send(new ViewMsg_Move_ACK(route_id)); } } @@ -814,7 +855,8 @@ void RenderViewHost::OnMsgShowWidget(int route_id, void RenderViewHost::OnMsgShowFullscreenWidget(int route_id) { RenderViewHostDelegate::View* view = delegate_->GetViewDelegate(); if (view) { - view->ShowCreatedFullscreenWidget(route_id); + if (!is_swapped_out_) + view->ShowCreatedFullscreenWidget(route_id); Send(new ViewMsg_Move_ACK(route_id)); } } @@ -936,7 +978,8 @@ void RenderViewHost::OnMsgUpdateEncoding(const std::string& encoding_name) { void RenderViewHost::OnMsgUpdateTargetURL(int32 page_id, const GURL& url) { - delegate_->UpdateTargetURL(page_id, url); + if (!is_swapped_out_) + delegate_->UpdateTargetURL(page_id, url); // Send a notification back to the renderer that we are ready to // receive more target urls. @@ -955,7 +998,8 @@ void RenderViewHost::OnMsgClose() { } void RenderViewHost::OnMsgRequestMove(const gfx::Rect& pos) { - delegate_->RequestMove(pos); + if (!is_swapped_out_) + delegate_->RequestMove(pos); Send(new ViewMsg_Move_ACK(routing_id())); } @@ -1077,8 +1121,8 @@ void RenderViewHost::OnMsgRunJavaScriptMessage( // process input events. process()->set_ignore_input_events(true); StopHangMonitorTimeout(); - delegate_->RunJavaScriptMessage(message, default_prompt, frame_url, flags, - reply_msg, + delegate_->RunJavaScriptMessage(this, message, default_prompt, frame_url, + flags, reply_msg, &are_javascript_messages_suppressed_); } @@ -1089,7 +1133,7 @@ void RenderViewHost::OnMsgRunBeforeUnloadConfirm(const GURL& frame_url, // shouldn't process input events. process()->set_ignore_input_events(true); StopHangMonitorTimeout(); - delegate_->RunBeforeUnloadConfirm(message, reply_msg); + delegate_->RunBeforeUnloadConfirm(this, message, reply_msg); } void RenderViewHost::MediaPlayerActionAt(const gfx::Point& location, @@ -1110,7 +1154,7 @@ void RenderViewHost::OnMsgStartDragging( const gfx::Point& image_offset) { RenderViewHostDelegate::View* view = delegate_->GetViewDelegate(); if (view) - view->StartDragging(drop_data, drag_operations_mask, image, image_offset); + view->StartDragging(drop_data, drag_operations_mask, image, image_offset); } void RenderViewHost::OnUpdateDragCursor(WebDragOperation current_op) { @@ -1167,7 +1211,7 @@ void RenderViewHost::OnMsgShouldCloseACK(bool proceed) { // If this renderer navigated while the beforeunload request was in flight, we // may have cleared this state in OnMsgNavigate, in which case we can ignore // this message. - if (!is_waiting_for_beforeunload_ack_) + if (!is_waiting_for_beforeunload_ack_ || is_swapped_out_) return; is_waiting_for_beforeunload_ack_ = false; @@ -1184,6 +1228,10 @@ void RenderViewHost::OnMsgShouldCloseACK(bool proceed) { delegate_->DidCancelLoading(); } +void RenderViewHost::OnMsgClosePageACK() { + ClosePageIgnoringUnloadEvents(); +} + void RenderViewHost::WindowMoveOrResizeStarted() { Send(new ViewMsg_MoveOrResizeStarted(routing_id())); } @@ -1326,7 +1374,7 @@ void RenderViewHost::FilterURL(ChildProcessSecurityPolicy* policy, void RenderViewHost::OnAccessibilityNotifications( const std::vector<ViewHostMsg_AccessibilityNotification_Params>& params) { - if (view()) + if (view() && !is_swapped_out_) view()->OnAccessibilityNotifications(params); if (!params.empty()) { diff --git a/content/browser/renderer_host/render_view_host.h b/content/browser/renderer_host/render_view_host.h index bfd7bf85..add44a4 100644 --- a/content/browser/renderer_host/render_view_host.h +++ b/content/browser/renderer_host/render_view_host.h @@ -163,27 +163,48 @@ class RenderViewHost : public RenderWidgetHost { // are_navigations_suspended() first. void SetNavigationsSuspended(bool suspend); + // Clears any suspended navigation state after a cross-site navigation is + // canceled or suspended. This is important if we later return to this + // RenderViewHost. + void CancelSuspendedNavigations(); + + // Whether this RenderViewHost has been swapped out to be displayed by a + // different process. + bool is_swapped_out() const { return is_swapped_out_; } + // Causes the renderer to invoke the onbeforeunload event handler. The - // result will be returned via ViewMsg_ShouldClose. See also ClosePage which - // will fire the PageUnload event. + // result will be returned via ViewMsg_ShouldClose. See also ClosePage and + // SwapOut, which fire the PageUnload event. // // Set bool for_cross_site_transition when this close is just for the current // RenderView in the case of a cross-site transition. False means we're // closing the entire tab. void FirePageBeforeUnload(bool for_cross_site_transition); + // Tells the renderer that this RenderView is being swapped out for one in a + // different renderer process. It should run its unload handler and move to + // a blank document. The renderer should preserve the Frame object until it + // exits, in case we come back. The renderer can exit if it has no other + // active RenderViews, but not until WasSwappedOut is called (when it is no + // longer visible). + // + // Please see ViewMsg_SwapOut_Params in view_messages.h for a description + // of the parameters. + void SwapOut(int new_render_process_host_id, int new_request_id); + + // Called by ResourceDispatcherHost after the SwapOutACK is received. + void OnSwapOutACK(); + + // Called to notify the renderer that it has been visibly swapped out and + // replaced by another RenderViewHost, after an earlier call to SwapOut. + // It is now safe for the process to exit if there are no other active + // RenderViews. + void WasSwappedOut(); + // Causes the renderer to close the current page, including running its // onunload event handler. A ClosePage_ACK message will be sent to the // ResourceDispatcherHost when it is finished. - // - // Please see ViewMsg_ClosePage in resource_messages_internal.h for a - // description of the parameters. - void ClosePage(bool for_cross_site_transition, - int new_render_process_host_id, - int new_request_id); - - // Called by ResourceDispatcherHost after the ClosePageACK is received. - void OnClosePageACK(bool for_cross_site_transition); + void ClosePage(); // Close the page ignoring whether it has unload events registers. // This is called after the beforeunload and unload events have fired @@ -516,6 +537,7 @@ class RenderViewHost : public RenderWidgetHost { void OnUpdateInspectorSetting(const std::string& key, const std::string& value); void OnMsgShouldCloseACK(bool proceed); + void OnMsgClosePageACK(); void OnAccessibilityNotifications( const std::vector<ViewHostMsg_AccessibilityNotification_Params>& params); @@ -570,6 +592,10 @@ class RenderViewHost : public RenderWidgetHost { // navigation occurs. scoped_ptr<ViewMsg_Navigate> suspended_nav_message_; + // Whether this RenderViewHost is currently swapped out, such that the view is + // being rendered by another process. + bool is_swapped_out_; + // If we were asked to RunModal, then this will hold the reply_msg that we // must return to the renderer to unblock it. IPC::Message* run_modal_reply_msg_; diff --git a/content/browser/renderer_host/render_view_host_delegate.h b/content/browser/renderer_host/render_view_host_delegate.h index 9e8cb54..cb48801 100644 --- a/content/browser/renderer_host/render_view_host_delegate.h +++ b/content/browser/renderer_host/render_view_host_delegate.h @@ -476,14 +476,16 @@ class RenderViewHostDelegate : public IPC::Channel::Listener { const std::string& target) {} // A javascript message, confirmation or prompt should be shown. - virtual void RunJavaScriptMessage(const std::wstring& message, + virtual void RunJavaScriptMessage(const RenderViewHost* rvh, + const std::wstring& message, const std::wstring& default_prompt, const GURL& frame_url, const int flags, IPC::Message* reply_msg, bool* did_suppress_message) {} - virtual void RunBeforeUnloadConfirm(const std::wstring& message, + virtual void RunBeforeUnloadConfirm(const RenderViewHost* rvh, + const std::wstring& message, IPC::Message* reply_msg) {} // |url| is assigned to a server that can provide alternate error pages. If diff --git a/content/browser/renderer_host/render_view_host_unittest.cc b/content/browser/renderer_host/render_view_host_unittest.cc index 26b8e39..315cdbc 100644 --- a/content/browser/renderer_host/render_view_host_unittest.cc +++ b/content/browser/renderer_host/render_view_host_unittest.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2009 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -48,10 +48,9 @@ TEST_F(RenderViewHostTest, ResetUnloadOnReload) { NavigateAndCommit(url1); controller().LoadURL(url2, GURL(), PageTransition::LINK); // Simulate the ClosePage call which is normally sent by the net::URLRequest. - rvh()->ClosePage(true, 0, 0); - // Needed so that navigations are not suspended on the RVH. Normally handled - // by way of ViewHostMsg_ShouldClose_ACK. - contents()->render_manager()->ShouldClosePage(true, true); + rvh()->ClosePage(); + // Needed so that navigations are not suspended on the RVH. + rvh()->SendShouldCloseACK(true); contents()->Stop(); controller().Reload(false); EXPECT_FALSE(rvh()->is_waiting_for_unload_ack()); diff --git a/content/browser/renderer_host/render_widget_helper.cc b/content/browser/renderer_host/render_widget_helper.cc index 161bf88..f297110 100644 --- a/content/browser/renderer_host/render_widget_helper.cc +++ b/content/browser/renderer_host/render_widget_helper.cc @@ -1,4 +1,4 @@ -// Copyright (c) 2010 The Chromium Authors. All rights reserved. +// Copyright (c) 2011 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. @@ -85,12 +85,12 @@ void RenderWidgetHelper::CancelResourceRequests(int render_widget_id) { render_widget_id)); } -void RenderWidgetHelper::CrossSiteClosePageACK( - const ViewMsg_ClosePage_Params& params) { +void RenderWidgetHelper::CrossSiteSwapOutACK( + const ViewMsg_SwapOut_Params& params) { BrowserThread::PostTask( BrowserThread::IO, FROM_HERE, NewRunnableMethod(this, - &RenderWidgetHelper::OnCrossSiteClosePageACK, + &RenderWidgetHelper::OnCrossSiteSwapOutACK, params)); } @@ -202,9 +202,9 @@ void RenderWidgetHelper::OnCancelResourceRequests( render_process_id_, render_widget_id); } -void RenderWidgetHelper::OnCrossSiteClosePageACK( - const ViewMsg_ClosePage_Params& params) { - resource_dispatcher_host_->OnClosePageACK(params); +void RenderWidgetHelper::OnCrossSiteSwapOutACK( + const ViewMsg_SwapOut_Params& params) { + resource_dispatcher_host_->OnSwapOutACK(params); } void RenderWidgetHelper::CreateNewWindow( diff --git a/content/browser/renderer_host/render_widget_helper.h b/content/browser/renderer_host/render_widget_helper.h index 6576700..0c7f695 100644 --- a/content/browser/renderer_host/render_widget_helper.h +++ b/content/browser/renderer_host/render_widget_helper.h @@ -28,7 +28,7 @@ class TimeDelta; class ResourceDispatcherHost; struct ViewHostMsg_CreateWindow_Params; -struct ViewMsg_ClosePage_Params; +struct ViewMsg_SwapOut_Params; // Instantiated per RenderProcessHost to provide various optimizations on @@ -107,7 +107,7 @@ class RenderWidgetHelper // corresponding functions in RenderProcessHost. See those declarations // for documentation. void CancelResourceRequests(int render_widget_id); - void CrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params); + void CrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params); bool WaitForUpdateMsg(int render_widget_id, const base::TimeDelta& max_delay, IPC::Message* msg); @@ -182,7 +182,7 @@ class RenderWidgetHelper void OnCancelResourceRequests(int render_widget_id); // Called on the IO thread to resume a cross-site response. - void OnCrossSiteClosePageACK(const ViewMsg_ClosePage_Params& params); + void OnCrossSiteSwapOutACK(const ViewMsg_SwapOut_Params& params); #if defined(OS_MACOSX) // Called on destruction to release all allocated transport DIBs diff --git a/content/browser/renderer_host/resource_dispatcher_host.cc b/content/browser/renderer_host/resource_dispatcher_host.cc index ce775c7..a9525dd 100644 --- a/content/browser/renderer_host/resource_dispatcher_host.cc +++ b/content/browser/renderer_host/resource_dispatcher_host.cc @@ -345,7 +345,7 @@ bool ResourceDispatcherHost::OnMessageReceived(const IPC::Message& message, IPC_MESSAGE_HANDLER(ResourceHostMsg_UploadProgress_ACK, OnUploadProgressACK) IPC_MESSAGE_HANDLER(ResourceHostMsg_CancelRequest, OnCancelRequest) IPC_MESSAGE_HANDLER(ResourceHostMsg_FollowRedirect, OnFollowRedirect) - IPC_MESSAGE_HANDLER(ViewHostMsg_ClosePage_ACK, OnClosePageACK) + IPC_MESSAGE_HANDLER(ViewHostMsg_SwapOut_ACK, OnSwapOutACK) IPC_MESSAGE_UNHANDLED(handled = false) IPC_END_MESSAGE_MAP_EX() @@ -717,30 +717,23 @@ ResourceDispatcherHost::CreateRequestInfoForBrowserRequest( &context); } -void ResourceDispatcherHost::OnClosePageACK( - const ViewMsg_ClosePage_Params& params) { - if (params.for_cross_site_transition) { - // Closes for cross-site transitions are handled such that the cross-site - // transition continues. - GlobalRequestID global_id(params.new_render_process_host_id, - params.new_request_id); - PendingRequestList::iterator i = pending_requests_.find(global_id); - if (i != pending_requests_.end()) { - // The response we were meant to resume could have already been canceled. - ResourceDispatcherHostRequestInfo* info = InfoForRequest(i->second); - if (info->cross_site_handler()) - info->cross_site_handler()->ResumeResponse(); - } - } else { - // This is a tab close, so we will close the tab in OnClosePageACK. - DCHECK(params.new_render_process_host_id == -1); - DCHECK(params.new_request_id == -1); +void ResourceDispatcherHost::OnSwapOutACK( + const ViewMsg_SwapOut_Params& params) { + // Closes for cross-site transitions are handled such that the cross-site + // transition continues. + GlobalRequestID global_id(params.new_render_process_host_id, + params.new_request_id); + PendingRequestList::iterator i = pending_requests_.find(global_id); + if (i != pending_requests_.end()) { + // The response we were meant to resume could have already been canceled. + ResourceDispatcherHostRequestInfo* info = InfoForRequest(i->second); + if (info->cross_site_handler()) + info->cross_site_handler()->ResumeResponse(); } // Update the RenderViewHost's internal state after the ACK. CallRenderViewHost(params.closing_process_id, params.closing_route_id, - &RenderViewHost::OnClosePageACK, - params.for_cross_site_transition); + &RenderViewHost::OnSwapOutACK); } // We are explicitly forcing the download of 'url'. diff --git a/content/browser/renderer_host/resource_dispatcher_host.h b/content/browser/renderer_host/resource_dispatcher_host.h index a8b621e..2354a23 100644 --- a/content/browser/renderer_host/resource_dispatcher_host.h +++ b/content/browser/renderer_host/resource_dispatcher_host.h @@ -44,7 +44,7 @@ class WebKitThread; struct DownloadSaveInfo; struct GlobalRequestID; struct ResourceHostMsg_Request; -struct ViewMsg_ClosePage_Params; +struct ViewMsg_SwapOut_Params; namespace content { class ResourceContext; @@ -157,8 +157,8 @@ class ResourceDispatcherHost : public net::URLRequest::Delegate { return webkit_thread_.get(); } - // Called when the onunload handler for a cross-site request has finished. - void OnClosePageACK(const ViewMsg_ClosePage_Params& params); + // Called when the unload handler for a cross-site request has finished. + void OnSwapOutACK(const ViewMsg_SwapOut_Params& params); // Force cancels any pending requests for the given process. void CancelRequestsForProcess(int process_unique_id); diff --git a/content/browser/renderer_host/resource_dispatcher_host_uitest.cc b/content/browser/renderer_host/resource_dispatcher_host_uitest.cc index 5de1430..7f21171 100644 --- a/content/browser/renderer_host/resource_dispatcher_host_uitest.cc +++ b/content/browser/renderer_host/resource_dispatcher_host_uitest.cc @@ -193,6 +193,38 @@ TEST_F(ResourceDispatcherTest, CrossSiteOnunloadCookie) { ASSERT_STREQ("foo", value_result.c_str()); } +// Tests that onunload is run for cross-site requests to URLs that complete +// without network loads (e.g., about:blank, data URLs). +TEST_F(ResourceDispatcherTest, CrossSiteImmediateLoadOnunloadCookie) { + net::TestServer test_server(net::TestServer::TYPE_HTTP, + FilePath(FILE_PATH_LITERAL("chrome/test/data"))); + ASSERT_TRUE(test_server.Start()); + + scoped_refptr<BrowserProxy> browser_proxy(automation()->GetBrowserWindow(0)); + ASSERT_TRUE(browser_proxy.get()); + scoped_refptr<TabProxy> tab(browser_proxy->GetActiveTab()); + ASSERT_TRUE(tab.get()); + + GURL url(test_server.GetURL("files/onunload_cookie.html")); + ASSERT_EQ(AUTOMATION_MSG_NAVIGATION_SUCCESS, tab->NavigateToURL(url)); + + // Confirm that the page has loaded (since it changes its title during load). + std::wstring tab_title; + EXPECT_TRUE(tab->GetTabTitle(&tab_title)); + EXPECT_EQ(L"set cookie on unload", tab_title); + + // Navigate to a cross-site page that loads immediately without making a + // network request. The unload event should still be run. + ASSERT_EQ(AUTOMATION_MSG_NAVIGATION_SUCCESS, + tab->NavigateToURL(GURL("about:blank"))); + + // Check that the cookie was set. + std::string value_result; + ASSERT_TRUE(tab->GetCookieByName(url, "onunloadCookie", &value_result)); + ASSERT_FALSE(value_result.empty()); + ASSERT_STREQ("foo", value_result.c_str()); +} + // Tests that the unload handler is not run for 204 responses. TEST_F(ResourceDispatcherTest, CrossSiteNoUnloadOn204) { net::TestServer test_server(net::TestServer::TYPE_HTTP, diff --git a/content/browser/renderer_host/test_render_view_host.cc b/content/browser/renderer_host/test_render_view_host.cc index 0ff0194..fd488d2 100644 --- a/content/browser/renderer_host/test_render_view_host.cc +++ b/content/browser/renderer_host/test_render_view_host.cc @@ -108,6 +108,10 @@ void TestRenderViewHost::SendNavigateWithTransition( OnMsgNavigate(msg); } +void TestRenderViewHost::SendShouldCloseACK(bool proceed) { + OnMsgShouldCloseACK(proceed); +} + void TestRenderViewHost::set_simulate_fetch_via_proxy(bool proxy) { simulate_fetch_via_proxy_ = proxy; } diff --git a/content/browser/renderer_host/test_render_view_host.h b/content/browser/renderer_host/test_render_view_host.h index a860806..37312cc 100644 --- a/content/browser/renderer_host/test_render_view_host.h +++ b/content/browser/renderer_host/test_render_view_host.h @@ -182,6 +182,9 @@ class TestRenderViewHost : public RenderViewHost { void SendNavigateWithTransition(int page_id, const GURL& url, PageTransition::Type transition); + // Calls OnMsgShouldCloseACK on the RenderViewHost with the given parameter. + void SendShouldCloseACK(bool proceed); + // If set, *delete_counter is incremented when this object destructs. void set_delete_counter(int* delete_counter) { delete_counter_ = delete_counter; diff --git a/content/browser/site_instance.cc b/content/browser/site_instance.cc index a121109..ed8cd9b 100644 --- a/content/browser/site_instance.cc +++ b/content/browser/site_instance.cc @@ -25,8 +25,11 @@ static bool IsURLSameAsAnySiteInstance(const GURL& url) { url.spec() == chrome::kAboutShorthangURL; } +int32 SiteInstance::next_site_instance_id_ = 1; + SiteInstance::SiteInstance(BrowsingInstance* browsing_instance) - : browsing_instance_(browsing_instance), + : id_(next_site_instance_id_++), + browsing_instance_(browsing_instance), render_process_host_factory_(NULL), process_(NULL), max_page_id_(-1), diff --git a/content/browser/site_instance.h b/content/browser/site_instance.h index 76f966e..3b6346d 100644 --- a/content/browser/site_instance.h +++ b/content/browser/site_instance.h @@ -50,6 +50,9 @@ class BrowsingInstance; class SiteInstance : public base::RefCounted<SiteInstance>, public NotificationObserver { public: + // Returns a unique ID for this SiteInstance. + int32 id() { return id_; } + // Get the BrowsingInstance to which this SiteInstance belongs. BrowsingInstance* browsing_instance() { return browsing_instance_; } @@ -162,6 +165,12 @@ class SiteInstance : public base::RefCounted<SiteInstance>, const NotificationSource& source, const NotificationDetails& details); + // The next available SiteInstance ID. + static int32 next_site_instance_id_; + + // A unique ID for this SiteInstance. + int32 id_; + NotificationRegistrar registrar_; // BrowsingInstance to which this SiteInstance belongs. diff --git a/content/browser/tab_contents/render_view_host_manager.cc b/content/browser/tab_contents/render_view_host_manager.cc index 22ae236..bac5374 100644 --- a/content/browser/tab_contents/render_view_host_manager.cc +++ b/content/browser/tab_contents/render_view_host_manager.cc @@ -49,6 +49,13 @@ RenderViewHostManager::~RenderViewHostManager() { RenderViewHost* render_view_host = render_view_host_; render_view_host_ = NULL; render_view_host->Shutdown(); + + // Shut down any swapped out RenderViewHosts. + for (RenderViewHostMap::iterator iter = swapped_out_hosts_.begin(); + iter != swapped_out_hosts_.end(); + ++iter) { + iter->second->Shutdown(); + } } void RenderViewHostManager::Init(Profile* profile, @@ -152,21 +159,17 @@ bool RenderViewHostManager::ShouldCloseTabOnUnresponsiveRenderer() { if (pending_render_view_host_->are_navigations_suspended()) pending_render_view_host_->SetNavigationsSuspended(false); } else { - // The request has been started and paused, while we're waiting for the + // The request has been started and paused while we're waiting for the // unload handler to finish. We'll pretend that it did, by notifying the // IO thread to let the response continue. The pending renderer will then // be swapped in as part of the usual DidNavigate logic. (If the unload // handler later finishes, this call will be ignored because the state in // CrossSiteResourceHandler will already be cleaned up.) - ViewMsg_ClosePage_Params params; - params.closing_process_id = - render_view_host_->process()->id(); - params.closing_route_id = render_view_host_->routing_id(); - params.for_cross_site_transition = true; + ViewMsg_SwapOut_Params params; params.new_render_process_host_id = pending_render_view_host_->process()->id(); params.new_request_id = pending_request_id; - current_host()->process()->CrossSiteClosePageACK(params); + current_host()->process()->CrossSiteSwapOutACK(params); } return false; } @@ -187,6 +190,14 @@ void RenderViewHostManager::DidNavigateMainFrame( if (render_view_host == pending_render_view_host_) { // The pending cross-site navigation completed, so show the renderer. + // If it committed without sending network requests (e.g., data URLs), + // then we still need to swap out the old RVH first and run its unload + // handler. OK for that to happen in the background. + if (pending_render_view_host_->GetPendingRequestId() == -1) { + OnCrossSiteResponse(pending_render_view_host_->process()->id(), + pending_render_view_host_->routing_id()); + } + CommitPending(); cross_navigation_pending_ = false; } else if (render_view_host == render_view_host_) { @@ -221,8 +232,23 @@ void RenderViewHostManager::RendererAbortedProvisionalLoad( void RenderViewHostManager::RendererProcessClosing( RenderProcessHost* render_process_host) { - // TODO(creis): Don't schedule new navigations in RenderViewHosts of this - // process. (Part of http://crbug.com/65953.) + // Remove any swapped out RVHs from this process, so that we don't try to + // swap them back in while the process is exiting. Start by finding them, + // since there could be more than one. + std::list<int> ids_to_remove; + for (RenderViewHostMap::iterator iter = swapped_out_hosts_.begin(); + iter != swapped_out_hosts_.end(); + ++iter) { + if (iter->second->process() == render_process_host) + ids_to_remove.push_back(iter->first); + } + + // Now delete them. + while (!ids_to_remove.empty()) { + swapped_out_hosts_[ids_to_remove.back()]->Shutdown(); + swapped_out_hosts_.erase(ids_to_remove.back()); + ids_to_remove.pop_back(); + } } void RenderViewHostManager::ShouldClosePage(bool for_cross_site_transition, @@ -254,7 +280,7 @@ void RenderViewHostManager::ShouldClosePage(bool for_cross_site_transition, if (proceed_to_fire_unload) { // This is not a cross-site navigation, the tab is being closed. - render_view_host_->ClosePage(false, -1, -1); + render_view_host_->ClosePage(); } } } @@ -266,12 +292,12 @@ void RenderViewHostManager::OnCrossSiteResponse(int new_render_process_host_id, return; DCHECK(pending_render_view_host_); - // Tell the old renderer to run its onunload handler. When it finishes, it - // will send a ClosePage_ACK to the ResourceDispatcherHost with the given - // IDs (of the pending RVH's request), allowing the pending RVH's response to - // resume. - render_view_host_->ClosePage(true, - new_render_process_host_id, new_request_id); + // Tell the old renderer it is being swapped out. This will fire the unload + // handler (without firing the beforeunload handler a second time). When the + // unload handler finishes and the navigation completes, we will send a + // message to the ResourceDispatcherHost with the given pending request IDs, + // allowing the pending RVH's response to resume. + render_view_host_->SwapOut(new_render_process_host_id, new_request_id); // ResourceDispatcherHost has told us to run the onunload handler, which // means it is not a download or unsafe page, and we are going to perform the @@ -482,6 +508,21 @@ bool RenderViewHostManager::CreatePendingRenderView( // we're about to switch away, so that it sends an UpdateState message. } + // Check if we've already created an RVH for this SiteInstance. + CHECK(instance); + RenderViewHostMap::iterator iter = + swapped_out_hosts_.find(instance->id()); + if (iter != swapped_out_hosts_.end()) { + // Re-use the existing RenderViewHost, which has already been initialized. + // We'll remove it from the list of swapped out hosts if it commits. + pending_render_view_host_ = iter->second; + + // Prevent the process from exiting while we're trying to use it. + pending_render_view_host_->process()->AddPendingView(); + + return true; + } + pending_render_view_host_ = RenderViewHostFactory::Create( instance, render_view_delegate_, MSG_ROUTING_NONE, delegate_-> GetControllerForRenderManager().session_storage_namespace()); @@ -541,14 +582,8 @@ void RenderViewHostManager::CommitPending() { bool focus_render_view = !will_focus_location_bar && render_view_host_->view() && render_view_host_->view()->HasFocus(); - // Hide the current view and prepare to destroy it. - // TODO(creis): Get the old RenderViewHost to send us an UpdateState message - // before we destroy it. - if (render_view_host_->view()) - render_view_host_->view()->Hide(); - RenderViewHost* old_render_view_host = render_view_host_; - // Swap in the pending view and make it active. + RenderViewHost* old_render_view_host = render_view_host_; render_view_host_ = pending_render_view_host_; pending_render_view_host_ = NULL; @@ -563,6 +598,12 @@ void RenderViewHostManager::CommitPending() { else delegate_->RenderViewGoneFromRenderManager(render_view_host_); + // Hide the old view now that the new one is visible. + if (old_render_view_host->view()) { + old_render_view_host->view()->Hide(); + old_render_view_host->WasSwappedOut(); + } + // Make sure the size is up to date. (Fix for bug 1079768.) delegate_->UpdateRenderViewSizeForRenderManager(); @@ -579,7 +620,18 @@ void RenderViewHostManager::CommitPending() { Source<NavigationController>(&delegate_->GetControllerForRenderManager()), Details<RenderViewHostSwitchedDetails>(&details)); - old_render_view_host->Shutdown(); + // If the pending view was on the swapped out list, we can remove it. + swapped_out_hosts_.erase(render_view_host_->site_instance()->id()); + + // If the old RVH is live, we are swapping it out and should keep track of it + // in case we navigate back to it. + if (old_render_view_host->IsRenderViewLive()) { + DCHECK(old_render_view_host->is_swapped_out()); + swapped_out_hosts_[old_render_view_host->site_instance()->id()] = + old_render_view_host; + } else { + old_render_view_host->Shutdown(); + } // Let the task manager know that we've swapped RenderViewHosts, since it // might need to update its process groupings. @@ -689,12 +741,26 @@ RenderViewHost* RenderViewHostManager::UpdateRendererStateForNavigate( void RenderViewHostManager::CancelPending() { RenderViewHost* pending_render_view_host = pending_render_view_host_; + pending_render_view_host_ = NULL; // We no longer need to prevent the process from exiting. pending_render_view_host->process()->RemovePendingView(); - pending_render_view_host_ = NULL; - pending_render_view_host->Shutdown(); + // The pending RVH may already be on the swapped out list if we started to + // swap it back in and then canceled. If so, make sure it gets swapped out + // again. If it's not on the swapped out list (e.g., aborting a pending + // load), then it's safe to shut down. + if (IsSwappedOut(pending_render_view_host)) { + // Any currently suspended navigations are no longer needed. + pending_render_view_host->CancelSuspendedNavigations(); + + // We can pass -1,-1 because there is no pending response in the + // ResourceDispatcherHost to unpause. + pending_render_view_host->SwapOut(-1, -1); + } else { + // We won't be coming back, so shut this one down. + pending_render_view_host->Shutdown(); + } pending_web_ui_.reset(); } @@ -711,17 +777,33 @@ void RenderViewHostManager::RenderViewDeleted(RenderViewHost* rvh) { NOTREACHED(); pending_render_view_host_ = NULL; } + + // Make sure deleted RVHs are not kept in the swapped out list while we are + // still alive. (If render_view_host_ is null, we're already being deleted.) + if (!render_view_host_) + return; + // We can't look it up by SiteInstance ID, which may no longer be valid. + for (RenderViewHostMap::iterator iter = swapped_out_hosts_.begin(); + iter != swapped_out_hosts_.end(); + ++iter) { + if (iter->second == rvh) { + swapped_out_hosts_.erase(iter); + break; + } + } } void RenderViewHostManager::SwapInRenderViewHost(RenderViewHost* rvh) { + // TODO(creis): Abstract out the common code between this and CommitPending. web_ui_.reset(); - // Hide the current view and prepare to destroy it. - if (render_view_host_->view()) - render_view_host_->view()->Hide(); - RenderViewHost* old_render_view_host = render_view_host_; + // Make sure the current RVH is swapped out so that it filters out any + // disruptive messages from the renderer. We can pass -1,-1 because there is + // no pending response in the ResourceDispatcherHost to unpause. + render_view_host_->SwapOut(-1, -1); // Swap in the new view and make it active. + RenderViewHost* old_render_view_host = render_view_host_; render_view_host_ = rvh; render_view_host_->set_delegate(render_view_delegate_); // Remove old RenderWidgetHostView with mocked out methods so it can be @@ -743,6 +825,12 @@ void RenderViewHostManager::SwapInRenderViewHost(RenderViewHost* rvh) { render_view_host_->view()->Show(); } + // Hide the current view and prepare to swap it out. + if (old_render_view_host->view()) { + old_render_view_host->view()->Hide(); + old_render_view_host->WasSwappedOut(); + } + delegate_->UpdateRenderViewSizeForRenderManager(); RenderViewHostSwitchedDetails details; @@ -753,10 +841,28 @@ void RenderViewHostManager::SwapInRenderViewHost(RenderViewHost* rvh) { Source<NavigationController>(&delegate_->GetControllerForRenderManager()), Details<RenderViewHostSwitchedDetails>(&details)); - // This will cause the old RenderViewHost to delete itself. - old_render_view_host->Shutdown(); + // If the given RVH was on the swapped out list, we can remove it. + swapped_out_hosts_.erase(render_view_host_->site_instance()->id()); + + // If the old RVH is live, we are swapping it out and should keep track of it + // in case we navigate back to it. + if (old_render_view_host->IsRenderViewLive()) { + DCHECK(old_render_view_host->is_swapped_out()); + swapped_out_hosts_[old_render_view_host->site_instance()->id()] = + old_render_view_host; + } else { + old_render_view_host->Shutdown(); + } // Let the task manager know that we've swapped RenderViewHosts, since it // might need to update its process groupings. delegate_->NotifySwappedFromRenderManager(); } + +bool RenderViewHostManager::IsSwappedOut(RenderViewHost* rvh) { + if (!rvh->site_instance()) + return false; + + return swapped_out_hosts_.find(rvh->site_instance()->id()) != + swapped_out_hosts_.end(); +} diff --git a/content/browser/tab_contents/render_view_host_manager.h b/content/browser/tab_contents/render_view_host_manager.h index e2033d15..6dc4d7d 100644 --- a/content/browser/tab_contents/render_view_host_manager.h +++ b/content/browser/tab_contents/render_view_host_manager.h @@ -12,6 +12,7 @@ #include "content/browser/renderer_host/render_view_host_delegate.h" #include "content/common/notification_observer.h" #include "content/common/notification_registrar.h" +#include "content/browser/site_instance.h" class WebUI; class InterstitialPage; @@ -20,7 +21,6 @@ class NavigationEntry; class Profile; class RenderWidgetHostView; class RenderViewHost; -class SiteInstance; // Manages RenderViewHosts for a TabContents. Normally there is only one and // it is easy to do. But we can also have transitions of processes (and hence @@ -186,6 +186,10 @@ class RenderViewHostManager // deleted. void SwapInRenderViewHost(RenderViewHost* rvh); + // Returns whether the given RenderViewHost is on the list of swapped out + // RenderViewHosts. + bool IsSwappedOut(RenderViewHost* rvh); + private: friend class TestTabContents; friend class RenderViewHostManagerTest; @@ -265,6 +269,10 @@ class RenderViewHostManager RenderViewHost* pending_render_view_host_; scoped_ptr<WebUI> pending_web_ui_; + // A map of site instance ID to swapped out RenderViewHosts. + typedef base::hash_map<int32, RenderViewHost*> RenderViewHostMap; + RenderViewHostMap swapped_out_hosts_; + // The intersitial page currently shown if any, not own by this class // (the InterstitialPage is self-owned, it deletes itself when hidden). InterstitialPage* interstitial_page_; diff --git a/content/browser/tab_contents/render_view_host_manager_unittest.cc b/content/browser/tab_contents/render_view_host_manager_unittest.cc index 591d8b7..c5ada3f 100644 --- a/content/browser/tab_contents/render_view_host_manager_unittest.cc +++ b/content/browser/tab_contents/render_view_host_manager_unittest.cc @@ -28,10 +28,23 @@ class RenderViewHostManagerTest : public RenderViewHostTestHarness { // won't have committed yet, so NavigateAndCommit does the wrong thing // for us. controller().LoadURL(url, GURL(), PageTransition::LINK); + TestRenderViewHost* old_rvh = rvh(); + + // Simulate the ShouldClose_ACK that is received from the current renderer + // for a cross-site navigation. + if (old_rvh != active_rvh()) + old_rvh->SendShouldCloseACK(true); + + // Commit the navigation. active_rvh()->SendNavigate( static_cast<MockRenderProcessHost*>(active_rvh()->process())-> max_page_id() + 1, url); + + // Simulate the SwapOut_ACK that fires if you commit a cross-site navigation + // without making any network requests. + if (old_rvh != active_rvh()) + old_rvh->OnSwapOutACK(); } bool ShouldSwapProcesses(RenderViewHostManager* manager, @@ -61,26 +74,30 @@ TEST_F(RenderViewHostManagerTest, NewTabPageProcesses) { // a RVH that's not pending (since there is no cross-site transition), so // we use the committed one, but the second one is the opposite. contents2.controller().LoadURL(ntp, GURL(), PageTransition::LINK); - static_cast<TestRenderViewHost*>(contents2.render_manager()-> - current_host())->SendNavigate(100, ntp); + TestRenderViewHost* ntp_rvh2 = static_cast<TestRenderViewHost*>( + contents2.render_manager()->current_host()); + ntp_rvh2->SendNavigate(100, ntp); contents2.controller().LoadURL(dest, GURL(), PageTransition::LINK); - static_cast<TestRenderViewHost*>(contents2.render_manager()-> - pending_render_view_host())->SendNavigate(101, dest); + TestRenderViewHost* dest_rvh2 = static_cast<TestRenderViewHost*>( + contents2.render_manager()->pending_render_view_host()); + dest_rvh2->SendNavigate(101, dest); + ntp_rvh2->OnSwapOutACK(); // The two RVH's should be different in every way. - EXPECT_NE(active_rvh()->process(), contents2.render_view_host()->process()); - EXPECT_NE(active_rvh()->site_instance(), - contents2.render_view_host()->site_instance()); + EXPECT_NE(active_rvh()->process(), dest_rvh2->process()); + EXPECT_NE(active_rvh()->site_instance(), dest_rvh2->site_instance()); EXPECT_NE(active_rvh()->site_instance()->browsing_instance(), - contents2.render_view_host()->site_instance()->browsing_instance()); + dest_rvh2->site_instance()->browsing_instance()); // Navigate both to the new tab page, and verify that they share a // SiteInstance. NavigateActiveAndCommit(ntp); contents2.controller().LoadURL(ntp, GURL(), PageTransition::LINK); + dest_rvh2->SendShouldCloseACK(true); static_cast<TestRenderViewHost*>(contents2.render_manager()-> pending_render_view_host())->SendNavigate(102, ntp); + dest_rvh2->OnSwapOutACK(); EXPECT_EQ(active_rvh()->site_instance(), contents2.render_view_host()->site_instance()); diff --git a/content/browser/tab_contents/tab_contents.cc b/content/browser/tab_contents/tab_contents.cc index 02b6386..4d33f4e 100644 --- a/content/browser/tab_contents/tab_contents.cc +++ b/content/browser/tab_contents/tab_contents.cc @@ -107,14 +107,17 @@ // - After RDH receives a response and determines that it is safe and not a // download, it pauses the response to first run the old page's onunload // handler. It does this by asynchronously calling the OnCrossSiteResponse -// method of TabContents on the UI thread, which sends a ClosePage message +// method of TabContents on the UI thread, which sends a SwapOut message // to the current RVH. -// - Once the onunload handler is finished, a ClosePage_ACK message is sent to +// - Once the onunload handler is finished, a SwapOut_ACK message is sent to // the ResourceDispatcherHost, who unpauses the response. Data is then sent // to the pending RVH. // - The pending renderer sends a FrameNavigate message that invokes the // DidNavigate method. This replaces the current RVH with the // pending RVH and goes back to the NORMAL RendererState. +// - The previous renderer is kept swapped out in RenderViewHostManager in case +// the user goes back. The process only stays live if another tab is using +// it, but if so, the existing frame relationships will be maintained. namespace { @@ -1703,7 +1706,9 @@ void TabContents::DidNavigate(RenderViewHost* rvh, void TabContents::UpdateState(RenderViewHost* rvh, int32 page_id, const std::string& state) { - DCHECK(rvh == render_view_host()); + // Ensure that this state update comes from either the active RVH or one of + // the swapped out RVHs. We don't expect to hear from any other RVHs. + DCHECK(rvh == render_view_host() || render_manager_.IsSwappedOut(rvh)); // We must be prepared to handle state updates for any page, these occur // when the user is scrolling and entering form data, as well as when we're @@ -1712,7 +1717,7 @@ void TabContents::UpdateState(RenderViewHost* rvh, // NavigationEntry and update it when it is notified via the delegate. int entry_index = controller_.GetEntryIndexWithPageID( - GetSiteInstance(), page_id); + rvh->site_instance(), page_id); if (entry_index < 0) return; NavigationEntry* entry = controller_.GetEntryAtIndex(entry_index); @@ -1874,6 +1879,7 @@ void TabContents::ProcessExternalHostMessage(const std::string& message, } void TabContents::RunJavaScriptMessage( + const RenderViewHost* rvh, const std::wstring& message, const std::wstring& default_prompt, const GURL& frame_url, @@ -1887,6 +1893,7 @@ void TabContents::RunJavaScriptMessage( // shown over the previous page, we don't want the hidden page dialogs to // interfere with the interstitial. bool suppress_this_message = + rvh->is_swapped_out() || suppress_javascript_messages_ || showing_interstitial_page() || (delegate() && delegate()->ShouldSuppressDialogs()); @@ -1916,11 +1923,14 @@ void TabContents::RunJavaScriptMessage( } } -void TabContents::RunBeforeUnloadConfirm(const std::wstring& message, +void TabContents::RunBeforeUnloadConfirm(const RenderViewHost* rvh, + const std::wstring& message, IPC::Message* reply_msg) { if (delegate()) delegate()->WillRunBeforeUnloadConfirm(); - if (delegate() && delegate()->ShouldSuppressDialogs()) { + bool suppress_this_message = rvh->is_swapped_out() || + (delegate() && delegate()->ShouldSuppressDialogs()); + if (suppress_this_message) { render_view_host()->JavaScriptMessageBoxClosed(reply_msg, true, std::wstring()); return; @@ -1984,6 +1994,10 @@ void TabContents::OnCrossSiteResponse(int new_render_process_host_id, void TabContents::RendererUnresponsive(RenderViewHost* rvh, bool is_during_unload) { + // Don't show hung renderer dialog for a swapped out RVH. + if (rvh != render_view_host()) + return; + if (is_during_unload) { // Hang occurred while firing the beforeunload/unload handler. // Pretend the handler fired so tab closing continues as if it had. diff --git a/content/browser/tab_contents/tab_contents.h b/content/browser/tab_contents/tab_contents.h index 8cb9cfe7..6f124435 100644 --- a/content/browser/tab_contents/tab_contents.h +++ b/content/browser/tab_contents/tab_contents.h @@ -758,13 +758,15 @@ class TabContents : public PageNavigator, virtual void ProcessExternalHostMessage(const std::string& message, const std::string& origin, const std::string& target); - virtual void RunJavaScriptMessage(const std::wstring& message, + virtual void RunJavaScriptMessage(const RenderViewHost* rvh, + const std::wstring& message, const std::wstring& default_prompt, const GURL& frame_url, const int flags, IPC::Message* reply_msg, bool* did_suppress_message); - virtual void RunBeforeUnloadConfirm(const std::wstring& message, + virtual void RunBeforeUnloadConfirm(const RenderViewHost* rvh, + const std::wstring& message, IPC::Message* reply_msg); virtual GURL GetAlternateErrorPageURL() const; virtual RendererPreferences GetRendererPrefs(Profile* profile) const; diff --git a/content/browser/tab_contents/test_tab_contents.cc b/content/browser/tab_contents/test_tab_contents.cc index 33a5a2b..18860fc 100644 --- a/content/browser/tab_contents/test_tab_contents.cc +++ b/content/browser/tab_contents/test_tab_contents.cc @@ -85,9 +85,10 @@ void TestTabContents::CommitPendingNavigation() { // notifying that it has unloaded so the pending RVH is resumed and can // navigate. ProceedWithCrossSiteNavigation(); + RenderViewHost* old_rvh = render_manager_.current_host(); TestRenderViewHost* rvh = pending_rvh(); if (!rvh) - rvh = static_cast<TestRenderViewHost*>(render_manager_.current_host()); + rvh = static_cast<TestRenderViewHost*>(old_rvh); const NavigationEntry* entry = controller().pending_entry(); DCHECK(entry); @@ -98,6 +99,11 @@ void TestTabContents::CommitPendingNavigation() { static_cast<MockRenderProcessHost*>(rvh->process())->max_page_id() + 1; } rvh->SendNavigate(page_id, entry->url()); + + // Simulate the SwapOut_ACK that fires if you commit a cross-site navigation + // without making any network requests. + if (old_rvh != rvh) + old_rvh->OnSwapOutACK(); } void TestTabContents::ProceedWithCrossSiteNavigation() { diff --git a/content/common/content_client.h b/content/common/content_client.h index e985f4b..5fda6d2 100644 --- a/content/common/content_client.h +++ b/content/common/content_client.h @@ -15,6 +15,10 @@ class GURL; struct GPUInfo; struct PepperPluginInfo; +namespace IPC { +class Message; +} + namespace content { class ContentBrowserClient; @@ -49,6 +53,16 @@ class ContentClient { // Gives the embedder a chance to register its own pepper plugins. virtual void AddPepperPlugins(std::vector<PepperPluginInfo>* plugins) {} + // Returns whether the given message should be allowed to be sent from a + // swapped out renderer. + virtual bool CanSendWhileSwappedOut(const IPC::Message* msg) { return false; } + + // Returns whether the given message should be processed in the browser on + // behalf of a swapped out renderer. + virtual bool CanHandleWhileSwappedOut(const IPC::Message& msg) { + return false; + } + private: // The embedder API for participating in browser logic. ContentBrowserClient* browser_; diff --git a/content/common/swapped_out_messages.cc b/content/common/swapped_out_messages.cc new file mode 100644 index 0000000..7c7d5cd --- /dev/null +++ b/content/common/swapped_out_messages.cc @@ -0,0 +1,79 @@ +// Copyright (c) 2011 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/swapped_out_messages.h" + +#include "content/common/content_client.h" +#include "content/common/view_messages.h" + +namespace content { + +bool SwappedOutMessages::CanSendWhileSwappedOut(const IPC::Message* msg) { + // We filter out most IPC messages when swapped out. However, some are + // important (e.g., ACKs) for keeping the browser and renderer state + // consistent in case we later return to the same renderer. + switch (msg->type()) { + // Handled by RenderWidget. + case ViewHostMsg_HandleInputEvent_ACK::ID: + case ViewHostMsg_PaintAtSize_ACK::ID: + case ViewHostMsg_UpdateRect::ID: + // Handled by RenderView. + case ViewHostMsg_RenderViewGone::ID: + case ViewHostMsg_ShouldClose_ACK::ID: + case ViewHostMsg_SwapOut_ACK::ID: + case ViewHostMsg_ClosePage_ACK::ID: + return true; + default: + break; + } + + // Check with the embedder as well. + ContentClient* client = GetContentClient(); + return client->CanSendWhileSwappedOut(msg); +} + +bool SwappedOutMessages::CanHandleWhileSwappedOut( + const IPC::Message& msg) { + // Any message the renderer is allowed to send while swapped out should + // be handled by the browser. + if (CanSendWhileSwappedOut(&msg)) + return true; + + // We drop most other messages that arrive from a swapped out renderer. + // However, some are important (e.g., ACKs) for keeping the browser and + // renderer state consistent in case we later return to the renderer. + switch (msg.type()) { + // Sends an ACK. + case ViewHostMsg_ShowView::ID: + // Sends an ACK. + case ViewHostMsg_ShowWidget::ID: + // Sends an ACK. + case ViewHostMsg_ShowFullscreenWidget::ID: + // Updates browser state. + case ViewHostMsg_RenderViewReady::ID: + // Updates the previous navigation entry. + case ViewHostMsg_UpdateState::ID: + // Sends an ACK. + case ViewHostMsg_UpdateTargetURL::ID: + // We allow closing even if we are in the process of swapping out. + case ViewHostMsg_Close::ID: + // Sends an ACK. + case ViewHostMsg_RequestMove::ID: + // Suppresses dialog and sends an ACK. + case ViewHostMsg_RunJavaScriptMessage::ID: + // Suppresses dialog and sends an ACK. + case ViewHostMsg_RunBeforeUnloadConfirm::ID: + // Sends an ACK. + case ViewHostMsg_AccessibilityNotifications::ID: + return true; + default: + break; + } + + // Check with the embedder as well. + ContentClient* client = GetContentClient(); + return client->CanHandleWhileSwappedOut(msg); +} + +} // namespace content diff --git a/content/common/swapped_out_messages.h b/content/common/swapped_out_messages.h new file mode 100644 index 0000000..559d6b1 --- /dev/null +++ b/content/common/swapped_out_messages.h @@ -0,0 +1,23 @@ +// Copyright (c) 2011 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_SWAPPED_OUT_MESSAGES_H_ +#define CONTENT_COMMON_SWAPPED_OUT_MESSAGES_H_ +#pragma once + +#include "ipc/ipc_message.h" + +namespace content { + +// Functions for filtering IPC messages sent from and received from a swapped +// out renderer. +class SwappedOutMessages { + public: + static bool CanSendWhileSwappedOut(const IPC::Message* msg); + static bool CanHandleWhileSwappedOut(const IPC::Message& msg); +}; + +} // namespace content + +#endif // CONTENT_COMMON_SWAPPED_OUT_MESSAGES_H_ diff --git a/content/common/view_messages.h b/content/common/view_messages.h index 9ed984c..16e07a2 100644 --- a/content/common/view_messages.h +++ b/content/common/view_messages.h @@ -631,37 +631,26 @@ IPC_STRUCT_BEGIN(ViewHostMsg_UpdateRect_Params) IPC_STRUCT_MEMBER(int, flags) IPC_STRUCT_END() -IPC_STRUCT_BEGIN(ViewMsg_ClosePage_Params) +IPC_STRUCT_BEGIN(ViewMsg_SwapOut_Params) // The identifier of the RenderProcessHost for the currently closing view. // // These first two parameters are technically redundant since they are // needed only when processing the ACK message, and the processor // theoretically knows both the process and route ID. However, this is // difficult to figure out with our current implementation, so this - // information is duplicate here. + // information is duplicated here. IPC_STRUCT_MEMBER(int, closing_process_id) // The route identifier for the currently closing RenderView. IPC_STRUCT_MEMBER(int, closing_route_id) - // True when this close is for the first (closing) tab of a cross-site - // transition where we switch processes. False indicates the close is for the - // entire tab. - // - // When true, the new_* variables below must be filled in. Otherwise they must - // both be -1. - IPC_STRUCT_MEMBER(bool, for_cross_site_transition) - // The identifier of the RenderProcessHost for the new view attempting to - // replace the closing one above. This must be valid when - // for_cross_site_transition is set, and must be -1 otherwise. + // replace the closing one above. IPC_STRUCT_MEMBER(int, new_render_process_host_id) // The identifier of the *request* the new view made that is causing the // cross-site transition. This is *not* a route_id, but the request that we - // will resume once the ACK from the closing view has been received. This - // must be valid when for_cross_site_transition is set, and must be -1 - // otherwise. + // will resume once the ACK from the closing view has been received. IPC_STRUCT_MEMBER(int, new_request_id) IPC_STRUCT_END() @@ -796,6 +785,10 @@ IPC_MESSAGE_ROUTED0(ViewMsg_WasHidden) IPC_MESSAGE_ROUTED1(ViewMsg_WasRestored, bool /* needs_repainting */) +// Sent to inform the view that it was swapped out. This allows the process to +// exit if no other views are using it. +IPC_MESSAGE_ROUTED0(ViewMsg_WasSwappedOut) + // Sent to render the view into the supplied transport DIB, resize // the web widget to match the |page_size|, scale it by the // appropriate scale to make it fit the |desired_size|, and return @@ -1062,13 +1055,19 @@ IPC_MESSAGE_ROUTED0(ViewMsg_CantFocus) // via ViewHostMsg_ShouldClose. IPC_MESSAGE_ROUTED0(ViewMsg_ShouldClose) -// Instructs the renderer to close the current page, including running the -// onunload event handler. See the struct in render_messages.h for more. +// Instructs the renderer to swap out for a cross-site transition, including +// running the unload event handler. See the struct above for more details. // -// Expects a ClosePage_ACK message when finished, where the parameters are +// Expects a SwapOut_ACK message when finished, where the parameters are // echoed back. -IPC_MESSAGE_ROUTED1(ViewMsg_ClosePage, - ViewMsg_ClosePage_Params) +IPC_MESSAGE_ROUTED1(ViewMsg_SwapOut, + ViewMsg_SwapOut_Params) + +// Instructs the renderer to close the current page, including running the +// onunload event handler. +// +// Expects a ClosePage_ACK message when finished. +IPC_MESSAGE_ROUTED0(ViewMsg_ClosePage) // Notifies the renderer about ui theme changes IPC_MESSAGE_ROUTED0(ViewMsg_ThemeChanged) @@ -1304,10 +1303,14 @@ IPC_MESSAGE_ROUTED5(ViewHostMsg_Find_Reply, IPC_MESSAGE_ROUTED1(ViewHostMsg_ShouldClose_ACK, bool /* proceed */) +// Indicates that the current renderer has swapped out, after a SwapOut +// message. The parameters are just echoed from the SwapOut request. +IPC_MESSAGE_ROUTED1(ViewHostMsg_SwapOut_ACK, + ViewMsg_SwapOut_Params) + // Indicates that the current page has been closed, after a ClosePage -// message. The parameters are just echoed from the ClosePage request. -IPC_MESSAGE_ROUTED1(ViewHostMsg_ClosePage_ACK, - ViewMsg_ClosePage_Params) +// message. +IPC_MESSAGE_ROUTED0(ViewHostMsg_ClosePage_ACK) // Notifies the browser that we have session history information. // page_id: unique ID that allows us to distinguish between history entries. diff --git a/content/content_common.gypi b/content/content_common.gypi index 667c997..ad21ae5 100644 --- a/content/content_common.gypi +++ b/content/content_common.gypi @@ -210,6 +210,8 @@ 'common/socket_stream_messages.h', 'common/speech_input_messages.h', 'common/speech_input_result.h', + 'common/swapped_out_messages.cc', + 'common/swapped_out_messages.h', 'common/unix_domain_socket_posix.cc', 'common/unix_domain_socket_posix.h', 'common/url_constants.cc', diff --git a/content/renderer/render_view.cc b/content/renderer/render_view.cc index 63c36b4..62b1e99 100644 --- a/content/renderer/render_view.cc +++ b/content/renderer/render_view.cc @@ -632,6 +632,7 @@ bool RenderView::OnMessageReceived(const IPC::Message& message) { OnEnumerateDirectoryResponse) IPC_MESSAGE_HANDLER(ViewMsg_RunFileChooserResponse, OnFileChooserResponse) IPC_MESSAGE_HANDLER(ViewMsg_ShouldClose, OnShouldClose) + IPC_MESSAGE_HANDLER(ViewMsg_SwapOut, OnSwapOut) IPC_MESSAGE_HANDLER(ViewMsg_ClosePage, OnClosePage) IPC_MESSAGE_HANDLER(ViewMsg_ThemeChanged, OnThemeChanged) IPC_MESSAGE_HANDLER(ViewMsg_DisassociateFromPopupCount, @@ -685,6 +686,10 @@ void RenderView::OnNavigate(const ViewMsg_Navigate_Params& params) { if (!webview()) return; + // Swap this renderer back in if necessary. + if (is_swapped_out_) + SetSwappedOut(false); + history_list_offset_ = params.current_history_list_offset; history_list_length_ = params.current_history_list_length; @@ -1564,6 +1569,12 @@ bool RenderView::runModalPromptDialog( bool RenderView::runModalBeforeUnloadDialog( WebFrame* frame, const WebString& message) { + // If we are swapping out, we have already run the beforeunload handler. + // TODO(creis): Fix OnSwapOut to clear the frame without running beforeunload + // at all, to avoid running it twice. + if (is_swapped_out_) + return true; + bool success = false; // This is an ignored return value, but is included so we can accept the same // response as RunJavaScriptMessage. @@ -1926,6 +1937,12 @@ void RenderView::loadURLExternally( WebNavigationPolicy RenderView::decidePolicyForNavigation( WebFrame* frame, const WebURLRequest& request, WebNavigationType type, const WebNode&, WebNavigationPolicy default_policy, bool is_redirect) { + // TODO(creis): Remove this when we fix OnSwapOut to not need a navigation. + if (is_swapped_out_) { + DCHECK(request.url() == GURL("about:swappedout")); + return default_policy; + } + // Webkit is asking whether to navigate to a new URL. // This is fine normally, except if we're showing UI from one security // context and they're trying to navigate to a different context. @@ -3510,7 +3527,38 @@ void RenderView::OnShouldClose() { Send(new ViewHostMsg_ShouldClose_ACK(routing_id_, should_close)); } -void RenderView::OnClosePage(const ViewMsg_ClosePage_Params& params) { +void RenderView::OnSwapOut(const ViewMsg_SwapOut_Params& params) { + if (is_swapped_out_) + return; + + // Swap this RenderView out so the tab can navigate to a page rendered by a + // different process. This involves running the unload handler and clearing + // the page. Once WasSwappedOut is called, we also allow this process to exit + // if there are no other active RenderViews in it. + + // Send an UpdateState message before we get swapped out. + SyncNavigationState(); + + // Synchronously run the unload handler before sending the ACK. + webview()->dispatchUnloadEvent(); + + // Swap out and stop sending any IPC messages that are not ACKs. + SetSwappedOut(true); + + // Replace the page with a blank dummy URL. The unload handler will not be + // 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. + webview()->mainFrame()->loadHTMLString(std::string(), + GURL("about:swappedout"), + GURL("about:swappedout"), + false); + + // Just echo back the params in the ACK. + Send(new ViewHostMsg_SwapOut_ACK(routing_id_, params)); +} + +void RenderView::OnClosePage() { // TODO(creis): We'd rather use webview()->Close() here, but that currently // sets the WebView's delegate_ to NULL, preventing any JavaScript dialogs // in the onunload handler from appearing. For now, we're bypassing that and @@ -3520,8 +3568,7 @@ void RenderView::OnClosePage(const ViewMsg_ClosePage_Params& params) { // http://b/issue?id=753080. webview()->dispatchUnloadEvent(); - // Just echo back the params in the ACK. - Send(new ViewHostMsg_ClosePage_ACK(routing_id_, params)); + Send(new ViewHostMsg_ClosePage_ACK(routing_id_)); } void RenderView::OnThemeChanged() { diff --git a/content/renderer/render_view.h b/content/renderer/render_view.h index 4cdf0e2..a5c7f28 100644 --- a/content/renderer/render_view.h +++ b/content/renderer/render_view.h @@ -71,7 +71,7 @@ class WebUIBindings; struct ContextMenuMediaParams; struct PP_Flash_NetAddress; struct ViewHostMsg_RunFileChooser_Params; -struct ViewMsg_ClosePage_Params; +struct ViewMsg_SwapOut_Params; struct ViewMsg_Navigate_Params; struct ViewMsg_StopFinding_Params; struct WebDropData; @@ -710,7 +710,7 @@ class RenderView : public RenderWidget, IPC::ChannelHandle handle); void OnCancelDownload(int32 download_id); void OnClearFocusedNode(); - void OnClosePage(const ViewMsg_ClosePage_Params& params); + void OnClosePage(); #if defined(ENABLE_FLAPPER_HACKS) void OnConnectTcpACK(int request_id, IPC::PlatformFileForTransit socket_for_transit, @@ -796,6 +796,7 @@ class RenderView : public RenderWidget, void OnShouldClose(); void OnStop(); void OnStopFinding(const ViewMsg_StopFinding_Params& params); + void OnSwapOut(const ViewMsg_SwapOut_Params& params); void OnThemeChanged(); void OnUndo(); void OnUpdateTargetURLAck(); diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc index 2597ad6..7572b7e 100644 --- a/content/renderer/render_widget.cc +++ b/content/renderer/render_widget.cc @@ -12,6 +12,7 @@ #include "base/metrics/histogram.h" #include "build/build_config.h" #include "content/common/content_switches.h" +#include "content/common/swapped_out_messages.h" #include "content/common/view_messages.h" #include "content/renderer/render_process.h" #include "content/renderer/render_thread.h" @@ -74,6 +75,7 @@ RenderWidget::RenderWidget(RenderThreadBase* render_thread, has_focus_(false), handling_input_event_(false), closing_(false), + is_swapped_out_(false), input_method_is_active_(false), text_input_type_(WebKit::WebTextInputTypeNone), popup_type_(popup_type), @@ -92,7 +94,9 @@ RenderWidget::~RenderWidget() { RenderProcess::current()->ReleaseTransportDIB(current_paint_buf_); current_paint_buf_ = NULL; } - RenderProcess::current()->ReleaseProcess(); + // If we are swapped out, we have released already. + if (!is_swapped_out_) + RenderProcess::current()->ReleaseProcess(); } // static @@ -160,6 +164,20 @@ void RenderWidget::CompleteInit(gfx::NativeViewId parent_hwnd, Send(new ViewHostMsg_RenderViewReady(routing_id_)); } +void RenderWidget::SetSwappedOut(bool is_swapped_out) { + // We should only toggle between states. + DCHECK(is_swapped_out_ != is_swapped_out); + is_swapped_out_ = is_swapped_out; + + // If we are swapping out, we will call ReleaseProcess, allowing the process + // to exit if all of its RenderViews are swapped out. We wait until the + // WasSwappedOut call to do this, to avoid showing the sad tab. + // If we are swapping in, we call AddRefProcess to prevent the process from + // exiting. + if (!is_swapped_out) + RenderProcess::current()->AddRefProcess(); +} + bool RenderWidget::OnMessageReceived(const IPC::Message& message) { bool handled = true; IPC_BEGIN_MESSAGE_MAP(RenderWidget, message) @@ -168,6 +186,7 @@ bool RenderWidget::OnMessageReceived(const IPC::Message& message) { IPC_MESSAGE_HANDLER(ViewMsg_Resize, OnResize) IPC_MESSAGE_HANDLER(ViewMsg_WasHidden, OnWasHidden) IPC_MESSAGE_HANDLER(ViewMsg_WasRestored, OnWasRestored) + IPC_MESSAGE_HANDLER(ViewMsg_WasSwappedOut, OnWasSwappedOut) IPC_MESSAGE_HANDLER(ViewMsg_UpdateRect_ACK, OnUpdateRectAck) IPC_MESSAGE_HANDLER(ViewMsg_HandleInputEvent, OnHandleInputEvent) IPC_MESSAGE_HANDLER(ViewMsg_MouseCaptureLost, OnMouseCaptureLost) @@ -185,8 +204,11 @@ bool RenderWidget::OnMessageReceived(const IPC::Message& message) { } bool RenderWidget::Send(IPC::Message* message) { - // Don't send any messages after the browser has told us to close. - if (closing_) { + // Don't send any messages after the browser has told us to close, and filter + // most outgoing messages while swapped out. + if ((is_swapped_out_ && + !content::SwappedOutMessages::CanSendWhileSwappedOut(message)) || + closing_) { delete message; return false; } @@ -299,6 +321,14 @@ void RenderWidget::OnWasRestored(bool needs_repainting) { } } +void RenderWidget::OnWasSwappedOut() { + // If we have been swapped out and no one else is using this process, + // it's safe to exit now. If we get swapped back in, we will call + // AddRefProcess in SetSwappedOut. + if (is_swapped_out_) + RenderProcess::current()->ReleaseProcess(); +} + void RenderWidget::OnRequestMoveAck() { DCHECK(pending_window_rect_count_); pending_window_rect_count_--; diff --git a/content/renderer/render_widget.h b/content/renderer/render_widget.h index 63580c7..74944f6 100644 --- a/content/renderer/render_widget.h +++ b/content/renderer/render_widget.h @@ -154,6 +154,12 @@ class RenderWidget : public IPC::Channel::Listener, void CompleteInit(gfx::NativeViewId parent, gfx::PluginWindowHandle compositing_surface); + // Sets whether this RenderWidget has been swapped out to be displayed by + // a RenderWidget in a different process. If so, no new IPC messages will be + // sent (only ACKs) and the process is free to exit when there are no other + // active RenderWidgets. + void SetSwappedOut(bool is_swapped_out); + // Paints the given rectangular region of the WebWidget into canvas (a // shared memory segment returned by AllocPaintBuf on Windows). The caller // must ensure that the given rect fits within the bounds of the WebWidget. @@ -183,6 +189,7 @@ class RenderWidget : public IPC::Channel::Listener, const gfx::Rect& resizer_rect); virtual void OnWasHidden(); virtual void OnWasRestored(bool needs_repainting); + virtual void OnWasSwappedOut(); void OnUpdateRectAck(); void OnCreateVideoAck(int32 video_id); void OnUpdateVideoAck(int32 video_id); @@ -343,6 +350,11 @@ class RenderWidget : public IPC::Channel::Listener, // be sent, except for a Close. bool closing_; + // Whether this RenderWidget is currently swapped out, such that the view is + // being rendered by another process. If all RenderWidgets in a process are + // swapped out, the process can exit. + bool is_swapped_out_; + // Indicates if an input method is active in the browser process. bool input_method_is_active_; |