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

Unified Diff: content/renderer/render_frame_impl.cc

Issue 1208143002: Move existing kSitePerProcess checks to a policy-oracle object (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@swapped_out_cmdline_checks
Patch Set: Partial fixes to Nasko's comments. Created 5 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/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 2e7d77a987884907027c9592b092779f2be946ff..e08464021b7a4d3cec6164e304400fffbd30a060 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -50,6 +50,7 @@
#include "content/public/common/isolated_world_ids.h"
#include "content/public/common/page_state.h"
#include "content/public/common/resource_response.h"
+#include "content/public/common/site_isolation_policy.h"
#include "content/public/common/url_constants.h"
#include "content/public/common/url_utils.h"
#include "content/public/renderer/browser_plugin_delegate.h"
@@ -509,11 +510,6 @@ bool IsReload(FrameMsg_Navigate_Type::Value navigation_type) {
navigation_type == FrameMsg_Navigate_Type::RELOAD_ORIGINAL_REQUEST_URL;
}
-bool IsSwappedOutStateForbidden() {
- return base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess);
-}
-
RenderFrameImpl::CreateRenderFrameImplFunction g_create_render_frame_impl =
nullptr;
@@ -588,8 +584,7 @@ void RenderFrameImpl::CreateFrame(
CHECK_IMPLIES(parent_routing_id == MSG_ROUTING_NONE, !web_frame->parent());
if (widget_params.routing_id != MSG_ROUTING_NONE) {
- CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess));
+ CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible());
render_frame->render_widget_ = RenderWidget::CreateForFrame(
widget_params.routing_id, widget_params.surface_id,
widget_params.hidden, render_frame->render_view_->screen_info(),
@@ -696,18 +691,15 @@ RenderFrameImpl::~RenderFrameImpl() {
#endif
if (!is_subframe_) {
- if (!IsSwappedOutStateForbidden()) {
- // When using swapped out frames, RenderFrameProxy is "owned" by
- // RenderFrameImpl in the case it is the main frame. Ensure it is deleted
- // along with this object.
- if (render_frame_proxy_ &&
- !base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess)) {
- // The following method calls back into this object and clears
- // |render_frame_proxy_|.
- render_frame_proxy_->frameDetached(
- blink::WebRemoteFrameClient::DetachType::Remove);
- }
+ // When using swapped out frames, RenderFrameProxy is owned by
+ // RenderFrameImpl in the case it is the main frame. Ensure it is deleted
+ // along with this object.
+ if (render_frame_proxy_ &&
+ !RenderFrameProxy::IsSwappedOutStateForbidden()) {
+ // The following method calls back into this object and clears
+ // |render_frame_proxy_|.
+ render_frame_proxy_->frameDetached(
+ blink::WebRemoteFrameClient::DetachType::Remove);
}
// Ensure the RenderView doesn't point to this object, once it is destroyed.
@@ -1121,12 +1113,12 @@ void RenderFrameImpl::OnSwapOut(
const FrameReplicationState& replicated_frame_state) {
TRACE_EVENT1("navigation", "RenderFrameImpl::OnSwapOut", "id", routing_id_);
RenderFrameProxy* proxy = NULL;
- bool is_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess);
+ bool swapped_out_forbidden = RenderFrameProxy::IsSwappedOutStateForbidden();
bool is_main_frame = !frame_->parent();
// This codepath should only be hit for subframes when in --site-per-process.
- CHECK_IMPLIES(!is_main_frame, is_site_per_process);
+ CHECK_IMPLIES(!is_main_frame,
+ SiteIsolationPolicy::AreCrossProcessFramesPossible());
// Only run unload if we're not swapped out yet, but send the ack either way.
if (!is_swapped_out_) {
@@ -1168,7 +1160,7 @@ void RenderFrameImpl::OnSwapOut(
// TODO(creis): Should we be stopping all frames here and using
// StopAltErrorPageFetcher with RenderView::OnStop, or just stopping this
// frame?
- if (!IsSwappedOutStateForbidden())
+ if (!swapped_out_forbidden)
OnStop();
// Transfer settings such as initial drawing parameters to the remote frame,
@@ -1180,7 +1172,7 @@ void RenderFrameImpl::OnSwapOut(
// run a second time, thanks to a check in FrameLoader::stopLoading.
// TODO(creis): Need to add a better way to do this that avoids running the
// beforeunload handler. For now, we just run it a second time silently.
- if (!IsSwappedOutStateForbidden())
+ if (!swapped_out_forbidden)
NavigateToSwappedOutURL();
// Let WebKit know that this view is hidden so it can drop resources and
@@ -1203,7 +1195,7 @@ void RenderFrameImpl::OnSwapOut(
// Now that all of the cleanup is complete and the browser side is notified,
// start using the RenderFrameProxy, if one is created.
- if (proxy && IsSwappedOutStateForbidden()) {
+ if (proxy && swapped_out_forbidden) {
frame_->swap(proxy->web_frame());
if (is_loading)
@@ -1218,7 +1210,7 @@ void RenderFrameImpl::OnSwapOut(
// in proxy->web_frame(), the RemoteFrame will not exist for main frames.
// When we do an unconditional swap for all frames, we can remove
// !is_main_frame below.
- if (proxy && IsSwappedOutStateForbidden())
+ if (proxy && swapped_out_forbidden)
proxy->SetReplicatedState(replicated_frame_state);
// Safe to exit if no one else is using the process.
@@ -2186,8 +2178,8 @@ void RenderFrameImpl::frameDetached(blink::WebFrame* frame, DetachType type) {
g_frame_map.Get().erase(it);
if (is_subframe_) {
- if (base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess) && render_widget_) {
+ if (render_widget_) {
+ CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible());
render_widget_->UnregisterRenderFrame(this);
}
@@ -2232,9 +2224,7 @@ void RenderFrameImpl::didChangeName(blink::WebLocalFrame* frame,
// can be optimized further by only sending the update if there are any
// remote frames in the frame tree, or delaying and batching up IPCs if
// updates are happening too frequently.
- bool is_site_per_process = base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess);
- if (is_site_per_process ||
+ if (SiteIsolationPolicy::AreCrossProcessFramesPossible() ||
render_view_->renderer_preferences_.report_frame_name_changes) {
Send(new FrameHostMsg_DidChangeName(routing_id_, base::UTF16ToUTF8(name)));
}
@@ -2504,9 +2494,8 @@ void RenderFrameImpl::didStartProvisionalLoad(blink::WebLocalFrame* frame,
DocumentState* document_state = DocumentState::FromDataSource(ds);
// We should only navigate to swappedout:// when is_swapped_out_ is true.
- CHECK((ds->request().url() != GURL(kSwappedOutURL)) ||
- is_swapped_out_) <<
- "Heard swappedout:// when not swapped out.";
+ CHECK_IMPLIES(ds->request().url() == GURL(kSwappedOutURL), is_swapped_out_)
+ << "Heard swappedout:// when not swapped out.";
// Update the request time if WebKit has better knowledge of it.
if (document_state->request_time().is_null() &&
@@ -3903,8 +3892,7 @@ void RenderFrameImpl::SendDidCommitProvisionalLoad(
// Make navigation state a part of the DidCommitProvisionalLoad message so
// that committed entry has it at all times.
HistoryEntry* entry = render_view_->history_controller()->GetCurrentEntry();
- if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess)) {
+ if (!SiteIsolationPolicy::UseSubframeNavigationEntries()) {
if (entry)
params.page_state = HistoryEntryToPageState(entry);
else
@@ -4160,10 +4148,9 @@ WebNavigationPolicy RenderFrameImpl::DecidePolicyForNavigation(
Referrer referrer(RenderViewImpl::GetReferrerFromRequest(info.frame,
info.urlRequest));
- const base::CommandLine& command_line =
- *base::CommandLine::ForCurrentProcess();
- if (command_line.HasSwitch(switches::kSitePerProcess) && is_subframe_) {
+ // TODO(nick): Is consulting |is_subframe_| here correct?
+ if (RenderFrameProxy::IsSwappedOutStateForbidden() && is_subframe_) {
// There's no reason to ignore navigations on subframes, since the swap out
// logic no longer applies.
} else {
@@ -4467,8 +4454,7 @@ void RenderFrameImpl::NavigateInternal(
if (!browser_side_navigation) {
scoped_ptr<NavigationParams> navigation_params(
new NavigationParams(*pending_navigation_params_.get()));
- if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
- switches::kSitePerProcess)) {
+ if (!SiteIsolationPolicy::UseSubframeNavigationEntries()) {
// By default, tell the HistoryController to go the deserialized
// HistoryEntry. This only works if all frames are in the same
// process.

Powered by Google App Engine
This is Rietveld 408576698