Remove swapped-out usage in --site-per-process.

content/renderer/render_frame_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 519 matching lines @@
 
 // static
 void RenderFrameImpl::CreateFrame(
     int routing_id,
     int parent_routing_id,
     int previous_sibling_routing_id,
     int proxy_routing_id,
     const FrameReplicationState& replicated_state,
     CompositorDependencies* compositor_deps,
     const FrameMsg_NewFrame_WidgetParams& widget_params) {
-  // TODO(nasko): For now, this message is only sent for subframes, as the
-  // top level frame is created when the RenderView is created through the
-  // ViewMsg_New IPC.
-  CHECK_NE(MSG_ROUTING_NONE, parent_routing_id);
-
   blink::WebLocalFrame* web_frame;
   RenderFrameImpl* render_frame;
   if (proxy_routing_id == MSG_ROUTING_NONE) {
     RenderFrameProxy* parent_proxy =
         RenderFrameProxy::FromRoutingID(parent_routing_id);
     // If the browser is sending a valid parent routing id, it should already
     // be created and registered.
     CHECK(parent_proxy);
     blink::WebRemoteFrame* parent_web_frame = parent_proxy->web_frame();
 
@@ skipping 16 matching lines @@
     CHECK(proxy);
     render_frame = RenderFrameImpl::Create(proxy->render_view(), routing_id);
     web_frame =
         blink::WebLocalFrame::create(replicated_state.scope, render_frame);
     render_frame->proxy_routing_id_ = proxy_routing_id;
     web_frame->initializeToReplaceRemoteFrame(
         proxy->web_frame(), WebString::fromUTF8(replicated_state.name),
         replicated_state.sandbox_flags);
   }
   render_frame->SetWebFrame(web_frame);
+  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));
     render_frame->render_widget_ = RenderWidget::CreateForFrame(
         widget_params.routing_id, widget_params.surface_id,
         widget_params.hidden, render_frame->render_view_->screen_info(),
         compositor_deps, web_frame);
     // TODO(kenrb): Observing shouldn't be necessary when we sort out
     // WasShown and WasHidden, separating page-level visibility from
@@ skipping 102 matching lines @@
     // 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_) {
       // 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.
-    CHECK_EQ(render_view_->main_render_frame_, this);
+    // TODO(nasko): Restore this check once we don't leak this object on
+    // frame swap.
+    //CHECK_EQ(render_view_->main_render_frame_, this);

[Charlie Reis, 2015/06/04 22:27:31]

We don't keep commented out code checked in.  Let's remove this line and change
the TODO to say "Add a CHECK that the main_render_frame_ is sane once we don't
leak this object on frame swap."

[nasko, 2015/06/04 23:38:36]

Done.

     render_view_->main_render_frame_ = nullptr;
   }
 
   render_view_->UnregisterRenderFrame(this);
   g_routing_id_frame_map.Get().erase(routing_id_);
   RenderThread::Get()->RemoveRoute(routing_id_);
 }
 
 void RenderFrameImpl::SetWebFrame(blink::WebLocalFrame* web_frame) {
   DCHECK(!frame_);
@@ skipping 431 matching lines @@
     // willCheckAndDispatchMessageEvent() that needs the proxy.
     if (proxy)
       set_render_frame_proxy(proxy);
 
     // Now that we're swapped out and filtering IPC messages, stop loading to
     // ensure that no other in-progress navigation continues.  We do this here
     // to avoid sending a DidStopLoading message to the browser process.
     // TODO(creis): Should we be stopping all frames here and using
     // StopAltErrorPageFetcher with RenderView::OnStop, or just stopping this
     // frame?
-    OnStop();
+    if (!is_site_per_process)
+      OnStop();
 
     // Transfer settings such as initial drawing parameters to the remote frame,
     // if one is created, that will replace this frame.
     if (!is_main_frame && proxy)
       proxy->web_frame()->initializeFromFrame(frame_);
 
     // Replace the page with a blank dummy URL. The unload handler will not be
     // 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 (!is_site_per_process || is_main_frame)
+    if (!is_site_per_process)
       NavigateToSwappedOutURL();
 
     // Let WebKit know that this view is hidden so it can drop resources and
     // stop compositing.
     // TODO(creis): Support this for subframes as well.
     if (is_main_frame) {
       render_view_->webview()->setVisibilityState(
           blink::WebPageVisibilityStateHidden, false);
     }
   }
 
   // It is now safe to show modal dialogs again.
   // TODO(creis): Deal with modal dialogs from subframes.
   if (is_main_frame)
     render_view_->suppress_dialogs_until_swap_out_ = false;
 
   Send(new FrameHostMsg_SwapOut_ACK(routing_id_));
 
