Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(597)

Unified Diff: content/browser/frame_host/render_frame_host_manager.cc

Issue 404613005: Start using RenderFrameProxyHost objects. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: content/browser/frame_host/render_frame_host_manager.cc
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index 9973a8657557899523f41b4e45c838445fc7a3ce..292794533232019afa65737f9c1d2452ee033de1 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -538,7 +538,10 @@ void RenderFrameHostManager::SwapOutOldPage() {
RenderFrameProxyHost* proxy = new RenderFrameProxyHost(
render_frame_host_->GetSiteInstance(), frame_tree_node_);
- proxy_hosts_[render_frame_host_->GetSiteInstance()->GetId()] = proxy;
+ std::pair<RenderFrameProxyHostMap::iterator, bool> result =
+ proxy_hosts_.insert(std::make_pair(
+ render_frame_host_->GetSiteInstance()->GetId(), proxy));
+ CHECK(result.second) << "Inserting a duplicate item.";
// Tell the old frame it is being swapped out. This will fire the unload
// handler in the background (without firing the beforeunload handler a second
@@ -909,24 +912,13 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrameHost(
if (frame_routing_id == MSG_ROUTING_NONE)
frame_routing_id = site_instance->GetProcess()->GetNextRoutingID();
- // Create a RVH for main frames, or find the existing one for subframes.
+ // Create a RVH for the new frame or find an existing one.
FrameTree* frame_tree = frame_tree_node_->frame_tree();
- RenderViewHostImpl* render_view_host = NULL;
- if (frame_tree_node_->IsMainFrame()) {
- render_view_host = frame_tree->CreateRenderViewHostForMainFrame(
+ RenderViewHostImpl* render_view_host =
+ frame_tree->GetRenderViewHost(site_instance);
+ if (!render_view_host) {
+ render_view_host = frame_tree->CreateRenderViewHost(
site_instance, view_routing_id, frame_routing_id, swapped_out, hidden);
- } else {
- render_view_host = frame_tree->GetRenderViewHostForSubFrame(site_instance);
-
- // If we haven't found a RVH for a subframe RFH, it's because we currently
- // do not create top-level RFHs for pending subframe navigations. Create
- // the RVH here for now.
- // TODO(creis): Mirror the frame tree so this check isn't necessary.
- if (!render_view_host) {
- render_view_host = frame_tree->CreateRenderViewHostForMainFrame(
- site_instance, view_routing_id, frame_routing_id, swapped_out,
- hidden);
- }
}
// TODO(creis): Pass hidden to RFH.
@@ -940,14 +932,20 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrameHost(
return render_frame_host.Pass();
}
-int RenderFrameHostManager::CreateRenderFrame(
- SiteInstance* instance,
- int opener_route_id,
- bool swapped_out,
- bool hidden) {
+int RenderFrameHostManager::CreateRenderFrame(SiteInstance* instance,
+ int opener_route_id,
+ bool swapped_out,
+ bool for_main_frame,
+ bool hidden) {
CHECK(instance);
DCHECK(!swapped_out || hidden); // Swapped out views should always be hidden.
+ // TODO(nasko): Remove this check once cross-site navigation no longer
+ // relies on swapped out RFH for the top-level frame.
+ if (!frame_tree_node_->IsMainFrame()) {
+ CHECK(!swapped_out);
+ }
+
scoped_ptr<RenderFrameHostImpl> new_render_frame_host;
RenderFrameHostImpl* frame_to_announce = NULL;
int routing_id = MSG_ROUTING_NONE;
@@ -962,7 +960,7 @@ int RenderFrameHostManager::CreateRenderFrame(
RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
if (proxy) {
- routing_id = proxy->GetRenderViewHost()->GetRoutingID();
+ routing_id = proxy->GetRoutingID();
// Delete the existing RenderFrameProxyHost, but reuse the RenderFrameHost.
// Prevent the process from exiting while we're trying to use it.
if (!swapped_out) {
@@ -1007,11 +1005,19 @@ int RenderFrameHostManager::CreateRenderFrame(
}
bool success = InitRenderView(
- render_view_host, opener_route_id, proxy_routing_id,
- frame_tree_node_->IsMainFrame());
- if (success && frame_tree_node_->IsMainFrame()) {
- // Don't show the main frame's view until we get a DidNavigate from it.
- render_view_host->GetView()->Hide();
+ render_view_host, opener_route_id, proxy_routing_id, for_main_frame);
+ if (success) {
+ if (frame_tree_node_->IsMainFrame()) {
+ // Don't show the main frame's view until we get a DidNavigate from it.
+ render_view_host->GetView()->Hide();
+ } else if (!swapped_out) {
+ // Init the RFH, so a RenderFrame is created in the renderer.
+ DCHECK(new_render_frame_host.get());
+ success = InitRenderFrame(new_render_frame_host.get());
+ }
+ if (swapped_out) {
+ proxy_hosts_[instance->GetId()]->InitRenderFrameProxy();
+ }
} else if (!swapped_out && pending_render_frame_host_) {
CancelPending();
}
@@ -1030,6 +1036,23 @@ int RenderFrameHostManager::CreateRenderFrame(
return routing_id;
}
+int RenderFrameHostManager::CreateRenderFrameProxy(SiteInstance* instance,
+ int opener_route_id) {
ncarter (slow) 2014/07/21 20:31:56 |opener_route_id| is unused (probably intentionall
kenrb 2014/07/24 17:53:53 I am removing it for now. I think Nasko intends to
+ // A RenderFrameProxyHost should never be created in the same SiteInstance as
+ // the current RFH.
+ CHECK(instance);
+ CHECK_NE(instance, render_frame_host_->GetSiteInstance());
+
+ RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
+ if (proxy)
+ return proxy->GetRoutingID();
+
+ proxy = new RenderFrameProxyHost(instance, frame_tree_node_);
+ proxy_hosts_[instance->GetId()] = proxy;
+ proxy->InitRenderFrameProxy();
+ return proxy->GetRoutingID();
+}
+
bool RenderFrameHostManager::InitRenderView(RenderViewHost* render_view_host,
int opener_route_id,
int proxy_routing_id,
@@ -1058,6 +1081,36 @@ bool RenderFrameHostManager::InitRenderView(RenderViewHost* render_view_host,
render_view_host, opener_route_id, proxy_routing_id, for_main_frame);
}
+bool RenderFrameHostManager::InitRenderFrame(
+ RenderFrameHost* render_frame_host) {
+ RenderFrameHostImpl* rfh =
+ static_cast<RenderFrameHostImpl*>(render_frame_host);
+ if (rfh->IsRenderFrameLive())
+ return true;
+
+ int parent_routing_id = MSG_ROUTING_NONE;
+ if (frame_tree_node_->parent()) {
+ parent_routing_id = frame_tree_node_->parent()->render_manager()->
+ GetRoutingIdForSiteInstance(render_frame_host->GetSiteInstance());
+ CHECK_NE(parent_routing_id, MSG_ROUTING_NONE);
+ }
+ return delegate_->CreateRenderFrameForRenderManager(render_frame_host,
+ parent_routing_id);
+}
+
+int RenderFrameHostManager::GetRoutingIdForSiteInstance(
+ SiteInstance* site_instance) {
+ if (render_frame_host_->GetSiteInstance() == site_instance)
+ return render_frame_host_->GetRoutingID();
+
+ RenderFrameProxyHostMap::iterator iter =
+ proxy_hosts_.find(site_instance->GetId());
+ if (iter != proxy_hosts_.end())
+ return iter->second->GetRoutingID();
+
+ return MSG_ROUTING_NONE;
+}
+
void RenderFrameHostManager::CommitPending() {
// First check whether we're going to want to focus the location bar after
// this commit. We do this now because the navigation hasn't formally
@@ -1175,7 +1228,7 @@ void RenderFrameHostManager::CommitPending() {
// the RenderFrameHost should be put in the map of RenderFrameHosts pending
// shutdown. Otherwise, it is stored in the map of proxy hosts.
if (old_render_frame_host->render_view_host()->rvh_state() ==
- RenderViewHostImpl::STATE_PENDING_SHUTDOWN) {
+ RenderViewHostImpl::STATE_PENDING_SHUTDOWN) {
// The proxy for this RenderFrameHost is created when sending the
// SwapOut message, so check if it already exists and delete it.
RenderFrameProxyHostMap::iterator iter =
@@ -1192,16 +1245,22 @@ void RenderFrameHostManager::CommitPending() {
linked_ptr<RenderFrameHostImpl>(old_render_frame_host.release());
}
} else {
+ CHECK(proxy_hosts_.find(render_frame_host_->GetSiteInstance()->GetId()) ==
+ proxy_hosts_.end());
+
// Capture the active view count on the old RFH SiteInstance, since the
- // ownership will be passed into the proxy and the pointer will be invalid.
+ // ownership might be passed into the proxy and the pointer will be
+ // invalid.
int active_view_count =
static_cast<SiteInstanceImpl*>(old_render_frame_host->GetSiteInstance())
->active_view_count();
- RenderFrameProxyHostMap::iterator iter =
- proxy_hosts_.find(old_site_instance_id);
- CHECK(iter != proxy_hosts_.end());
- iter->second->TakeFrameHostOwnership(old_render_frame_host.Pass());
+ if (is_main_frame) {
+ RenderFrameProxyHostMap::iterator iter =
+ proxy_hosts_.find(old_site_instance_id);
+ CHECK(iter != proxy_hosts_.end());
+ iter->second->TakeFrameHostOwnership(old_render_frame_host.Pass());
+ }
ncarter (slow) 2014/07/21 20:31:56 To be clear, this now means we're freeing old_rend
kenrb 2014/07/24 17:53:53 This appears to be correct, that we are destroying
// 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
@@ -1262,11 +1321,12 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
cross_navigation_pending_ = false;
}
- // render_frame_host_'s SiteInstance and new_instance will not be deleted
- // before the end of this method, so we don't have to worry about their ref
- // counts dropping to zero.
+ // render_frame_host_'s SiteInstance will not be deleted before the end of
+ // this method, so we don't have to worry about the ref count dropping to
+ // zero. The new_instance is possibly passed to CreateProxiesForSiteInstance,
+ // which expects properly ref-counted object, so use a scoped_refptr for it.
SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
- SiteInstance* new_instance = current_instance;
+ scoped_refptr<SiteInstance> new_instance(current_instance);
// We do not currently swap processes for navigations in webview tag guests.
bool is_guest_scheme = current_instance->GetSiteURL().SchemeIs(kGuestScheme);
@@ -1308,11 +1368,38 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
if (new_instance->IsRelatedSiteInstance(current_instance)) {
opener_route_id =
delegate_->CreateOpenerRenderViewsForRenderManager(new_instance);
+
+ if (CommandLine::ForCurrentProcess()->HasSwitch(
+ switches::kSitePerProcess)) {
+ // Create the swapped out RVH for the new SiteInstance. This will create
+ // a top-level swapped out RFH as well, which will be wrapped by a
+ // RenderFrameProxyHost in the subsequent call to create proxies.
+ if (!frame_tree_node_->IsMainFrame()) {
+ RenderViewHostImpl* render_view_host =
+ frame_tree_node_->frame_tree()->GetRenderViewHost(new_instance);
+ if (!render_view_host) {
+ frame_tree_node_->frame_tree()->root()->render_manager()
+ ->CreateRenderFrame(new_instance,
+ MSG_ROUTING_NONE,
+ true,
+ false,
+ true);
+ }
+ }
+
+ // Ensure that the frame tree has RenderFrameProxyHosts for the new
+ // SiteInstance in all nodes except the current one.
+ frame_tree_node_->frame_tree()->CreateProxiesForSiteInstance(
+ frame_tree_node_, new_instance);
+ }
}
// Create a non-swapped-out pending RFH with the given opener and navigate
// it.
- int route_id = CreateRenderFrame(new_instance, opener_route_id, false,
+ int route_id = CreateRenderFrame(new_instance,
+ opener_route_id,
+ false,
+ frame_tree_node_->IsMainFrame(),
delegate_->IsHidden());
if (route_id == MSG_ROUTING_NONE)
return NULL;
@@ -1437,7 +1524,9 @@ void RenderFrameHostManager::CancelPending() {
new RenderFrameProxyHost(site_instance, frame_tree_node_);
proxy_hosts_[site_instance->GetId()] = proxy;
pending_render_frame_host->SwapOut(proxy);
- proxy->TakeFrameHostOwnership(pending_render_frame_host.Pass());
+ if (frame_tree_node_->IsMainFrame())
+ proxy->TakeFrameHostOwnership(pending_render_frame_host.Pass());
+ proxy_hosts_[site_instance->GetId()] = proxy;
ncarter (slow) 2014/07/21 20:31:56 If it's really necessary to do this both here and
kenrb 2014/07/24 17:53:53 Removed. This looks like a no-op.
} else {
// We won't be coming back, so delete this one.
pending_render_frame_host.reset();
@@ -1478,6 +1567,10 @@ bool RenderFrameHostManager::IsRVHOnSwappedOutList(
rvh->GetSiteInstance());
if (!proxy)
return false;
+ // If there is a proxy without RFH, it is for a subframe in the SiteInstance
+ // of |rvh|. Subframes should be ignored in this case.
+ if (!proxy->render_frame_host())
+ return false;
return IsOnSwappedOutList(proxy->render_frame_host());
}

Powered by Google App Engine
This is Rietveld 408576698