Remove redundant |swapped_out| from mojom::content::CreateViewParams

The field is true when there is no routing id for the main frame (ie
there is a proxy main frame), and false otherwise. So we can just use
that.

In the browser side it is actually a separate state, is_active_ on the
RenderViewHostImpl. However it is always set to true/false when the
main frame routing id is set as well. So we remove it and have
is_active() report if the main frame routing id is present instead.

R=alexmos@chromium.org
TBR=dcheng

Change-Id: I87c89f9d06ec99e769528e164e0ea0ed3d6fc6bc
Bug: 419087
Reviewed-on: https://chromium-review.googlesource.com/c/1336210
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608799}

content/browser/frame_host/render_frame_host_impl.cc

@@ -2259,9 +2259,6 @@ void RenderFrameHostImpl::SwapOut(
 
   if (web_ui())
     web_ui()->RenderFrameHostSwappingOut();
-
-  if (frame_tree_node_->IsMainFrame())
-    render_view_host_->SetIsActive(false);
 }
 
 void RenderFrameHostImpl::OnBeforeUnloadACK(

content/browser/frame_host/render_frame_host_manager.cc

@@ -430,8 +430,7 @@ void RenderFrameHostManager::DiscardUnusedFrame(
   // shortly, since |render_frame_host| is its last active frame and will be
   // deleted below.  See https://crbug.com/627400.
   if (frame_tree_node_->IsMainFrame()) {
-    rvh->set_main_frame_routing_id(MSG_ROUTING_NONE);
-    rvh->SetIsActive(false);
+    rvh->SetMainFrameRoutingId(MSG_ROUTING_NONE);
     rvh->set_is_swapped_out(true);
   }
 
@@ -2233,7 +2232,10 @@ void RenderFrameHostManager::CommitPending() {
   // to MSG_ROUTING_NONE.
   if (is_main_frame) {
     RenderViewHostImpl* rvh = render_frame_host_->render_view_host();
-    rvh->set_main_frame_routing_id(render_frame_host_->routing_id());
+    // Recall if the RenderViewHostImpl had a main frame routing id already. If
+    // not then it is transitioning from swapped out to active.
+    bool was_active = rvh->is_active();
+    rvh->SetMainFrameRoutingId(render_frame_host_->routing_id());
 
     // If the RenderViewHost is transitioning from swapped out to active state,
     // it was reused, so dispatch a RenderViewReady event.  For example, this
@@ -2242,12 +2244,11 @@ void RenderFrameHostManager::CommitPending() {
     //
     // TODO(alexmos):  Remove this and move RenderViewReady consumers to use
     // the main frame's RenderFrameCreated instead.
-    if (!rvh->is_active())
+    if (!was_active)
       rvh->PostRenderViewReady();
 
-    rvh->SetIsActive(true);
     rvh->set_is_swapped_out(false);
-    old_render_frame_host->render_view_host()->set_main_frame_routing_id(
+    old_render_frame_host->render_view_host()->SetMainFrameRoutingId(
         MSG_ROUTING_NONE);
   }
 

content/browser/renderer_host/render_view_host_impl.cc

@@ -214,7 +214,6 @@ RenderViewHostImpl::RenderViewHostImpl(
       frames_ref_count_(0),
       delegate_(delegate),
       instance_(static_cast<SiteInstanceImpl*>(instance)),
-      is_active_(!swapped_out),
       is_swapped_out_(swapped_out),
       routing_id_(routing_id),
       main_frame_routing_id_(main_frame_routing_id),
@@ -245,7 +244,7 @@ RenderViewHostImpl::RenderViewHostImpl(
   // make their way to the new renderer once its restarted.
   GetProcess()->EnableSendQueue();
 
-  if (!is_active_)
+  if (!is_active())
     GetWidget()->UpdatePriority();
 
   base::PostTaskWithTraits(
@@ -345,11 +344,10 @@ bool RenderViewHostImpl::CreateRenderView(
       delegate_->GetSessionStorageNamespace(instance_.get())->id();
   // Ensure the RenderView sets its opener correctly.
   params->opener_frame_route_id = opener_frame_route_id;
-  params->swapped_out = !is_active_;
   params->replicated_frame_state = replicated_frame_state;
   params->proxy_routing_id = proxy_route_id;
-  params->hidden = is_active_ ? GetWidget()->is_hidden()
-                              : GetWidget()->delegate()->IsHidden();
+  params->hidden = is_active() ? GetWidget()->is_hidden()
+                               : GetWidget()->delegate()->IsHidden();
   params->never_visible = delegate_->IsNeverVisible();
   params->window_was_created_with_opener = window_was_created_with_opener;
   if (main_rfh) {
@@ -380,10 +378,8 @@ bool RenderViewHostImpl::CreateRenderView(
   return true;
 }
 
-void RenderViewHostImpl::SetIsActive(bool is_active) {
-  if (is_active_ == is_active)
-    return;
-  is_active_ = is_active;
+void RenderViewHostImpl::SetMainFrameRoutingId(int routing_id) {
+  main_frame_routing_id_ = routing_id;
   GetWidget()->UpdatePriority();
 }
 
@@ -900,11 +896,11 @@ bool RenderViewHostImpl::MayRenderWidgetForwardKeyboardEvent(
 }
 
 bool RenderViewHostImpl::ShouldContributePriorityToProcess() {
-  return is_active_;
+  return is_active();
 }
 
 void RenderViewHostImpl::RequestSetBounds(const gfx::Rect& bounds) {
-  if (is_active_)
+  if (is_active())
     delegate_->RequestSetBounds(bounds);
 }
 

content/browser/renderer_host/render_view_host_impl.h

@@ -138,8 +138,7 @@ class CONTENT_EXPORT RenderViewHostImpl : public RenderViewHost,
   // Tracks whether this RenderViewHost is in an active state (rather than
   // pending swap out or swapped out), according to its main frame
   // RenderFrameHost.
-  bool is_active() const { return is_active_; }
-  void SetIsActive(bool is_active);
+  bool is_active() const { return main_frame_routing_id_ != MSG_ROUTING_NONE; }
 
   // Tracks whether this RenderViewHost is swapped out, according to its main
   // frame RenderFrameHost.
@@ -197,9 +196,9 @@ class CONTENT_EXPORT RenderViewHostImpl : public RenderViewHost,
   // GpuSwitchingObserver implementation.
   void OnGpuSwitched() override;
 
-  void set_main_frame_routing_id(int routing_id) {
-    main_frame_routing_id_ = routing_id;
-  }
+  // Sets the routing id for the main frame. When set to MSG_ROUTING_NONE, the
+  // view is not considered active.
+  void SetMainFrameRoutingId(int routing_id);
 
   // Increases the refcounting on this RVH. This is done by the FrameTree on
   // creation of a RenderFrameHost or RenderFrameProxyHost.
@@ -303,13 +302,8 @@ class CONTENT_EXPORT RenderViewHostImpl : public RenderViewHost,
   // over time.
   scoped_refptr<SiteInstanceImpl> instance_;
 
-  // Tracks whether this RenderViewHost is in an active state.  False if the
-  // main frame is pending swap out, pending deletion, or swapped out, because
-  // it is not visible to the user in any of these cases.
-  bool is_active_;
-
   // Tracks whether the main frame RenderFrameHost is swapped out.  Unlike
-  // is_active_, this is false when the frame is pending swap out or deletion.
+  // is_active(), this is false when the frame is pending swap out or deletion.
   // TODO(creis): Remove this when we no longer filter IPCs after swap out.
   // See https://crbug.com/745091.
   bool is_swapped_out_;

content/common/renderer.mojom

@@ -26,7 +26,8 @@ struct CreateViewParams {
   int32 view_id = IPC.mojom.kRoutingIdNone;
 
   // The ID of the main frame hosted in the view, or None if creating a view to
-  // host a main frame proxy.
+  // host a main frame proxy. If this is None, then |proxy_routing_id| should be
+  // set to something other than None.
   int32 main_frame_routing_id = IPC.mojom.kRoutingIdNone;
 
   // The InterfaceProvider through which the main RenderFrame can access
@@ -46,13 +47,9 @@ struct CreateViewParams {
   // set one (MSG_ROUTING_NONE otherwise).
   int32 opener_frame_route_id = IPC.mojom.kRoutingIdNone;
 
-  // Whether the RenderView should initially be swapped out.
-  bool swapped_out;
-
   // Carries replicated information, such as frame name and sandbox flags, for
-  // this view's main frame, which will be a proxy in |swapped_out|
-  // views when in --site-per-process mode, or a RenderFrame in all other
-  // cases.
+  // this view's main frame, which will be a proxy when |main_frame_routing_id|
+  // is None (due to site isolation), or a RenderFrame in all other cases.
   FrameReplicationState replicated_frame_state;
 
   // Used for devtools instrumentation and trace-ability. The token is
@@ -64,7 +61,8 @@ struct CreateViewParams {
   mojo_base.mojom.UnguessableToken devtools_main_frame_token;
 
   // The ID of the proxy object for the main frame in this view. It is only
-  // used if |swapped_out| is true.
+  // used if |main_frame_routing_id| is None, and exactly one of the two
+  // should be set to an id other than None at any time.
   int32 proxy_routing_id = IPC.mojom.kRoutingIdNone;
 
   // Whether the RenderView should initially be hidden.

content/public/test/render_view_test.cc

@@ -349,7 +349,6 @@ void RenderViewTest::SetUp() {
       mojo::MakeRequest(&view_params->main_frame_interface_provider));
   view_params->session_storage_namespace_id =
       blink::AllocateSessionStorageNamespaceId();
-  view_params->swapped_out = false;
   view_params->replicated_frame_state = FrameReplicationState();
   view_params->proxy_routing_id = MSG_ROUTING_NONE;
   view_params->hidden = false;

content/renderer/render_view_impl.cc

@@ -431,14 +431,15 @@ cc::BrowserControlsState ContentToCc(BrowserControlsState state) {
 
 RenderViewImpl::RenderViewImpl(CompositorDependencies* compositor_deps,
                                const mojom::CreateViewParams& params)
-    : RenderWidget(params.main_frame_widget_routing_id,
-                   compositor_deps,
-                   WidgetType::kFrame,
-                   params.visual_properties.screen_info,
-                   params.visual_properties.display_mode,
-                   params.swapped_out,
-                   params.hidden,
-                   params.never_visible),
+    : RenderWidget(
+          params.main_frame_widget_routing_id,
+          compositor_deps,
+          WidgetType::kFrame,
+          params.visual_properties.screen_info,
+          params.visual_properties.display_mode,
+          /*swapped_out=*/params.main_frame_routing_id == MSG_ROUTING_NONE,
+          params.hidden,
+          params.never_visible),
       routing_id_(params.view_id),
       renderer_wide_named_frame_lookup_(
           params.renderer_wide_named_frame_lookup),
@@ -537,7 +538,6 @@ void RenderViewImpl::Initialize(
         opener_frame, params->devtools_main_frame_token,
         params->replicated_frame_state, params->has_committed_real_load);
   } else {
-    CHECK(params->swapped_out);
     RenderFrameProxy::CreateFrameProxy(params->proxy_routing_id, GetRoutingID(),
                                        opener_frame, MSG_ROUTING_NONE,
                                        params->replicated_frame_state,
@@ -1405,7 +1405,6 @@ WebView* RenderViewImpl::CreateView(
       reply->cloned_session_storage_namespace_id;
   DCHECK(!view_params->session_storage_namespace_id.empty())
       << "Session storage namespace must be populated.";
-  view_params->swapped_out = false;
   view_params->replicated_frame_state.frame_policy.sandbox_flags =
       sandbox_flags;
   view_params->replicated_frame_state.name = frame_name_utf8;

content/renderer/render_view_impl.h

@@ -108,11 +108,12 @@ class CONTENT_EXPORT RenderViewImpl : private RenderWidget,
  public:
   // Creates a new RenderView. Note that if the original opener has been closed,
   // |params.window_was_created_with_opener| will be true and
-  // |params.opener_frame_route_id| will be MSG_ROUTING_NONE. When
-  // |params.swapped_out| is true, |params.proxy_routing_id| is specified, so a
-  // RenderFrameProxy can be created for this RenderView's main RenderFrame. The
-  // opener should provide a non-null value for |show_callback| if it needs to
-  // send an additional IPC to finish making this view visible.
+  // |params.opener_frame_route_id| will be MSG_ROUTING_NONE.
+  // When |params.proxy_routing_id| instead of |params.main_frame_routing_id| is
+  // specified, a RenderFrameProxy will be created for this RenderView's main
+  // RenderFrame.
+  // The opener should provide a non-null value for |show_callback| if it needs
+  // to send an additional IPC to finish making this view visible.
   static RenderViewImpl* Create(
       CompositorDependencies* compositor_deps,
       mojom::CreateViewParamsPtr params,