+  RenderViewImpl* render_view = render_view_.get();
+
   // Now that all of the cleanup is complete and the browser side is notified,
   // start using the RenderFrameProxy, if one is created.
   if (proxy) {
-    if (!is_main_frame) {
+    if (is_site_per_process || !is_main_frame) {
       frame_->swap(proxy->web_frame());
 
       if (is_loading)
         proxy->OnDidStartLoading();
-
-      if (is_site_per_process) {
-        // TODO(nasko): delete the frame here, since we've replaced it with a
-        // proxy.
-      }
     }
   }
 
   // In --site-per-process, initialize the WebRemoteFrame with the replication
   // state passed by the process that is now rendering the frame.
   // TODO(alexmos): We cannot yet do this for swapped-out main frames, because
   // in that case we leave the LocalFrame as the main frame visible to Blink
   // and don't call swap() above. Because swap() is what creates a RemoteFrame
   // 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 (is_site_per_process && proxy && !is_main_frame)
+  if (is_site_per_process && proxy)
     proxy->SetReplicatedState(replicated_frame_state);
 
   // Safe to exit if no one else is using the process.
-  if (is_main_frame)
-    render_view_->WasSwappedOut();
+  // TODO(nasko): Remove the dependency on RenderViewImpl here and ref count
+  // the process based on the lifetime of this RenderFrameImpl object.
+  if (is_main_frame) {
+    render_view->WasSwappedOut();
+
+    // TODO(nasko): Currently, this RenderFrame is leaked due to issues in
+    // Blink, therefore the destructor won't be invoked to destroy this
+    // object. In the meantime, set the main frame of the RenderView to null
+    // here.
+    if (is_site_per_process) {
+      CHECK_EQ(render_view_->main_render_frame_, this);
+      render_view->main_render_frame_ = nullptr;
+    }
+  }
 }
 
 void RenderFrameImpl::OnContextMenuClosed(
     const CustomContextMenuContext& custom_context) {
   if (custom_context.request_id) {
     // External request, should be in our map.
     ContextMenuClient* client =
         pending_context_menus_.Lookup(custom_context.request_id);
     if (client) {
       client->OnMenuClosed(custom_context.request_id);
@@ skipping 1361 matching lines @@
       static_cast<NavigationStateImpl*>(document_state->navigation_state());
 
   if (proxy_routing_id_ != MSG_ROUTING_NONE) {
     RenderFrameProxy* proxy =
         RenderFrameProxy::FromRoutingID(proxy_routing_id_);
     CHECK(proxy);
     proxy->web_frame()->swap(frame_);
     proxy_routing_id_ = MSG_ROUTING_NONE;
 
     // If this is the main frame going from a remote frame to a local frame,
-    // it needs to set RenderViewImpl's pointer for the main frame to itself.
+    // it needs to set RenderViewImpl's pointer for the main frame to itself
+    // and ensure RenderWidget is no longer in swapped out mode.
     if (!is_subframe_) {
       CHECK(!render_view_->main_render_frame_);
       render_view_->main_render_frame_ = this;
+      if (render_view_->is_swapped_out())
+        render_view_->SetSwappedOut(false);
     }
   }
 
   // When we perform a new navigation, we need to update the last committed
   // session history entry with state for the page we are leaving. Do this
   // before updating the HistoryController state.
   render_view_->UpdateSessionHistory(frame);
 
   render_view_->history_controller()->UpdateForCommit(
       this, item, commit_type, navigation_state->WasWithinSamePage());
@@ skipping 1748 matching lines @@
   }
 
   Send(new FrameHostMsg_OpenURL(routing_id_, params));
 }
 
 void RenderFrameImpl::NavigateInternal(
     const CommonNavigationParams& common_params,
     const StartNavigationParams& start_params,
     const RequestNavigationParams& request_params,
     scoped_ptr<StreamOverrideParameters> stream_params) {
+  if (proxy_routing_id_ != MSG_ROUTING_NONE) {
+    RenderFrameProxy* proxy =
+        RenderFrameProxy::FromRoutingID(proxy_routing_id_);
+    CHECK(proxy);
+    proxy->web_frame()->swap(frame_);
+    proxy_routing_id_ = MSG_ROUTING_NONE;
+
+    // If this is the main frame going from a remote frame to a local frame,
+    // it needs to set RenderViewImpl's pointer for the main frame to itself.
+    if (!is_subframe_) {
+      CHECK(!render_view_->main_render_frame_);
+      render_view_->main_render_frame_ = this;
+    }
+  }
+
   bool browser_side_navigation =
       base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableBrowserSideNavigation);
   bool is_reload = IsReload(common_params.navigation_type);
   bool is_history_navigation = request_params.page_state.IsValid();
   WebURLRequest::CachePolicy cache_policy =
       WebURLRequest::UseProtocolCachePolicy;
   if (!RenderFrameImpl::PrepareRenderViewForNavigation(
           common_params.url, is_history_navigation, request_params, &is_reload,
           &cache_policy)) {
@@ skipping 535 matching lines @@
 #elif defined(ENABLE_BROWSER_CDMS)
         cdm_manager_,
 #endif
         this);
   }
 
   return cdm_factory_;
 }
 
 }  // namespace content