summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorcreis@chromium.org <creis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-02-24 22:53:26 +0000
committercreis@chromium.org <creis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-02-24 22:53:26 +0000
commit7e8ac7b1c05ce26761b13577290bae8b7ad2ea87 (patch)
tree540fd168b0f6ae8866969d13faf2ffee5ddaffa1
parent54199868a48747959568c1ee4116eff33860b79e (diff)
downloadchromium_src-7e8ac7b1c05ce26761b13577290bae8b7ad2ea87.zip
chromium_src-7e8ac7b1c05ce26761b13577290bae8b7ad2ea87.tar.gz
chromium_src-7e8ac7b1c05ce26761b13577290bae8b7ad2ea87.tar.bz2
Revert 249676 "Have the unload event execute in background on cr..."
Speculative revert to see if it resolves bug 345757. Suspected because UnregisterRenderFrameHost could lead to RenderViewHost outliving WebContents, causing at least some of the ObserverListBase crashes we're seeing. > Have the unload event execute in background on cross-site navigations > > Cross-site navigations trigger renderer swap. This CL makes it so that the swap > does not need for the old renderer unload event to be fired. Instead, it will > be executed in the background, and the new renderer will be swapped in > immediately. > BUG=323528 > > Review URL: https://codereview.chromium.org/88503002 TBR=clamy@chromium.org Review URL: https://codereview.chromium.org/177093009 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@253018 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/renderer_host/web_cache_manager_browsertest.cc11
-rw-r--r--content/browser/frame_host/frame_tree.cc69
-rw-r--r--content/browser/frame_host/frame_tree.h20
-rw-r--r--content/browser/frame_host/navigation_controller_impl_unittest.cc1
-rw-r--r--content/browser/frame_host/render_frame_host_impl.cc6
-rw-r--r--content/browser/frame_host/render_frame_host_impl.h5
-rw-r--r--content/browser/frame_host/render_frame_host_manager.cc78
-rw-r--r--content/browser/frame_host/render_frame_host_manager.h12
-rw-r--r--content/browser/frame_host/render_frame_host_manager_unittest.cc264
-rw-r--r--content/browser/renderer_host/render_process_host_impl.h7
-rw-r--r--content/browser/renderer_host/render_view_host_impl.cc189
-rw-r--r--content/browser/renderer_host/render_view_host_impl.h109
-rw-r--r--content/browser/renderer_host/render_view_host_unittest.cc16
-rw-r--r--content/browser/renderer_host/render_widget_host_impl.cc3
-rw-r--r--content/browser/web_contents/web_contents_impl.cc8
-rw-r--r--content/browser/web_contents/web_contents_impl_unittest.cc6
-rw-r--r--content/public/browser/render_process_host.h5
-rw-r--r--content/public/test/mock_render_process_host.cc4
-rw-r--r--content/public/test/mock_render_process_host.h2
-rw-r--r--content/public/test/test_renderer_host.cc3
-rw-r--r--content/test/test_render_view_host.h10
-rw-r--r--content/test/test_web_contents.cc7
22 files changed, 201 insertions, 634 deletions
diff --git a/chrome/browser/renderer_host/web_cache_manager_browsertest.cc b/chrome/browser/renderer_host/web_cache_manager_browsertest.cc
index 7caee2f..a917ed5 100644
--- a/chrome/browser/renderer_host/web_cache_manager_browsertest.cc
+++ b/chrome/browser/renderer_host/web_cache_manager_browsertest.cc
@@ -57,13 +57,10 @@ IN_PROC_BROWSER_TEST_F(WebCacheManagerBrowserTest, CrashOnceOnly) {
ui_test_utils::NavigateToURL(browser(), url);
- // Depending on the speed of execution of the unload event, we may have one or
- // two active renderers at that point (one executing the unload event in
- // background).
- EXPECT_GE(WebCacheManager::GetInstance()->active_renderers_.size(), 1U);
- EXPECT_LE(WebCacheManager::GetInstance()->active_renderers_.size(), 2U);
+ EXPECT_EQ(
+ WebCacheManager::GetInstance()->active_renderers_.size(), 1U);
EXPECT_EQ(
WebCacheManager::GetInstance()->inactive_renderers_.size(), 0U);
- EXPECT_GE(WebCacheManager::GetInstance()->stats_.size(), 1U);
- EXPECT_LE(WebCacheManager::GetInstance()->stats_.size(), 2U);
+ EXPECT_EQ(
+ WebCacheManager::GetInstance()->stats_.size(), 1U);
}
diff --git a/content/browser/frame_host/frame_tree.cc b/content/browser/frame_host/frame_tree.cc
index 23fc768..bd52e2b 100644
--- a/content/browser/frame_host/frame_tree.cc
+++ b/content/browser/frame_host/frame_tree.cc
@@ -168,17 +168,7 @@ RenderViewHostImpl* FrameTree::CreateRenderViewHostForMainFrame(
DCHECK(main_frame_routing_id != MSG_ROUTING_NONE);
RenderViewHostMap::iterator iter =
render_view_host_map_.find(site_instance->GetId());
- if (iter != render_view_host_map_.end()) {
- // If a RenderViewHost is pending shutdown for this |site_instance|, put it
- // in the map of RenderViewHosts pending shutdown. Otherwise there should
- // not be a RenderViewHost for the SiteInstance.
- CHECK_EQ(RenderViewHostImpl::STATE_PENDING_SHUTDOWN,
- iter->second->rvh_state());
- render_view_host_pending_shutdown_map_.insert(
- std::pair<int, RenderViewHostImpl*>(site_instance->GetId(),
- iter->second));
- render_view_host_map_.erase(iter);
- }
+ CHECK(iter == render_view_host_map_.end());
RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>(
RenderViewHostFactory::Create(site_instance,
render_view_delegate_,
@@ -188,7 +178,8 @@ RenderViewHostImpl* FrameTree::CreateRenderViewHostForMainFrame(
swapped_out,
hidden));
- render_view_host_map_[site_instance->GetId()] = rvh;
+ render_view_host_map_[site_instance->GetId()] =
+ RenderViewHostRefCount(rvh, 0);
return rvh;
}
@@ -199,7 +190,8 @@ RenderViewHostImpl* FrameTree::GetRenderViewHostForSubFrame(
// TODO(creis): Mirror the frame tree so this check can't fail.
if (iter == render_view_host_map_.end())
return NULL;
- return iter->second;
+ RenderViewHostRefCount rvh_refcount = iter->second;
+ return rvh_refcount.first;
}
void FrameTree::RegisterRenderFrameHost(
@@ -210,51 +202,26 @@ void FrameTree::RegisterRenderFrameHost(
render_view_host_map_.find(site_instance->GetId());
CHECK(iter != render_view_host_map_.end());
- iter->second->increment_ref_count();
+ // Increment the refcount.
+ CHECK_GE(iter->second.second, 0);
+ iter->second.second++;
}
void FrameTree::UnregisterRenderFrameHost(
RenderFrameHostImpl* render_frame_host) {
SiteInstance* site_instance =
render_frame_host->render_view_host()->GetSiteInstance();
- int32 site_instance_id = site_instance->GetId();
RenderViewHostMap::iterator iter =
- render_view_host_map_.find(site_instance_id);
- if (iter != render_view_host_map_.end() &&
- iter->second == render_frame_host->render_view_host()) {
- // Decrement the refcount and shutdown the RenderViewHost if no one else is
- // using it.
- CHECK_GT(iter->second->ref_count(), 0);
- iter->second->decrement_ref_count();
- if (iter->second->ref_count() == 0) {
- iter->second->Shutdown();
- render_view_host_map_.erase(iter);
- }
- } else {
- // The RenderViewHost should be in the list of RenderViewHosts pending
- // shutdown.
- bool render_view_host_found = false;
- std::pair<RenderViewHostMultiMap::iterator,
- RenderViewHostMultiMap::iterator> result =
- render_view_host_pending_shutdown_map_.equal_range(site_instance_id);
- for (RenderViewHostMultiMap::iterator multi_iter = result.first;
- multi_iter != result.second;
- ++multi_iter) {
- if (multi_iter->second != render_frame_host->render_view_host())
- continue;
- render_view_host_found = true;
- RenderViewHostImpl* rvh = multi_iter->second;
- // Decrement the refcount and shutdown the RenderViewHost if no one else
- // is using it.
- CHECK_GT(rvh->ref_count(), 0);
- rvh->decrement_ref_count();
- if (rvh->ref_count() == 0) {
- rvh->Shutdown();
- render_view_host_pending_shutdown_map_.erase(multi_iter);
- }
- break;
- }
- CHECK(render_view_host_found);
+ render_view_host_map_.find(site_instance->GetId());
+ CHECK(iter != render_view_host_map_.end());
+
+ // Decrement the refcount and shutdown the RenderViewHost if no one else is
+ // using it.
+ CHECK_GT(iter->second.second, 0);
+ iter->second.second--;
+ if (iter->second.second == 0) {
+ iter->second.first->Shutdown();
+ render_view_host_map_.erase(iter);
}
}
diff --git a/content/browser/frame_host/frame_tree.h b/content/browser/frame_host/frame_tree.h
index 1a43618..8e18956 100644
--- a/content/browser/frame_host/frame_tree.h
+++ b/content/browser/frame_host/frame_tree.h
@@ -116,8 +116,8 @@ class CONTENT_EXPORT FrameTree {
void UnregisterRenderFrameHost(RenderFrameHostImpl* render_frame_host);
private:
- typedef base::hash_map<int, RenderViewHostImpl*> RenderViewHostMap;
- typedef std::multimap<int, RenderViewHostImpl*> RenderViewHostMultiMap;
+ typedef std::pair<RenderViewHostImpl*, int> RenderViewHostRefCount;
+ typedef base::hash_map<int, RenderViewHostRefCount> RenderViewHostMap;
// These delegates are installed into all the RenderViewHosts and
// RenderFrameHosts that we create.
@@ -126,23 +126,15 @@ class CONTENT_EXPORT FrameTree {
RenderWidgetHostDelegate* render_widget_delegate_;
RenderFrameHostManager::Delegate* manager_delegate_;
- // Map of SiteInstance ID to a RenderViewHost. This allows us to look up the
- // RenderViewHost for a given SiteInstance when creating RenderFrameHosts.
- // Combined with the refcount on RenderViewHost, this allows us to call
- // Shutdown on the RenderViewHost and remove it from the map when no more
- // RenderFrameHosts are using it.
+ // Map of SiteInstance ID to a (RenderViewHost, refcount) pair. This allows
+ // us to look up the RenderViewHost for a given SiteInstance when creating
+ // RenderFrameHosts, and it allows us to call Shutdown on the RenderViewHost
+ // and remove it from the map when no more RenderFrameHosts are using it.
//
// Must be declared before |root_| so that it is deleted afterward. Otherwise
// the map will be cleared before we delete the RenderFrameHosts in the tree.
RenderViewHostMap render_view_host_map_;
- // Map of SiteInstance ID to RenderViewHosts that are pending shutdown. The
- // renderers of these RVH are currently executing the unload event in
- // background. When the SwapOutACK is received, they will be deleted. In the
- // meantime, they are kept in this map, as they should not be reused (part of
- // their state is already gone away).
- RenderViewHostMultiMap render_view_host_pending_shutdown_map_;
-
scoped_ptr<FrameTreeNode> root_;
base::Callback<void(RenderViewHostImpl*, int)> on_frame_removed_;
diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc
index 6a2a9a2..f41b5e8 100644
--- a/content/browser/frame_host/navigation_controller_impl_unittest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc
@@ -1095,7 +1095,6 @@ TEST_F(NavigationControllerTest, LoadURL_WithBindings) {
contents()->GetPendingRenderViewHost()->AllowBindings(1);
static_cast<TestRenderViewHost*>(
contents()->GetPendingRenderViewHost())->SendNavigate(1, url2);
- orig_rvh->OnSwappedOut(false);
// The second load should be committed, and bindings should be remembered.
EXPECT_EQ(controller.GetEntryCount(), 2);
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 97305f3..4ff41aa 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -243,7 +243,7 @@ void RenderFrameHostImpl::OnNavigate(const IPC::Message& msg) {
// unload request. It will either respond to the unload request soon or our
// timer will expire. Either way, we should ignore this message, because we
// have already committed to closing this renderer.
- if (render_view_host_->IsWaitingForUnloadACK())
+ if (render_view_host_->is_waiting_for_unload_ack_)
return;
RenderProcessHost* process = GetProcess();
@@ -331,10 +331,6 @@ void RenderFrameHostImpl::OnContextMenu(const ContextMenuParams& params) {
delegate_->ShowContextMenu(this, validated_params);
}
-void RenderFrameHostImpl::SetPendingShutdown(const base::Closure& on_swap_out) {
- render_view_host_->SetPendingShutdown(on_swap_out);
-}
-
bool RenderFrameHostImpl::CanCommitURL(const GURL& url) {
// TODO(creis): We should also check for WebUI pages here. Also, when the
// out-of-process iframes implementation is ready, we should check for
diff --git a/content/browser/frame_host/render_frame_host_impl.h b/content/browser/frame_host/render_frame_host_impl.h
index 12a0059..cf58b87 100644
--- a/content/browser/frame_host/render_frame_host_impl.h
+++ b/content/browser/frame_host/render_frame_host_impl.h
@@ -7,7 +7,6 @@
#include <string>
-#include "base/callback.h"
#include "base/compiler_specific.h"
#include "base/strings/string16.h"
#include "content/common/content_export.h"
@@ -83,10 +82,6 @@ class CONTENT_EXPORT RenderFrameHostImpl : public RenderFrameHost {
is_swapped_out_ = is_swapped_out;
}
- // Sets the RVH for |this| as pending shutdown. |on_swap_out| will be called
- // when the SwapOutACK is received.
- void SetPendingShutdown(const base::Closure& on_swap_out);
-
// TODO(nasko): This method is public so RenderViewHostImpl::Navigate can
// call it directly. It should be made private once Navigate moves here.
void OnDidStartLoading();
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 3a36904..e69abd9 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -72,8 +72,7 @@ RenderFrameHostManager::RenderFrameHostManager(
render_frame_host_(NULL),
pending_render_frame_host_(NULL),
interstitial_page_(NULL),
- cross_process_frame_connector_(NULL),
- weak_factory_(this) {}
+ cross_process_frame_connector_(NULL) {}
RenderFrameHostManager::~RenderFrameHostManager() {
if (pending_render_frame_host_)
@@ -244,7 +243,7 @@ bool RenderFrameHostManager::ShouldCloseTabOnUnresponsiveRenderer() {
// If the tab becomes unresponsive during {before}unload while doing a
// cross-site navigation, proceed with the navigation. (This assumes that
// the pending RenderFrameHost is still responsive.)
- if (render_frame_host_->render_view_host()->IsWaitingForUnloadACK()) {
+ if (render_frame_host_->render_view_host()->is_waiting_for_unload_ack()) {
// The request has been started and paused while we're waiting for the
// unload handler to finish. We'll pretend that it did. The pending
// renderer will then be swapped in as part of the usual DidNavigate logic.
@@ -565,15 +564,6 @@ void RenderFrameHostManager::SwapOutOldPage() {
}
}
-void RenderFrameHostManager::ClearPendingShutdownRFHForSiteInstance(
- int32 site_instance_id,
- RenderFrameHostImpl* rfh) {
- RFHPendingDeleteMap::iterator iter =
- pending_delete_hosts_.find(site_instance_id);
- if (iter != pending_delete_hosts_.end() && iter->second.get() == rfh)
- pending_delete_hosts_.erase(site_instance_id);
-}
-
void RenderFrameHostManager::Observe(
int type,
const NotificationSource& source,
@@ -595,30 +585,8 @@ bool RenderFrameHostManager::ClearSwappedOutRFHsInSiteInstance(
FrameTreeNode* node) {
RenderFrameHostMap::iterator iter =
node->render_manager()->swapped_out_hosts_.find(site_instance_id);
- if (iter != node->render_manager()->swapped_out_hosts_.end()) {
- RenderFrameHostImpl* swapped_out_rfh = iter->second;
- // If the RVH is pending swap out, it needs to switch state to
- // pending shutdown. Otherwise it is deleted.
- if (swapped_out_rfh->render_view_host()->rvh_state() ==
- RenderViewHostImpl::STATE_PENDING_SWAP_OUT) {
- swapped_out_rfh->SetPendingShutdown(base::Bind(
- &RenderFrameHostManager::ClearPendingShutdownRFHForSiteInstance,
- node->render_manager()->weak_factory_.GetWeakPtr(),
- site_instance_id,
- swapped_out_rfh));
- RFHPendingDeleteMap::iterator pending_delete_iter =
- node->render_manager()->pending_delete_hosts_.find(site_instance_id);
- if (pending_delete_iter ==
- node->render_manager()->pending_delete_hosts_.end() ||
- pending_delete_iter->second.get() != iter->second) {
- node->render_manager()->pending_delete_hosts_[site_instance_id] =
- linked_ptr<RenderFrameHostImpl>(swapped_out_rfh);
- }
- } else {
- delete swapped_out_rfh;
- }
- node->render_manager()->swapped_out_hosts_.erase(site_instance_id);
- }
+ if (iter != node->render_manager()->swapped_out_hosts_.end())
+ delete iter->second;
return true;
}
@@ -1001,7 +969,7 @@ bool RenderFrameHostManager::InitRenderView(RenderViewHost* render_view_host,
// process unless it's swapped out.
RenderViewHostImpl* rvh_impl =
static_cast<RenderViewHostImpl*>(render_view_host);
- if (!rvh_impl->IsSwappedOut()) {
+ if (!rvh_impl->is_swapped_out()) {
CHECK(!ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
render_view_host->GetProcess()->GetID()));
}
@@ -1080,16 +1048,10 @@ void RenderFrameHostManager::CommitPending() {
// If the old view is live and top-level, hide it now that the new one is
// visible.
- int32 old_site_instance_id =
- old_render_frame_host->render_view_host()->GetSiteInstance()->GetId();
if (old_render_frame_host->render_view_host()->GetView()) {
if (is_main_frame) {
old_render_frame_host->render_view_host()->GetView()->Hide();
- old_render_frame_host->render_view_host()->WasSwappedOut(base::Bind(
- &RenderFrameHostManager::ClearPendingShutdownRFHForSiteInstance,
- weak_factory_.GetWeakPtr(),
- old_site_instance_id,
- old_render_frame_host));
+ old_render_frame_host->render_view_host()->WasSwappedOut();
} else {
// TODO(creis): We'll need to set this back to false if we navigate back.
old_render_frame_host->set_swapped_out(true);
@@ -1123,17 +1085,18 @@ void RenderFrameHostManager::CommitPending() {
if (old_render_frame_host->render_view_host()->IsRenderViewLive()) {
// If the old RFH is live, we are swapping it out and should keep track of
- // it in case we navigate back to it, or it is waiting for the unload event
- // to execute in the background.
+ // it in case we navigate back to it.
// TODO(creis): Swap out the subframe in --site-per-process.
if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess))
DCHECK(old_render_frame_host->is_swapped_out() ||
- !RenderViewHostImpl::IsRVHStateActive(
- old_render_frame_host->render_view_host()->rvh_state()));
+ old_render_frame_host->render_view_host()->is_swapped_out());
+
// Temp fix for http://crbug.com/90867 until we do a better cleanup to make
// sure we don't get different rvh instances for the same site instance
// in the same rvhmgr.
// TODO(creis): Clean this up.
+ int32 old_site_instance_id =
+ old_render_frame_host->render_view_host()->GetSiteInstance()->GetId();
RenderFrameHostMap::iterator iter =
swapped_out_hosts_.find(old_site_instance_id);
if (iter != swapped_out_hosts_.end() &&
@@ -1141,23 +1104,7 @@ void RenderFrameHostManager::CommitPending() {
// Delete the RFH that will be replaced in the map to avoid a leak.
delete iter->second;
}
- // If the RenderViewHost backing the RenderFrameHost is pending shutdown,
- // the RenderFrameHost should be put in the map of RenderFrameHosts pending
- // shutdown. Otherwise, it is stored in the map of swapped out
- // RenderFrameHosts.
- if (old_render_frame_host->render_view_host()->rvh_state() ==
- RenderViewHostImpl::STATE_PENDING_SHUTDOWN) {
- swapped_out_hosts_.erase(old_site_instance_id);
- RFHPendingDeleteMap::iterator pending_delete_iter =
- pending_delete_hosts_.find(old_site_instance_id);
- if (pending_delete_iter == pending_delete_hosts_.end() ||
- pending_delete_iter->second.get() != old_render_frame_host) {
- pending_delete_hosts_[old_site_instance_id] =
- linked_ptr<RenderFrameHostImpl>(old_render_frame_host);
- }
- } else {
- swapped_out_hosts_[old_site_instance_id] = old_render_frame_host;
- }
+ swapped_out_hosts_[old_site_instance_id] = old_render_frame_host;
// If there are no active views in this SiteInstance, it means that
// this RFH was the last active one in the SiteInstance. Now that we
@@ -1200,6 +1147,7 @@ void RenderFrameHostManager::ShutdownRenderFrameHostsInSiteInstance(
tree->ForEach(base::Bind(
&RenderFrameHostManager::ClearSwappedOutRFHsInSiteInstance,
site_instance_id));
+ // rvh is now deleted.
}
}
}
diff --git a/content/browser/frame_host/render_frame_host_manager.h b/content/browser/frame_host/render_frame_host_manager.h
index 768532a8..39b6b9f 100644
--- a/content/browser/frame_host/render_frame_host_manager.h
+++ b/content/browser/frame_host/render_frame_host_manager.h
@@ -12,7 +12,6 @@
#include "content/browser/renderer_host/render_view_host_delegate.h"
#include "content/browser/site_instance_impl.h"
#include "content/common/content_export.h"
-#include "content/public/browser/global_request_id.h"
#include "content/public/browser/notification_observer.h"
#include "content/public/browser/notification_registrar.h"
#include "content/public/common/referrer.h"
@@ -271,10 +270,6 @@ class CONTENT_EXPORT RenderFrameHostManager
// to a new process after this completes or times out.
void SwapOutOldPage();
- // Deletes a RenderFrameHost that was pending shutdown.
- void ClearPendingShutdownRFHForSiteInstance(int32 site_instance_id,
- RenderFrameHostImpl* rfh);
-
private:
friend class RenderFrameHostManagerTest;
friend class TestWebContents;
@@ -445,11 +440,6 @@ class CONTENT_EXPORT RenderFrameHostManager
typedef base::hash_map<int32, RenderFrameHostImpl*> RenderFrameHostMap;
RenderFrameHostMap swapped_out_hosts_;
- // A map of RenderFrameHosts pending shutdown.
- typedef base::hash_map<int32, linked_ptr<RenderFrameHostImpl> >
- RFHPendingDeleteMap;
- RFHPendingDeleteMap pending_delete_hosts_;
-
// The intersitial page currently shown if any, not own by this class
// (the InterstitialPage is self-owned, it deletes itself when hidden).
InterstitialPageImpl* interstitial_page_;
@@ -463,8 +453,6 @@ class CONTENT_EXPORT RenderFrameHostManager
// process as its parent.
CrossProcessFrameConnector* cross_process_frame_connector_;
- base::WeakPtrFactory<RenderFrameHostManager> weak_factory_;
-
DISALLOW_COPY_AND_ASSIGN(RenderFrameHostManager);
};
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 2a82834..10f14ae 100644
--- a/content/browser/frame_host/render_frame_host_manager_unittest.cc
+++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc
@@ -172,10 +172,10 @@ class RenderFrameHostManagerTest
// BeforeUnload finishes.
ntp_rvh->SendShouldCloseACK(true);
+ // Assume SwapOutACK times out, so the dest_rvh proceeds and commits.
dest_rvh->SendNavigate(101, kDestUrl);
- ntp_rvh->OnSwappedOut(false);
- EXPECT_TRUE(ntp_rvh->IsSwappedOut());
+ EXPECT_TRUE(ntp_rvh->is_swapped_out());
return ntp_rvh;
}
@@ -299,7 +299,7 @@ TEST_F(RenderFrameHostManagerTest, FilterMessagesWhileSwappedOut) {
// The old renderer, being slow, now updates the title. It should be filtered
// out and not take effect.
- EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, ntp_rvh->rvh_state());
+ EXPECT_TRUE(ntp_rvh->is_swapped_out());
EXPECT_TRUE(ntp_rvh->OnMessageReceived(
ViewHostMsg_UpdateTitle(rvh()->GetRoutingID(), 0, ntp_title, direction)));
EXPECT_EQ(dest_title, contents()->GetTitle());
@@ -364,7 +364,7 @@ TEST_F(RenderFrameHostManagerTest, WhiteListDidActivateAcceleratedCompositing) {
// widgets.
TEST_F(RenderFrameHostManagerTest, GetRenderWidgetHostsReturnsActiveViews) {
TestRenderViewHost* swapped_out_rvh = CreateSwappedOutRenderViewHost();
- EXPECT_TRUE(swapped_out_rvh->IsSwappedOut());
+ EXPECT_TRUE(swapped_out_rvh->is_swapped_out());
scoped_ptr<RenderWidgetHostIterator> widgets(
RenderWidgetHost::GetRenderWidgetHosts());
@@ -374,8 +374,7 @@ TEST_F(RenderFrameHostManagerTest, GetRenderWidgetHostsReturnsActiveViews) {
RenderWidgetHost* widget = widgets->GetNextHost();
EXPECT_FALSE(widgets->GetNextHost());
RenderViewHost* rvh = RenderViewHost::From(widget);
- EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT,
- static_cast<RenderViewHostImpl*>(rvh)->rvh_state());
+ EXPECT_FALSE(static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out());
}
// Test if RenderViewHost::GetRenderWidgetHosts() returns a subset of
@@ -386,7 +385,7 @@ TEST_F(RenderFrameHostManagerTest, GetRenderWidgetHostsReturnsActiveViews) {
TEST_F(RenderFrameHostManagerTest,
GetRenderWidgetHostsWithinGetAllRenderWidgetHosts) {
TestRenderViewHost* swapped_out_rvh = CreateSwappedOutRenderViewHost();
- EXPECT_TRUE(swapped_out_rvh->IsSwappedOut());
+ EXPECT_TRUE(swapped_out_rvh->is_swapped_out());
scoped_ptr<RenderWidgetHostIterator> widgets(
RenderWidgetHost::GetRenderWidgetHosts());
@@ -772,7 +771,7 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) {
// CrossSiteResourceHandler::StartCrossSiteTransition triggers a
// call of RenderFrameHostManager::SwapOutOldPage before
// RenderFrameHostManager::DidNavigateMainFrame is called.
- // The RVH is swapped out after receiving the unload ack.
+ // The RVH is not swapped out until the commit.
manager->SwapOutOldPage();
EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching(
ViewMsg_SwapOut::ID));
@@ -800,7 +799,9 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) {
EXPECT_NE(host3, host);
EXPECT_NE(host3->GetProcess()->GetID(), host2_process_id);
- // Navigations in the new RVH should be suspended.
+ // Navigations in the new RVH should be suspended, which is ok because the
+ // old RVH is not yet swapped out and can respond to a second beforeunload
+ // request.
EXPECT_TRUE(static_cast<RenderViewHostImpl*>(
host3->render_view_host())->are_navigations_suspended());
EXPECT_EQ(host, manager->current_frame_host());
@@ -814,6 +815,7 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyReNavigation) {
// CrossSiteResourceHandler::StartCrossSiteTransition triggers a
// call of RenderFrameHostManager::SwapOutOldPage before
// RenderFrameHostManager::DidNavigateMainFrame is called.
+ // The RVH is not swapped out until the commit.
manager->SwapOutOldPage();
EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching(
ViewMsg_SwapOut::ID));
@@ -1030,12 +1032,14 @@ TEST_F(RenderFrameHostManagerTest, NavigateAfterMissingSwapOutACK) {
contents()->ProceedWithCrossSiteNavigation();
EXPECT_FALSE(rvh2->is_waiting_for_beforeunload_ack());
rvh2->SwapOut();
- EXPECT_TRUE(rvh2->IsWaitingForUnloadACK());
+ EXPECT_TRUE(rvh2->is_waiting_for_unload_ack());
- // The back navigation commits.
+ // The back navigation commits. We should proactively clear the
+ // is_waiting_for_unload_ack state to be safe.
const NavigationEntry* entry1 = contents()->GetController().GetPendingEntry();
rvh1->SendNavigate(entry1->GetPageID(), entry1->GetURL());
- EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, rvh2->rvh_state());
+ EXPECT_TRUE(rvh2->is_swapped_out());
+ EXPECT_FALSE(rvh2->is_waiting_for_unload_ack());
// We should be able to navigate forward.
contents()->GetController().GoForward();
@@ -1043,10 +1047,8 @@ TEST_F(RenderFrameHostManagerTest, NavigateAfterMissingSwapOutACK) {
const NavigationEntry* entry2 = contents()->GetController().GetPendingEntry();
rvh2->SendNavigate(entry2->GetPageID(), entry2->GetURL());
EXPECT_EQ(rvh2, rvh());
- EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh2->rvh_state());
- EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, rvh1->rvh_state());
- rvh1->OnSwappedOut(false);
- EXPECT_TRUE(rvh1->IsSwappedOut());
+ EXPECT_FALSE(rvh2->is_swapped_out());
+ EXPECT_TRUE(rvh1->is_swapped_out());
}
// Test that we create swapped out RVHs for the opener chain when navigating an
@@ -1093,13 +1095,13 @@ TEST_F(RenderFrameHostManagerTest, CreateSwappedOutOpenerRVHs) {
TestRenderViewHost* opener1_rvh = static_cast<TestRenderViewHost*>(
opener1_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance()));
EXPECT_TRUE(opener1_manager->IsRVHOnSwappedOutList(opener1_rvh));
- EXPECT_TRUE(opener1_rvh->IsSwappedOut());
+ EXPECT_TRUE(opener1_rvh->is_swapped_out());
// Ensure a swapped out RVH is created in the second opener tab.
TestRenderViewHost* opener2_rvh = static_cast<TestRenderViewHost*>(
opener2_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance()));
EXPECT_TRUE(opener2_manager->IsRVHOnSwappedOutList(opener2_rvh));
- EXPECT_TRUE(opener2_rvh->IsSwappedOut());
+ EXPECT_TRUE(opener2_rvh->is_swapped_out());
// Navigate to a cross-BrowsingInstance URL.
contents()->NavigateAndCommit(kChromeUrl);
@@ -1201,7 +1203,7 @@ TEST_F(RenderFrameHostManagerTest, EnableWebUIWithSwappedOutOpener) {
TestRenderViewHost* opener1_rvh = static_cast<TestRenderViewHost*>(
opener1_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance()));
EXPECT_TRUE(opener1_manager->IsRVHOnSwappedOutList(opener1_rvh));
- EXPECT_TRUE(opener1_rvh->IsSwappedOut());
+ EXPECT_TRUE(opener1_rvh->is_swapped_out());
// Ensure the new RVH has WebUI bindings.
EXPECT_TRUE(rvh2->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
@@ -1347,228 +1349,4 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyClose) {
EXPECT_EQ(host, manager->current_frame_host());
}
-// This checks that the given RVH has been properly deleted.
-class RenderViewHostDestructionObserver : public WebContentsObserver {
- public:
- RenderViewHostDestructionObserver(RenderViewHost* render_view_host)
- : WebContentsObserver(WebContents::FromRenderViewHost(render_view_host)),
- render_view_host_(render_view_host),
- rvh_deleted_(false) {}
-
- bool rvh_deleted() { return rvh_deleted_; }
-
- virtual void RenderViewDeleted(RenderViewHost* render_view_host) OVERRIDE {
- if (render_view_host == render_view_host_)
- rvh_deleted_ = true;
- }
-
- private:
- RenderViewHost* render_view_host_;
- bool rvh_deleted_;
-
- DISALLOW_COPY_AND_ASSIGN(RenderViewHostDestructionObserver);
-};
-
-// Tests that the RenderViewHost is properly deleted when the SwapOutACK is
-// received before the new page commits.
-TEST_F(RenderFrameHostManagerTest,
- SwapOutACKBeforeNewPageCommitsLeadsToDeletion) {
- const GURL kUrl1("http://www.google.com/");
- const GURL kUrl2("http://www.chromium.org/");
-
- // Navigate to the first page.
- contents()->NavigateAndCommit(kUrl1);
- TestRenderViewHost* rvh1 = test_rvh();
- RenderViewHostDestructionObserver destruction_observer(rvh1);
- EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh1->rvh_state());
-
- // Navigate to new site, simulating onbeforeunload approval.
- controller().LoadURL(kUrl2, Referrer(), PAGE_TRANSITION_LINK, std::string());
- base::TimeTicks now = base::TimeTicks::Now();
- rvh1->OnMessageReceived(ViewHostMsg_ShouldClose_ACK(0, true, now, now));
- EXPECT_TRUE(contents()->cross_navigation_pending());
- TestRenderViewHost* rvh2 =
- static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
-
- // Simulate rvh2's response, which leads to an unload request being sent to
- // rvh1.
- std::vector<GURL> url_chain;
- url_chain.push_back(GURL());
- contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
- rvh2, GlobalRequestID(0, 0), scoped_ptr<CrossSiteTransferringRequest>(),
- url_chain, Referrer(), PAGE_TRANSITION_TYPED, 1, false);
- EXPECT_TRUE(contents()->cross_navigation_pending());
- EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
- rvh1->rvh_state());
-
- // Simulate the swap out ack.
- rvh1->OnSwappedOut(false);
- EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_COMMIT, rvh1->rvh_state());
-
- // The new page commits.
- contents()->TestDidNavigate(rvh2, 1, kUrl2, PAGE_TRANSITION_TYPED);
- EXPECT_FALSE(contents()->cross_navigation_pending());
- EXPECT_EQ(rvh2, rvh());
- EXPECT_TRUE(contents()->GetPendingRenderViewHost() == NULL);
- EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh2->rvh_state());
-
- // rvh1 should have been deleted.
- EXPECT_TRUE(destruction_observer.rvh_deleted());
- rvh1 = NULL;
-}
-
-// Tests that the RenderViewHost is properly swapped out when the SwapOutACK is
-// received before the new page commits.
-TEST_F(RenderFrameHostManagerTest,
- SwapOutACKBeforeNewPageCommitsLeadsToSwapOut) {
- const GURL kUrl1("http://www.google.com/");
- const GURL kUrl2("http://www.chromium.org/");
-
- // Navigate to the first page.
- contents()->NavigateAndCommit(kUrl1);
- TestRenderViewHost* rvh1 = test_rvh();
- RenderViewHostDestructionObserver destruction_observer(rvh1);
- EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh1->rvh_state());
-
- // Increment the number of active views in SiteInstanceImpl so that rvh2 is
- // not deleted on swap out.
- static_cast<SiteInstanceImpl*>(
- rvh1->GetSiteInstance())->increment_active_view_count();
-
- // Navigate to new site, simulating onbeforeunload approval.
- controller().LoadURL(kUrl2, Referrer(), PAGE_TRANSITION_LINK, std::string());
- base::TimeTicks now = base::TimeTicks::Now();
- rvh1->OnMessageReceived(ViewHostMsg_ShouldClose_ACK(0, true, now, now));
- EXPECT_TRUE(contents()->cross_navigation_pending());
- TestRenderViewHost* rvh2 =
- static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
-
- // Simulate rvh2's response, which leads to an unload request being sent to
- // rvh1.
- std::vector<GURL> url_chain;
- url_chain.push_back(GURL());
- contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
- rvh2, GlobalRequestID(0, 0), scoped_ptr<CrossSiteTransferringRequest>(),
- url_chain, Referrer(), PAGE_TRANSITION_TYPED, 1, false);
- EXPECT_TRUE(contents()->cross_navigation_pending());
- EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
- rvh1->rvh_state());
-
- // Simulate the swap out ack.
- rvh1->OnSwappedOut(false);
- EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_COMMIT, rvh1->rvh_state());
-
- // The new page commits.
- contents()->TestDidNavigate(rvh2, 1, kUrl2, PAGE_TRANSITION_TYPED);
- EXPECT_FALSE(contents()->cross_navigation_pending());
- EXPECT_EQ(rvh2, rvh());
- EXPECT_TRUE(contents()->GetPendingRenderViewHost() == NULL);
- EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh2->rvh_state());
-
- // rvh1 should be swapped out.
- EXPECT_FALSE(destruction_observer.rvh_deleted());
- EXPECT_TRUE(rvh1->IsSwappedOut());
-}
-
-// Tests that the RenderViewHost is properly deleted when the new
-// page commits before the swap out ack is received.
-TEST_F(RenderFrameHostManagerTest,
- NewPageCommitsBeforeSwapOutACKLeadsToDeletion) {
- const GURL kUrl1("http://www.google.com/");
- const GURL kUrl2("http://www.chromium.org/");
-
- // Navigate to the first page.
- contents()->NavigateAndCommit(kUrl1);
- TestRenderViewHost* rvh1 = test_rvh();
- RenderViewHostDestructionObserver destruction_observer(rvh1);
- EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh1->rvh_state());
-
- // Navigate to new site, simulating onbeforeunload approval.
- controller().LoadURL(kUrl2, Referrer(), PAGE_TRANSITION_LINK, std::string());
- base::TimeTicks now = base::TimeTicks::Now();
- rvh1->OnMessageReceived(ViewHostMsg_ShouldClose_ACK(0, true, now, now));
- EXPECT_TRUE(contents()->cross_navigation_pending());
- TestRenderViewHost* rvh2 =
- static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
-
- // Simulate rvh2's response, which leads to an unload request being sent to
- // rvh1.
- std::vector<GURL> url_chain;
- url_chain.push_back(GURL());
- contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
- rvh2, GlobalRequestID(0, 0), scoped_ptr<CrossSiteTransferringRequest>(),
- url_chain, Referrer(), PAGE_TRANSITION_TYPED, 1, false);
- EXPECT_TRUE(contents()->cross_navigation_pending());
- EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
- rvh1->rvh_state());
-
- // The new page commits.
- contents()->TestDidNavigate(rvh2, 1, kUrl2, PAGE_TRANSITION_TYPED);
- EXPECT_FALSE(contents()->cross_navigation_pending());
- EXPECT_EQ(rvh2, rvh());
- EXPECT_TRUE(contents()->GetPendingRenderViewHost() == NULL);
- EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh2->rvh_state());
- EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SHUTDOWN, rvh1->rvh_state());
-
- // Simulate the swap out ack.
- rvh1->OnSwappedOut(false);
-
- // rvh1 should have been deleted.
- EXPECT_TRUE(destruction_observer.rvh_deleted());
- rvh1 = NULL;
-}
-
-// Tests that the RenderViewHost is properly swapped out when the new page
-// commits before the swap out ack is received.
-TEST_F(RenderFrameHostManagerTest,
- NewPageCommitsBeforeSwapOutACKLeadsToSwapOut) {
- const GURL kUrl1("http://www.google.com/");
- const GURL kUrl2("http://www.chromium.org/");
-
- // Navigate to the first page.
- contents()->NavigateAndCommit(kUrl1);
- TestRenderViewHost* rvh1 = test_rvh();
- RenderViewHostDestructionObserver destruction_observer(rvh1);
- EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh1->rvh_state());
-
- // Increment the number of active views in SiteInstanceImpl so that rvh1 is
- // not deleted on swap out.
- static_cast<SiteInstanceImpl*>(
- rvh1->GetSiteInstance())->increment_active_view_count();
-
- // Navigate to new site, simulating onbeforeunload approval.
- controller().LoadURL(kUrl2, Referrer(), PAGE_TRANSITION_LINK, std::string());
- base::TimeTicks now = base::TimeTicks::Now();
- rvh1->OnMessageReceived(ViewHostMsg_ShouldClose_ACK(0, true, now, now));
- EXPECT_TRUE(contents()->cross_navigation_pending());
- TestRenderViewHost* rvh2 =
- static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
-
- // Simulate rvh2's response, which leads to an unload request being sent to
- // rvh1.
- std::vector<GURL> url_chain;
- url_chain.push_back(GURL());
- contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
- rvh2, GlobalRequestID(0, 0), scoped_ptr<CrossSiteTransferringRequest>(),
- url_chain, Referrer(), PAGE_TRANSITION_TYPED, 1, false);
- EXPECT_TRUE(contents()->cross_navigation_pending());
- EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
- rvh1->rvh_state());
-
- // The new page commits.
- contents()->TestDidNavigate(rvh2, 1, kUrl2, PAGE_TRANSITION_TYPED);
- EXPECT_FALSE(contents()->cross_navigation_pending());
- EXPECT_EQ(rvh2, rvh());
- EXPECT_TRUE(contents()->GetPendingRenderViewHost() == NULL);
- EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh2->rvh_state());
- EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, rvh1->rvh_state());
-
- // Simulate the swap out ack.
- rvh1->OnSwappedOut(false);
-
- // rvh1 should be swapped out.
- EXPECT_FALSE(destruction_observer.rvh_deleted());
- EXPECT_TRUE(rvh1->IsSwappedOut());
-}
-
} // namespace content
diff --git a/content/browser/renderer_host/render_process_host_impl.h b/content/browser/renderer_host/render_process_host_impl.h
index 2fea656..943fd5f 100644
--- a/content/browser/renderer_host/render_process_host_impl.h
+++ b/content/browser/renderer_host/render_process_host_impl.h
@@ -17,6 +17,7 @@
#include "content/browser/geolocation/geolocation_dispatcher_host.h"
#include "content/browser/power_monitor_message_broadcaster.h"
#include "content/common/content_export.h"
+#include "content/public/browser/global_request_id.h"
#include "content/public/browser/gpu_data_manager_observer.h"
#include "content/public/browser/render_process_host.h"
#include "ipc/ipc_channel_proxy.h"
@@ -126,8 +127,6 @@ class CONTENT_EXPORT RenderProcessHostImpl
virtual void SetWebRtcLogMessageCallback(
base::Callback<void(const std::string&)> callback) OVERRIDE;
#endif
- virtual void ResumeDeferredNavigation(const GlobalRequestID& request_id)
- OVERRIDE;
// IPC::Sender via RenderProcessHost.
virtual bool Send(IPC::Message* msg) OVERRIDE;
@@ -142,6 +141,10 @@ class CONTENT_EXPORT RenderProcessHostImpl
scoped_refptr<AudioRendererHost> audio_renderer_host() const;
+ // Tells the ResourceDispatcherHost to resume a deferred navigation without
+ // transferring it to a new renderer process.
+ void ResumeDeferredNavigation(const GlobalRequestID& request_id);
+
// Call this function when it is evident that the child process is actively
// performing some operation, for example if we just received an IPC message.
void mark_child_process_activity_time() {
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index dec7a50..fd5a5fd 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -36,7 +36,6 @@
#include "content/browser/loader/resource_dispatcher_host_impl.h"
#include "content/browser/renderer_host/cross_site_transferring_request.h"
#include "content/browser/renderer_host/dip_util.h"
-#include "content/browser/renderer_host/input/timeout_monitor.h"
#include "content/browser/renderer_host/media/audio_renderer_host.h"
#include "content/browser/renderer_host/render_process_host_impl.h"
#include "content/browser/renderer_host/render_view_host_delegate.h"
@@ -152,16 +151,6 @@ void DismissVirtualKeyboardTask() {
// RenderViewHost, public:
// static
-bool RenderViewHostImpl::IsRVHStateActive(RenderViewHostImplState rvh_state) {
- if (rvh_state == STATE_DEFAULT ||
- rvh_state == STATE_WAITING_FOR_UNLOAD_ACK ||
- rvh_state == STATE_WAITING_FOR_COMMIT ||
- rvh_state == STATE_WAITING_FOR_CLOSE)
- return true;
- return false;
-}
-
-// static
RenderViewHost* RenderViewHost::FromID(int render_process_id,
int render_view_id) {
return RenderViewHostImpl::FromID(render_process_id, render_view_id);
@@ -198,32 +187,29 @@ RenderViewHostImpl::RenderViewHostImpl(
instance->GetProcess(),
routing_id,
hidden),
- frames_ref_count_(0),
delegate_(delegate),
instance_(static_cast<SiteInstanceImpl*>(instance)),
waiting_for_drag_context_response_(false),
enabled_bindings_(0),
navigations_suspended_(false),
has_accessed_initial_document_(false),
+ is_swapped_out_(swapped_out),
main_frame_routing_id_(main_frame_routing_id),
is_waiting_for_beforeunload_ack_(false),
+ is_waiting_for_unload_ack_(false),
+ has_timed_out_on_unload_(false),
unload_ack_is_for_cross_site_transition_(false),
are_javascript_messages_suppressed_(false),
sudden_termination_allowed_(false),
render_view_termination_status_(base::TERMINATION_STATUS_STILL_RUNNING),
- virtual_keyboard_requested_(false),
- weak_factory_(this) {
+ virtual_keyboard_requested_(false) {
DCHECK(instance_.get());
CHECK(delegate_); // http://crbug.com/82827
GetProcess()->EnableSendQueue();
- if (swapped_out) {
- rvh_state_ = STATE_SWAPPED_OUT;
- } else {
- rvh_state_ = STATE_DEFAULT;
+ if (!swapped_out)
instance_->increment_active_view_count();
- }
if (ResourceDispatcherHostImpl::Get()) {
BrowserThread::PostTask(
@@ -236,9 +222,6 @@ RenderViewHostImpl::RenderViewHostImpl(
#if defined(OS_ANDROID)
media_player_manager_.reset(BrowserMediaPlayerManager::Create(this));
#endif
-
- unload_event_monitor_timeout_.reset(new TimeoutMonitor(base::Bind(
- &RenderViewHostImpl::OnSwappedOut, weak_factory_.GetWeakPtr(), true)));
}
RenderViewHostImpl::~RenderViewHostImpl() {
@@ -258,7 +241,7 @@ RenderViewHostImpl::~RenderViewHostImpl() {
// If this was swapped out, it already decremented the active view
// count of the SiteInstance it belongs to.
- if (IsRVHStateActive(rvh_state_))
+ if (!is_swapped_out_)
instance_->decrement_active_view_count();
}
@@ -309,7 +292,7 @@ bool RenderViewHostImpl::CreateRenderView(
params.frame_name = frame_name;
// Ensure the RenderView sets its opener correctly.
params.opener_route_id = opener_route_id;
- params.swapped_out = !IsRVHStateActive(rvh_state_);
+ params.swapped_out = is_swapped_out_;
params.hidden = is_hidden();
params.next_page_id = next_page_id;
GetWebScreenInfo(&params.screen_info);
@@ -593,7 +576,7 @@ void RenderViewHostImpl::Navigate(const ViewMsg_Navigate_Params& params) {
} else {
// Get back to a clean state, in case we start a new navigation without
// completing a RVH swap or unload handler.
- SetState(STATE_DEFAULT);
+ SetSwappedOut(false);
Send(new ViewMsg_Navigate(GetRoutingID(), params));
}
@@ -639,7 +622,7 @@ void RenderViewHostImpl::SetNavigationsSuspended(
// There's navigation message params waiting to be sent. Now that we're not
// suspended anymore, resume navigation by sending them. If we were swapped
// out, we should also stop filtering out the IPC messages now.
- SetState(STATE_DEFAULT);
+ SetSwappedOut(false);
DCHECK(!proceed_time.is_null());
suspended_nav_params_->browser_navigation_start = proceed_time;
@@ -722,14 +705,22 @@ void RenderViewHostImpl::SuppressDialogsUntilSwapOut() {
}
void RenderViewHostImpl::SwapOut() {
- SetState(STATE_WAITING_FOR_UNLOAD_ACK);
- unload_event_monitor_timeout_->Start(
- base::TimeDelta::FromMilliseconds(kUnloadTimeoutMS));
+ // 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.
+ // Increment the in-flight event count, to ensure that input events won't
+ // cancel the timeout timer.
+ increment_in_flight_event_count();
+ StartHangMonitorTimeout(TimeDelta::FromMilliseconds(kUnloadTimeoutMS));
if (IsRenderViewLive()) {
Send(new ViewMsg_SwapOut(GetRoutingID()));
+ } else {
+ // This RenderViewHost doesn't have a live renderer, so just skip the unload
+ // event.
+ OnSwappedOut(true);
}
- delegate_->SwappedOut(this);
}
void RenderViewHostImpl::OnSwapOutACK() {
@@ -737,11 +728,36 @@ void RenderViewHostImpl::OnSwapOutACK() {
}
void RenderViewHostImpl::OnSwappedOut(bool timed_out) {
- // Ignore spurious swap out ack.
- if (!IsWaitingForUnloadACK())
- return;
- unload_event_monitor_timeout_->Stop();
- if (timed_out) {
+ // Stop the hang monitor now that the unload handler has finished.
+ decrement_in_flight_event_count();
+ StopHangMonitorTimeout();
+ is_waiting_for_unload_ack_ = false;
+ has_timed_out_on_unload_ = timed_out;
+ delegate_->SwappedOut(this);
+}
+
+void RenderViewHostImpl::WasSwappedOut() {
+ // Don't bother reporting hung state anymore.
+ StopHangMonitorTimeout();
+
+ // If we have timed out on running the unload handler, we consider
+ // the process hung and we should terminate it if there are no other tabs
+ // using the process. If there are other views using this process, the
+ // unresponsive renderer timeout will catch it.
+ bool hung = has_timed_out_on_unload_;
+
+ // Now that we're no longer the active RVH in the tab, start filtering out
+ // most IPC messages. Usually the renderer will have stopped sending
+ // messages as of OnSwapOutACK. However, we may have timed out waiting
+ // for that message, and additional IPC messages may keep streaming in.
+ // We filter them out, as long as that won't cause problems (e.g., we
+ // still allow synchronous messages through).
+ SetSwappedOut(true);
+
+ // If we are not running the renderer in process and no other tab is using
+ // the hung process, consider it eligible to be killed, assuming it is a real
+ // process (unit tests don't have real processes).
+ if (hung) {
base::ProcessHandle process_handle = GetProcess()->GetHandle();
int views = 0;
@@ -778,50 +794,13 @@ void RenderViewHostImpl::OnSwappedOut(bool timed_out) {
}
}
- switch (rvh_state_) {
- case STATE_WAITING_FOR_UNLOAD_ACK:
- SetState(STATE_WAITING_FOR_COMMIT);
- break;
- case STATE_PENDING_SWAP_OUT:
- SetState(STATE_SWAPPED_OUT);
- break;
- case STATE_PENDING_SHUTDOWN:
- DCHECK(!pending_shutdown_on_swap_out_.is_null());
- pending_shutdown_on_swap_out_.Run();
- break;
- default:
- NOTREACHED();
- }
-}
-
-void RenderViewHostImpl::WasSwappedOut(
- const base::Closure& pending_delete_on_swap_out) {
+ // Inform the renderer that it can exit if no one else is using it.
Send(new ViewMsg_WasSwappedOut(GetRoutingID()));
- if (rvh_state_ == STATE_WAITING_FOR_UNLOAD_ACK) {
- if (instance_->active_view_count())
- SetState(STATE_PENDING_SWAP_OUT);
- else
- SetPendingShutdown(pending_delete_on_swap_out);
- } else if (rvh_state_ == STATE_WAITING_FOR_COMMIT) {
- SetState(STATE_SWAPPED_OUT);
- } else if (rvh_state_ == STATE_DEFAULT) {
- // When the RenderView is not live, the RenderFrameHostManager will call
- // CommitPending directly, without calling SwapOut on the old RVH. This will
- // cause WasSwappedOut to be called directly on the live old RVH.
- DCHECK(!IsRenderViewLive());
- SetState(STATE_SWAPPED_OUT);
- } else {
- NOTREACHED();
- }
-}
-
-void RenderViewHostImpl::SetPendingShutdown(const base::Closure& on_swap_out) {
- pending_shutdown_on_swap_out_ = on_swap_out;
- SetState(STATE_PENDING_SHUTDOWN);
}
void RenderViewHostImpl::ClosePage() {
- SetState(STATE_WAITING_FOR_CLOSE);
+ // 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()) {
@@ -848,6 +827,7 @@ void RenderViewHostImpl::ClosePage() {
void RenderViewHostImpl::ClosePageIgnoringUnloadEvents() {
StopHangMonitorTimeout();
is_waiting_for_beforeunload_ack_ = false;
+ is_waiting_for_unload_ack_ = false;
sudden_termination_allowed_ = true;
delegate_->Close(this);
@@ -1020,7 +1000,8 @@ void RenderViewHostImpl::JavaScriptDialogClosed(
bool success,
const base::string16& user_input) {
GetProcess()->SetIgnoreInputEvents(false);
- bool is_waiting = is_waiting_for_beforeunload_ack_ || IsWaitingForUnloadACK();
+ bool is_waiting =
+ is_waiting_for_beforeunload_ack_ || is_waiting_for_unload_ack_;
// If we are executing as part of (before)unload event handling, we don't
// want to use the regular hung_renderer_delay_ms_ if the user has agreed to
@@ -1043,7 +1024,7 @@ void RenderViewHostImpl::JavaScriptDialogClosed(
// correctly while waiting for a response.
if (is_waiting && are_javascript_messages_suppressed_)
delegate_->RendererUnresponsive(
- this, is_waiting_for_beforeunload_ack_, IsWaitingForUnloadACK());
+ this, is_waiting_for_beforeunload_ack_, is_waiting_for_unload_ack_);
}
void RenderViewHostImpl::DragSourceEndedAt(
@@ -1199,7 +1180,7 @@ bool RenderViewHostImpl::OnMessageReceived(const IPC::Message& msg) {
// Filter out most IPC messages if this renderer is swapped out.
// We still want to handle certain ACKs to keep our state consistent.
- if (IsSwappedOut()) {
+ if (is_swapped_out_) {
if (!SwappedOutMessages::CanHandleWhileSwappedOut(msg)) {
// If this is a synchronous message and we decided not to handle it,
// we must send an error reply, or else the renderer will be stuck
@@ -1341,7 +1322,7 @@ void RenderViewHostImpl::OnShowView(int route_id,
WindowOpenDisposition disposition,
const gfx::Rect& initial_pos,
bool user_gesture) {
- if (IsRVHStateActive(rvh_state_)) {
+ if (!is_swapped_out_) {
delegate_->ShowCreatedWindow(
route_id, disposition, initial_pos, user_gesture);
}
@@ -1350,13 +1331,13 @@ void RenderViewHostImpl::OnShowView(int route_id,
void RenderViewHostImpl::OnShowWidget(int route_id,
const gfx::Rect& initial_pos) {
- if (IsRVHStateActive(rvh_state_))
+ if (!is_swapped_out_)
delegate_->ShowCreatedWidget(route_id, initial_pos);
Send(new ViewMsg_Move_ACK(route_id));
}
void RenderViewHostImpl::OnShowFullscreenWidget(int route_id) {
- if (IsRVHStateActive(rvh_state_))
+ if (!is_swapped_out_)
delegate_->ShowCreatedFullscreenWidget(route_id);
Send(new ViewMsg_Move_ACK(route_id));
}
@@ -1428,7 +1409,7 @@ void RenderViewHostImpl::OnUpdateEncoding(const std::string& encoding_name) {
}
void RenderViewHostImpl::OnUpdateTargetURL(int32 page_id, const GURL& url) {
- if (IsRVHStateActive(rvh_state_))
+ if (!is_swapped_out_)
delegate_->UpdateTargetURL(page_id, url);
// Send a notification back to the renderer that we are ready to
@@ -1449,7 +1430,7 @@ void RenderViewHostImpl::OnClose() {
}
void RenderViewHostImpl::OnRequestMove(const gfx::Rect& pos) {
- if (IsRVHStateActive(rvh_state_))
+ if (!is_swapped_out_)
delegate_->RequestMove(pos);
Send(new ViewMsg_Move_ACK(GetRoutingID()));
}
@@ -1678,7 +1659,7 @@ void RenderViewHostImpl::OnShouldCloseACK(
// If this renderer navigated while the beforeunload request was in flight, we
// may have cleared this state in OnNavigate, in which case we can ignore
// this message.
- if (!is_waiting_for_beforeunload_ack_ || rvh_state_ != STATE_DEFAULT)
+ if (!is_waiting_for_beforeunload_ack_ || is_swapped_out_)
return;
is_waiting_for_beforeunload_ack_ = false;
@@ -1721,7 +1702,7 @@ void RenderViewHostImpl::OnClosePageACK() {
void RenderViewHostImpl::NotifyRendererUnresponsive() {
delegate_->RendererUnresponsive(
- this, is_waiting_for_beforeunload_ack_, IsWaitingForUnloadACK());
+ this, is_waiting_for_beforeunload_ack_, is_waiting_for_unload_ack_);
}
void RenderViewHostImpl::NotifyRendererResponsive() {
@@ -1826,13 +1807,6 @@ void RenderViewHostImpl::ToggleSpeechInput() {
Send(new InputTagSpeechMsg_ToggleSpeechInput(GetRoutingID()));
}
-bool RenderViewHostImpl::IsWaitingForUnloadACK() const {
- return rvh_state_ == STATE_WAITING_FOR_UNLOAD_ACK ||
- rvh_state_ == STATE_WAITING_FOR_CLOSE ||
- rvh_state_ == STATE_PENDING_SHUTDOWN ||
- rvh_state_ == STATE_PENDING_SWAP_OUT;
-}
-
void RenderViewHostImpl::ExitFullscreen() {
RejectMouseLockOrUnlockIfNecessary();
// Notify delegate_ and renderer of fullscreen state change.
@@ -1845,7 +1819,7 @@ WebPreferences RenderViewHostImpl::GetWebkitPreferences() {
void RenderViewHostImpl::DisownOpener() {
// This should only be called when swapped out.
- DCHECK(IsSwappedOut());
+ DCHECK(is_swapped_out_);
Send(new ViewMsg_DisownOpener(GetRoutingID()));
}
@@ -1927,7 +1901,7 @@ void RenderViewHostImpl::NotifyMoveOrResizeStarted() {
void RenderViewHostImpl::OnAccessibilityEvents(
const std::vector<AccessibilityHostMsg_EventParams>& params) {
if ((accessibility_mode() & AccessibilityModeFlagPlatform) && view_ &&
- IsRVHStateActive(rvh_state_)) {
+ !is_swapped_out_) {
view_->CreateBrowserAccessibilityManagerIfNeeded();
BrowserAccessibilityManager* manager =
view_->GetBrowserAccessibilityManager();
@@ -1959,7 +1933,7 @@ void RenderViewHostImpl::OnAccessibilityEvents(
void RenderViewHostImpl::OnAccessibilityLocationChanges(
const std::vector<AccessibilityHostMsg_LocationChangeParams>& params) {
- if (view_ && IsRVHStateActive(rvh_state_)) {
+ if (view_ && !is_swapped_out_) {
view_->CreateBrowserAccessibilityManagerIfNeeded();
BrowserAccessibilityManager* manager =
view_->GetBrowserAccessibilityManager();
@@ -2055,25 +2029,22 @@ void RenderViewHostImpl::OnShowPopup(
}
#endif
-void RenderViewHostImpl::SetState(RenderViewHostImplState rvh_state) {
+void RenderViewHostImpl::SetSwappedOut(bool is_swapped_out) {
// We update the number of RenderViews in a SiteInstance when the
- // swapped out status of this RenderView gets flipped to/from live.
- if (!IsRVHStateActive(rvh_state_) && IsRVHStateActive(rvh_state))
+ // swapped out status of this RenderView gets flipped.
+ if (is_swapped_out_ && !is_swapped_out)
instance_->increment_active_view_count();
- else if (IsRVHStateActive(rvh_state_) && !IsRVHStateActive(rvh_state))
+ else if (!is_swapped_out_ && is_swapped_out)
instance_->decrement_active_view_count();
- // Whenever we change the RVH state to and from live or swapped out state, we
- // should not be waiting for beforeunload or unload acks. We clear them here
- // to be safe, since they can cause navigations to be ignored in OnNavigate.
- if (rvh_state == STATE_DEFAULT ||
- rvh_state == STATE_SWAPPED_OUT ||
- rvh_state_ == STATE_DEFAULT ||
- rvh_state_ == STATE_SWAPPED_OUT) {
- is_waiting_for_beforeunload_ack_ = false;
- }
- rvh_state_ = rvh_state;
+ is_swapped_out_ = is_swapped_out;
+ // Whenever we change swap out state, we should not be waiting for
+ // beforeunload or unload acks. We clear them here to be safe, since they
+ // can cause navigations to be ignored in OnNavigate.
+ is_waiting_for_beforeunload_ack_ = false;
+ is_waiting_for_unload_ack_ = false;
+ has_timed_out_on_unload_ = false;
}
bool RenderViewHostImpl::CanAccessFilesOfPageState(
diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h
index d161059..958afab2 100644
--- a/content/browser/renderer_host/render_view_host_impl.h
+++ b/content/browser/renderer_host/render_view_host_impl.h
@@ -9,7 +9,6 @@
#include <string>
#include <vector>
-#include "base/callback.h"
#include "base/compiler_specific.h"
#include "base/logging.h"
#include "base/memory/scoped_ptr.h"
@@ -66,7 +65,6 @@ class RenderWidgetHostDelegate;
class SessionStorageNamespace;
class SessionStorageNamespaceImpl;
class TestRenderViewHost;
-class TimeoutMonitor;
struct FileChooserParams;
struct Referrer;
struct ShowDesktopNotificationHostMsgParams;
@@ -101,39 +99,6 @@ class CONTENT_EXPORT RenderViewHostImpl
: public RenderViewHost,
public RenderWidgetHostImpl {
public:
- // Keeps track of the state of the RenderViewHostImpl, particularly with
- // respect to swap out.
- enum RenderViewHostImplState {
- // The standard state for a RVH handling the communication with a
- // RenderView.
- STATE_DEFAULT = 0,
- // The RVH has sent the SwapOut request to the renderer, but has not
- // received the SwapOutACK yet. The new page has not been committed yet
- // either.
- STATE_WAITING_FOR_UNLOAD_ACK,
- // The RVH received the SwapOutACK from the RenderView, but the new page has
- // not been committed yet.
- STATE_WAITING_FOR_COMMIT,
- // The RVH is waiting for the CloseACK from the RenderView.
- STATE_WAITING_FOR_CLOSE,
- // The RVH has not received the SwapOutACK yet, but the new page has
- // committed in a different RVH. The number of active views of the RVH
- // SiteInstanceImpl is not zero. Upon reception of the SwapOutACK, the RVH
- // will be swapped out.
- STATE_PENDING_SWAP_OUT,
- // The RVH has not received the SwapOutACK yet, but the new page has
- // committed in a different RVH. The number of active views of the RVH
- // SiteInstanceImpl is zero. Upon reception of the SwapOutACK, the RVH will
- // be shutdown.
- STATE_PENDING_SHUTDOWN,
- // The RVH is swapped out, and it is being used as a placeholder to allow
- // for cross-process communication.
- STATE_SWAPPED_OUT,
- };
- // Helper function to determine whether the RVH state should contribute to the
- // number of active views of a SiteInstance or not.
- static bool IsRVHStateActive(RenderViewHostImplState rvh_state);
-
// Convenience function, just like RenderViewHost::FromID.
static RenderViewHostImpl* FromID(int render_process_id, int render_view_id);
@@ -315,10 +280,7 @@ class CONTENT_EXPORT RenderViewHostImpl
// Whether this RenderViewHost has been swapped out to be displayed by a
// different process.
- bool IsSwappedOut() const { return rvh_state_ == STATE_SWAPPED_OUT; }
-
- // The current state of this RVH.
- RenderViewHostImplState rvh_state() const { return rvh_state_; }
+ bool is_swapped_out() const { return is_swapped_out_; }
// Called on the pending RenderViewHost when the network response is ready to
// commit. We should ensure that the old RenderViewHost runs its unload
@@ -351,15 +313,11 @@ class CONTENT_EXPORT RenderViewHostImpl
// out.
void OnSwappedOut(bool timed_out);
- // Called when the RenderFrameHostManager has swapped in a new
- // RenderFrameHost. Should |this| RVH switch to the pending shutdown state,
- // |pending_delete_on_swap_out| will be executed upon reception of the
- // SwapOutACK, or when the unload timer times out.
- void WasSwappedOut(const base::Closure& pending_delete_on_swap_out);
-
- // Set |this| as pending shutdown. |on_swap_out| will be called
- // when the SwapOutACK is received, or when the unload timer times out.
- void SetPendingShutdown(const base::Closure& on_swap_out);
+ // 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();
// Close the page ignoring whether it has unload events registers.
// This is called after the beforeunload and unload events have fired
@@ -490,8 +448,9 @@ class CONTENT_EXPORT RenderViewHostImpl
return is_waiting_for_beforeunload_ack_;
}
- // Whether the RVH is waiting for the unload ack from the renderer.
- bool IsWaitingForUnloadACK() const;
+ bool is_waiting_for_unload_ack() {
+ return is_waiting_for_unload_ack_;
+ }
// Update the FrameTree to use this RenderViewHost's main frame
// RenderFrameHost. Called when the RenderViewHost is committed.
@@ -509,18 +468,6 @@ class CONTENT_EXPORT RenderViewHostImpl
bool main_frame,
const GURL& url);
- // Increases the refcounting on this RVH. This is done by the FrameTree on
- // creation of a RenderFrameHost.
- void increment_ref_count() { ++frames_ref_count_; }
-
- // Decreases the refcounting on this RVH. This is done by the FrameTree on
- // destruction of a RenderFrameHost.
- void decrement_ref_count() { --frames_ref_count_; }
-
- // Returns the refcount on this RVH, that is the number of RenderFrameHosts
- // currently using it.
- int ref_count() { return frames_ref_count_; }
-
// NOTE: Do not add functions that just send an IPC message that are called in
// one or two places. Have the caller send the IPC message directly (unless
// the caller places are in different platforms, in which case it's better
@@ -635,15 +582,12 @@ class CONTENT_EXPORT RenderViewHostImpl
FRIEND_TEST_ALL_PREFIXES(RenderViewHostTest, BasicRenderFrameHost);
FRIEND_TEST_ALL_PREFIXES(RenderViewHostTest, RoutingIdSane);
- // Updates the state of this RenderViewHost and clears any waiting state
- // that is no longer relevant.
- void SetState(RenderViewHostImplState rvh_state);
+ // Sets whether this RenderViewHost is swapped out in favor of another,
+ // and clears any waiting state that is no longer relevant.
+ void SetSwappedOut(bool is_swapped_out);
bool CanAccessFilesOfPageState(const PageState& state) const;
- // The number of RenderFrameHosts which have a reference to this RVH.
- int frames_ref_count_;
-
// Our delegate, which wants to know about changes in the RenderView.
RenderViewHostDelegate* delegate_;
@@ -678,23 +622,29 @@ class CONTENT_EXPORT RenderViewHostImpl
// first commit.
bool has_accessed_initial_document_;
- // The current state of this RVH.
- RenderViewHostImplState rvh_state_;
+ // Whether this RenderViewHost is currently swapped out, such that the view is
+ // being rendered by another process.
+ bool is_swapped_out_;
// Routing ID for the main frame's RenderFrameHost.
int main_frame_routing_id_;
// Set to true when there is a pending ViewMsg_ShouldClose message. This
// ensures we don't spam the renderer with multiple beforeunload requests.
- // When either this value or IsWaitingForUnloadACK is true, the value of
+ // When either this value or is_waiting_for_unload_ack_ is true, the value of
// unload_ack_is_for_cross_site_transition_ indicates whether this is for a
// cross-site transition or a tab close attempt.
- // TODO(clamy): Remove this boolean and add one more state to the state
- // machine.
bool is_waiting_for_beforeunload_ack_;
+ // Set to true when there is a pending ViewMsg_Close message. Also see
+ // is_waiting_for_beforeunload_ack_, unload_ack_is_for_cross_site_transition_.
+ bool is_waiting_for_unload_ack_;
+
+ // Set to true when waiting for ViewHostMsg_SwapOut_ACK has timed out.
+ bool has_timed_out_on_unload_;
+
// Valid only when is_waiting_for_beforeunload_ack_ or
- // IsWaitingForUnloadACK is true. This tells us if the unload request
+ // is_waiting_for_unload_ack_ is true. This tells us if the unload request
// is for closing the entire tab ( = false), or only this RenderViewHost in
// the case of a cross-site transition ( = true).
bool unload_ack_is_for_cross_site_transition_;
@@ -729,17 +679,6 @@ class CONTENT_EXPORT RenderViewHostImpl
scoped_ptr<BrowserMediaPlayerManager> media_player_manager_;
#endif
- // Used to swap out or shutdown this RVH when the unload event is taking too
- // long to execute, depending on the number of active views in the
- // SiteInstance.
- scoped_ptr<TimeoutMonitor> unload_event_monitor_timeout_;
-
- // Called after receiving the SwapOutACK when the RVH is in state pending
- // shutdown. Also called if the unload timer times out.
- base::Closure pending_shutdown_on_swap_out_;
-
- base::WeakPtrFactory<RenderViewHostImpl> weak_factory_;
-
DISALLOW_COPY_AND_ASSIGN(RenderViewHostImpl);
};
diff --git a/content/browser/renderer_host/render_view_host_unittest.cc b/content/browser/renderer_host/render_view_host_unittest.cc
index f2f1d97..73060c3 100644
--- a/content/browser/renderer_host/render_view_host_unittest.cc
+++ b/content/browser/renderer_host/render_view_host_unittest.cc
@@ -72,9 +72,10 @@ TEST_F(RenderViewHostTest, CreateFullscreenWidget) {
test_rvh()->CreateNewFullscreenWidget(routing_id);
}
-// Makes sure that the RenderViewHost is not waiting for an unload ack when
-// reloading a page. If this is not the case, when reloading, the contents may
-// get closed out even though the user pressed the reload button.
+// Makes sure that RenderViewHost::is_waiting_for_unload_ack_ is false when
+// reloading a page. If is_waiting_for_unload_ack_ is not false when reloading
+// the contents may get closed out even though the user pressed the reload
+// button.
TEST_F(RenderViewHostTest, ResetUnloadOnReload) {
const GURL url1("http://foo1");
const GURL url2("http://foo2");
@@ -84,11 +85,12 @@ TEST_F(RenderViewHostTest, ResetUnloadOnReload) {
// . go to a page.
// . go to a new page, preferably one that takes a while to resolve, such
// as one on a site that doesn't exist.
- // . After this step IsWaitingForUnloadACK returns true on the first RVH.
+ // . After this step is_waiting_for_unload_ack_ has been set to true on
+ // the first RVH.
// . click stop before the page has been commited.
// . click reload.
- // . IsWaitingForUnloadACK still returns true, and if the hang monitor fires
- // the contents gets closed.
+ // . is_waiting_for_unload_ack_ is still true, and the if the hang monitor
+ // fires the contents gets closed.
NavigateAndCommit(url1);
controller().LoadURL(
@@ -99,7 +101,7 @@ TEST_F(RenderViewHostTest, ResetUnloadOnReload) {
test_rvh()->SendShouldCloseACK(true);
contents()->Stop();
controller().Reload(false);
- EXPECT_FALSE(test_rvh()->IsWaitingForUnloadACK());
+ EXPECT_FALSE(test_rvh()->is_waiting_for_unload_ack());
}
// Ensure we do not grant bindings to a process shared with unprivileged views.
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 8529331..b4f54df 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -313,8 +313,7 @@ scoped_ptr<RenderWidgetHostIterator> RenderWidgetHost::GetRenderWidgetHosts() {
// Add only active RenderViewHosts.
RenderViewHost* rvh = RenderViewHost::From(widget);
- if (RenderViewHostImpl::IsRVHStateActive(
- static_cast<RenderViewHostImpl*>(rvh)->rvh_state()))
+ if (!static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out())
hosts->Add(widget);
}
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 3c0645b..2095af8 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -2779,7 +2779,7 @@ void WebContentsImpl::RenderViewCreated(RenderViewHost* render_view_host) {
// Don't send notifications if we are just creating a swapped-out RVH for
// the opener chain. These won't be used for view-source or WebUI, so it's
// ok to return early.
- if (static_cast<RenderViewHostImpl*>(render_view_host)->IsSwappedOut())
+ if (static_cast<RenderViewHostImpl*>(render_view_host)->is_swapped_out())
return;
if (delegate_)
@@ -3061,7 +3061,7 @@ void WebContentsImpl::RequestOpenURL(RenderViewHost* rvh,
bool user_gesture) {
// If this came from a swapped out RenderViewHost, we only allow the request
// if we are still in the same BrowsingInstance.
- if (static_cast<RenderViewHostImpl*>(rvh)->IsSwappedOut() &&
+ if (static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out() &&
!rvh->GetSiteInstance()->IsRelatedSiteInstance(GetSiteInstance())) {
return;
}
@@ -3232,7 +3232,7 @@ void WebContentsImpl::RunJavaScriptMessage(
// showing an interstitial as it's shown over the previous page and we don't
// want the hidden page's dialogs to interfere with the interstitial.
bool suppress_this_message =
- static_cast<RenderViewHostImpl*>(rvh)->IsSwappedOut() ||
+ static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out() ||
ShowingInterstitialPage() ||
!delegate_ ||
delegate_->ShouldSuppressDialogs() ||
@@ -3277,7 +3277,7 @@ void WebContentsImpl::RunBeforeUnloadConfirm(RenderViewHost* rvh,
delegate_->WillRunBeforeUnloadConfirm();
bool suppress_this_message =
- rvhi->rvh_state() != RenderViewHostImpl::STATE_DEFAULT ||
+ rvhi->is_swapped_out() ||
!delegate_ ||
delegate_->ShouldSuppressDialogs() ||
!delegate_->GetJavaScriptDialogManager();
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index 635543d..2d9259f 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -513,7 +513,6 @@ TEST_F(WebContentsImplTest, CrossSiteBoundaries) {
EXPECT_TRUE(contents()->GetRenderManagerForTesting()->
IsOnSwappedOutList(pending_rfh));
EXPECT_EQ(pending_rvh_delete_count, 0);
- pending_rvh->OnSwappedOut(false);
// Close contents and ensure RVHs are deleted.
DeleteContents();
@@ -711,7 +710,6 @@ TEST_F(WebContentsImplTest, NavigateDoesNotUseUpSiteInstance) {
EXPECT_TRUE(contents()->GetRenderManagerForTesting()->IsOnSwappedOutList(
orig_rfh));
EXPECT_EQ(orig_rvh_delete_count, 0);
- orig_rvh->OnSwappedOut(false);
// Close contents and ensure RVHs are deleted.
DeleteContents();
@@ -1140,7 +1138,7 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationCanceled) {
EXPECT_TRUE(contents()->cross_navigation_pending());
// Simulate swap out message when the response arrives.
- orig_rvh->OnSwappedOut(false);
+ orig_rvh->set_is_swapped_out(true);
// Suppose the navigation doesn't get a chance to commit, and the user
// navigates in the current RVH's SiteInstance.
@@ -1152,7 +1150,7 @@ TEST_F(WebContentsImplTest, CrossSiteNavigationCanceled) {
SiteInstance* instance2 = contents()->GetSiteInstance();
EXPECT_FALSE(contents()->cross_navigation_pending());
EXPECT_EQ(orig_rvh, rvh());
- EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, orig_rvh->rvh_state());
+ EXPECT_FALSE(orig_rvh->is_swapped_out());
EXPECT_EQ(instance1, instance2);
EXPECT_TRUE(contents()->GetPendingRenderViewHost() == NULL);
}
diff --git a/content/public/browser/render_process_host.h b/content/public/browser/render_process_host.h
index a259f6d..b6ffaf9 100644
--- a/content/public/browser/render_process_host.h
+++ b/content/public/browser/render_process_host.h
@@ -29,7 +29,6 @@ class BrowserMessageFilter;
class RenderProcessHostObserver;
class RenderWidgetHost;
class StoragePartition;
-struct GlobalRequestID;
typedef base::Thread* (*RendererMainThreadFactoryFunction)(
const std::string& id);
@@ -224,10 +223,6 @@ class CONTENT_EXPORT RenderProcessHost : public IPC::Sender,
base::Callback<void(const std::string&)> callback) = 0;
#endif
- // Tells the ResourceDispatcherHost to resume a deferred navigation without
- // transferring it to a new renderer process.
- virtual void ResumeDeferredNavigation(const GlobalRequestID& request_id) = 0;
-
// Static management functions -----------------------------------------------
// Flag to run the renderer in process. This is primarily
diff --git a/content/public/test/mock_render_process_host.cc b/content/public/test/mock_render_process_host.cc
index a2b058a..5e67bf8 100644
--- a/content/public/test/mock_render_process_host.cc
+++ b/content/public/test/mock_render_process_host.cc
@@ -12,7 +12,6 @@
#include "content/browser/renderer_host/render_view_host_impl.h"
#include "content/browser/renderer_host/render_widget_host_impl.h"
#include "content/common/child_process_host_impl.h"
-#include "content/public/browser/global_request_id.h"
#include "content/public/browser/render_widget_host_iterator.h"
#include "content/public/browser/storage_partition.h"
@@ -264,9 +263,6 @@ void MockRenderProcessHost::SetWebRtcLogMessageCallback(
}
#endif
-void MockRenderProcessHost::ResumeDeferredNavigation(
- const GlobalRequestID& request_id) {}
-
bool MockRenderProcessHost::OnMessageReceived(const IPC::Message& msg) {
IPC::Listener* listener = listeners_.Lookup(msg.routing_id());
if (listener)
diff --git a/content/public/test/mock_render_process_host.h b/content/public/test/mock_render_process_host.h
index 063cb8e..d3c9739 100644
--- a/content/public/test/mock_render_process_host.h
+++ b/content/public/test/mock_render_process_host.h
@@ -81,8 +81,6 @@ class MockRenderProcessHost : public RenderProcessHost {
virtual void SetWebRtcLogMessageCallback(
base::Callback<void(const std::string&)> callback) OVERRIDE;
#endif
- virtual void ResumeDeferredNavigation(const GlobalRequestID& request_id)
- OVERRIDE;
// IPC::Sender via RenderProcessHost.
virtual bool Send(IPC::Message* msg) OVERRIDE;
diff --git a/content/public/test/test_renderer_host.cc b/content/public/test/test_renderer_host.cc
index dc323ca..2c13db4 100644
--- a/content/public/test/test_renderer_host.cc
+++ b/content/public/test/test_renderer_host.cc
@@ -46,8 +46,7 @@ RenderViewHost* RenderViewHostTester::GetPendingForController(
// static
bool RenderViewHostTester::IsRenderViewHostSwappedOut(RenderViewHost* rvh) {
- return static_cast<RenderViewHostImpl*>(rvh)->rvh_state() ==
- RenderViewHostImpl::STATE_SWAPPED_OUT;
+ return static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out();
}
// static
diff --git a/content/test/test_render_view_host.h b/content/test/test_render_view_host.h
index 985b4d9..cf963e4 100644
--- a/content/test/test_render_view_host.h
+++ b/content/test/test_render_view_host.h
@@ -285,10 +285,16 @@ class TestRenderViewHost
return is_waiting_for_beforeunload_ack_;
}
+ // Returns whether the RenderViewHost is currently waiting to hear the result
+ // of an unload handler from the renderer.
+ bool is_waiting_for_unload_ack() const {
+ return is_waiting_for_unload_ack_;
+ }
+
// Sets whether the RenderViewHost is currently swapped out, and thus
// filtering messages from the renderer.
- void set_rvh_state(RenderViewHostImplState rvh_state) {
- rvh_state_ = rvh_state;
+ void set_is_swapped_out(bool is_swapped_out) {
+ is_swapped_out_ = is_swapped_out;
}
// If set, navigations will appear to have loaded through a proxy
diff --git a/content/test/test_web_contents.cc b/content/test/test_web_contents.cc
index ec14917..b7efad0 100644
--- a/content/test/test_web_contents.cc
+++ b/content/test/test_web_contents.cc
@@ -151,11 +151,12 @@ void TestWebContents::CommitPendingNavigation() {
page_id = GetMaxPageIDForSiteInstance(rvh->GetSiteInstance()) + 1;
}
- rvh->SendNavigate(page_id, entry->GetURL());
- // Simulate the SwapOut_ACK. This is needed when cross-site navigation happens
- // (old_rvh != rvh).
+ // Simulate the SwapOut_ACK that happens when we swap out the old
+ // RVH, before the navigation commits. This is needed when
+ // cross-site navigation happens (old_rvh != rvh).
if (old_rvh != rvh)
static_cast<RenderViewHostImpl*>(old_rvh)->OnSwappedOut(false);
+ rvh->SendNavigate(page_id, entry->GetURL());
}
void TestWebContents::ProceedWithCrossSiteNavigation() {