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

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

Issue 753173002: Preparation steps for adding speculative renderer creation. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Minor missing things. Created 6 years, 1 month 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 7780cc05e2c90df1464137488038106a0f2316f6..c20a8bd3462a1d8c4f59c987f9ac4c9aad00e396 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -134,23 +134,25 @@ RenderFrameProxyHost* RenderFrameHostManager::GetProxyToParent() {
return iter->second;
}
-void RenderFrameHostManager::SetPendingWebUI(const GURL& url,
- int bindings) {
- pending_web_ui_.reset(
- delegate_->CreateWebUIForRenderManager(url));
+void RenderFrameHostManager::SetPendingWebUI(const GURL& url, int bindings) {
+ pending_web_ui_.reset(CreateWebUI(url, bindings));
pending_and_current_web_ui_.reset();
+}
+
+WebUIImpl* RenderFrameHostManager::CreateWebUI(const GURL& url, int bindings) {
+ WebUIImpl* new_web_ui = delegate_->CreateWebUIForRenderManager(url);
// If we have assigned (zero or more) bindings to this NavigationEntry in the
// past, make sure we're not granting it different bindings than it had
// before. If so, note it and don't give it any bindings, to avoid a
// potential privilege escalation.
- if (pending_web_ui_.get() &&
- bindings != NavigationEntryImpl::kInvalidBindings &&
- pending_web_ui_->GetBindings() != bindings) {
- RecordAction(
- base::UserMetricsAction("ProcessSwapBindingsMismatch_RVHM"));
- pending_web_ui_.reset();
+ if (new_web_ui && bindings != NavigationEntryImpl::kInvalidBindings &&
+ new_web_ui->GetBindings() != bindings) {
+ RecordAction(base::UserMetricsAction("ProcessSwapBindingsMismatch_RVHM"));
+ delete new_web_ui;
+ return nullptr;
}
+ return new_web_ui;
}
RenderFrameHostImpl* RenderFrameHostManager::Navigate(
@@ -557,6 +559,30 @@ void RenderFrameHostManager::SwapOutOldFrame(
}
}
+void RenderFrameHostManager::DiscardRenderFrameHost(
+ scoped_ptr<RenderFrameHostImpl> render_frame_host) {
+ // TODO(carlosk): this code is very similar to what can be found in
+ // SwapOutOldFrame and we should see that these are unified at some point.
+
+ // If the SiteInstance for the pending RFH is being used by others, don't
+ // delete the RFH, just swap it out and it can be reused at a later point.
+ SiteInstanceImpl* site_instance = render_frame_host->GetSiteInstance();
+ if (site_instance->HasSite() && site_instance->active_frame_count() > 1) {
+ // Any currently suspended navigations are no longer needed.
+ render_frame_host->CancelSuspendedNavigations();
+
+ RenderFrameProxyHost* proxy =
+ new RenderFrameProxyHost(site_instance, frame_tree_node_);
+ proxy_hosts_[site_instance->GetId()] = proxy;
+ render_frame_host->SwapOut(proxy);
+ if (frame_tree_node_->IsMainFrame())
+ proxy->TakeFrameHostOwnership(render_frame_host.Pass());
+ } else {
+ // We won't be coming back, so delete this one.
+ render_frame_host.reset();
+ }
+}
+
void RenderFrameHostManager::MoveToPendingDeleteHosts(
scoped_ptr<RenderFrameHostImpl> render_frame_host) {
// |render_frame_host| will be deleted when its SwapOut ACK is received, or
@@ -997,9 +1023,24 @@ void RenderFrameHostManager::CreateRenderFrameHostForNewSiteInstance(
int create_render_frame_flags = 0;
if (is_main_frame)
create_render_frame_flags |= CREATE_RF_FOR_MAIN_FRAME_NAVIGATION;
- // Ensure that we have created RFHs for the new RFH's opener chain if
- // we are staying in the same BrowsingInstance. This allows the new RFH
- // to send cross-process script calls to its opener(s).
+ if (delegate_->IsHidden())
nasko 2014/11/24 17:00:27 nit: empty line before the if statement
carlosk 2014/11/24 18:15:29 Done.
+ create_render_frame_flags |= CREATE_RF_HIDDEN;
+ int opener_route_id =
nasko 2014/11/24 17:00:28 nit: Same here, empty line above it. Unrelated sta
carlosk 2014/11/24 18:15:29 Done.
+ CreateOpenerRenderViewsIfNeeded(old_instance, new_instance);
+ // TODO(carlosk): does this "earlier" call for CancelPending affects anything?
+ // It used to happen inside the creation method iff the new RFH was
+ // successfully created.
+ if (pending_render_frame_host_)
+ CancelPending();
+ // Create a non-swapped-out RFH with the given opener.
+ pending_render_frame_host_ =
+ CreateRenderFrame(new_instance, opener_route_id,
+ create_render_frame_flags, pending_web_ui(), nullptr);
+}
+
+int RenderFrameHostManager::CreateOpenerRenderViewsIfNeeded(
+ SiteInstance* old_instance,
+ SiteInstance* new_instance) {
int opener_route_id = MSG_ROUTING_NONE;
if (new_instance->IsRelatedSiteInstance(old_instance)) {
opener_route_id =
@@ -1012,17 +1053,7 @@ void RenderFrameHostManager::CreateRenderFrameHostForNewSiteInstance(
frame_tree_node_, new_instance);
}
}
-
- if (delegate_->IsHidden())
- create_render_frame_flags |= CREATE_RF_HIDDEN;
-
- // Create a non-swapped-out RFH with the given opener.
- int route_id = CreateRenderFrame(new_instance, opener_route_id,
- create_render_frame_flags);
- if (route_id == MSG_ROUTING_NONE) {
- pending_render_frame_host_.reset();
- return;
- }
+ return opener_route_id;
}
scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrameHost(
@@ -1056,9 +1087,12 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrameHost(
return render_frame_host.Pass();
}
-int RenderFrameHostManager::CreateRenderFrame(SiteInstance* instance,
- int opener_route_id,
- int flags) {
+scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
+ SiteInstance* instance,
+ int opener_route_id,
+ int flags,
+ const WebUIImpl* web_ui,
+ int* routing_id_ptr) {
nasko 2014/11/24 17:00:27 Why take in this as param? The callers can use the
carlosk 2014/11/24 18:15:29 Because if the RFH is flagged to be created swappe
bool swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT);
CHECK(instance);
// Swapped out views should always be hidden.
@@ -1071,20 +1105,21 @@ int RenderFrameHostManager::CreateRenderFrame(SiteInstance* instance,
}
scoped_ptr<RenderFrameHostImpl> new_render_frame_host;
- RenderFrameHostImpl* frame_to_announce = NULL;
- int routing_id = MSG_ROUTING_NONE;
+ bool success = true;
+ if (routing_id_ptr)
nasko 2014/11/24 17:00:27 Not passing in this pointer as argument will also
carlosk 2014/11/24 18:15:29 I don't think that's possible as explained above.
+ *routing_id_ptr = MSG_ROUTING_NONE;
// We are creating a pending or swapped out RFH here. We should never create
- // it in the same SiteInstance as our current RFH.
+ // never create it in the same SiteInstance as our current RFH.
nasko 2014/11/24 17:00:27 nit: "never create" appears twice.
carlosk 2014/11/24 18:15:29 Done.
CHECK_NE(render_frame_host_->GetSiteInstance(), instance);
// Check if we've already created an RFH for this SiteInstance. If so, try
// to re-use the existing one, which has already been initialized. We'll
// remove it from the list of proxy hosts below if it will be active.
RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
-
if (proxy && proxy->render_frame_host()) {
- routing_id = proxy->GetRenderViewHost()->GetRoutingID();
+ if (routing_id_ptr)
+ *routing_id_ptr = proxy->GetRenderViewHost()->GetRoutingID();
// Delete the existing RenderFrameProxyHost, but reuse the RenderFrameHost.
// Prevent the process from exiting while we're trying to use it.
if (!swapped_out) {
@@ -1098,13 +1133,13 @@ int RenderFrameHostManager::CreateRenderFrame(SiteInstance* instance,
// gets a RenderViewHost in the SiteInstance of its opener WebContents.
// If not used in the first navigation, this RVH is swapped out and is not
// granted bindings, so we may need to grant them when swapping it in.
- if (pending_web_ui() &&
- !new_render_frame_host->GetProcess()->IsIsolatedGuest()) {
- int required_bindings = pending_web_ui()->GetBindings();
- RenderViewHost* rvh = new_render_frame_host->render_view_host();
- if ((rvh->GetEnabledBindings() & required_bindings) !=
- required_bindings) {
- rvh->AllowBindings(required_bindings);
+ if (web_ui && !new_render_frame_host->GetProcess()->IsIsolatedGuest()) {
+ int required_bindings = web_ui->GetBindings();
+ RenderViewHost* render_view_host =
+ new_render_frame_host->render_view_host();
+ if ((render_view_host->GetEnabledBindings() & required_bindings) !=
+ required_bindings) {
+ render_view_host->AllowBindings(required_bindings);
}
}
}
@@ -1129,7 +1164,7 @@ int RenderFrameHostManager::CreateRenderFrame(SiteInstance* instance,
proxy->TakeFrameHostOwnership(new_render_frame_host.Pass());
}
- bool success =
+ success =
InitRenderView(render_view_host, opener_route_id, proxy_routing_id,
!!(flags & CREATE_RF_FOR_MAIN_FRAME_NAVIGATION));
if (success) {
@@ -1141,22 +1176,27 @@ int RenderFrameHostManager::CreateRenderFrame(SiteInstance* instance,
DCHECK(new_render_frame_host.get());
success = InitRenderFrame(new_render_frame_host.get());
}
- } else if (!swapped_out && pending_render_frame_host_) {
- CancelPending();
+ if (success) {
+ if (routing_id_ptr)
+ *routing_id_ptr = render_view_host->GetRoutingID();
+ // If a brand new RFH was created, announce it to observers.
+ // TODO(carlosk): verify there's no problem that now this RFH will only
+ // be set as the pending one *after* this delegate call (used to be
+ // before).
+ if (new_render_frame_host) {
+ render_frame_delegate_->RenderFrameCreated(
+ new_render_frame_host.get());
+ }
+ }
}
- routing_id = render_view_host->GetRoutingID();
- frame_to_announce = new_render_frame_host.get();
}
- // Use this as our new pending RFH if it isn't swapped out.
- if (!swapped_out)
- pending_render_frame_host_ = new_render_frame_host.Pass();
-
- // If a brand new RFH was created, announce it to observers.
- if (frame_to_announce)
- render_frame_delegate_->RenderFrameCreated(frame_to_announce);
-
- return routing_id;
+ // Returns the new RFH if it isn't swapped out.
+ if (success && !swapped_out) {
+ DCHECK(new_render_frame_host->GetSiteInstance() == instance);
+ return new_render_frame_host.Pass();
+ }
+ return nullptr;
nasko 2014/11/24 17:00:27 Is this a valid return value for a scoped_ptr?
carlosk 2014/11/24 18:15:29 I was also in doubt but: a) the code compiled with
}
int RenderFrameHostManager::CreateRenderFrameProxy(SiteInstance* instance) {
@@ -1558,24 +1598,7 @@ void RenderFrameHostManager::CancelPending() {
// We no longer need to prevent the process from exiting.
pending_render_frame_host->GetProcess()->RemovePendingView();
- // If the SiteInstance for the pending RFH is being used by others, don't
- // delete the RFH, just swap it out and it can be reused at a later point.
- SiteInstanceImpl* site_instance =
- pending_render_frame_host->GetSiteInstance();
- if (site_instance->active_frame_count() > 1) {
- // Any currently suspended navigations are no longer needed.
- pending_render_frame_host->CancelSuspendedNavigations();
-
- RenderFrameProxyHost* proxy =
- new RenderFrameProxyHost(site_instance, frame_tree_node_);
- proxy_hosts_[site_instance->GetId()] = proxy;
- pending_render_frame_host->SwapOut(proxy);
- if (frame_tree_node_->IsMainFrame())
- proxy->TakeFrameHostOwnership(pending_render_frame_host.Pass());
- } else {
- // We won't be coming back, so delete this one.
- pending_render_frame_host.reset();
- }
+ DiscardRenderFrameHost(pending_render_frame_host.Pass());
pending_web_ui_.reset();
pending_and_current_web_ui_.reset();

Powered by Google App Engine
This is Rietveld 408576698