Convert main_render_frame_ to raw pointer in RenderViewImpl.

The main RenderFrame object for a page is currently owned directly
through a scoped_ptr by RenderViewImpl. Since we want the main
RenderFrame to be deleted when we do a cross-process navigation,
it needs to be cleaned by as part of the regular callbacks from
Blink - WebFrameClient::frameDetached. This requires the frame to not
be owned by RenderViewImpl, so this CL changes it from scoped_ptr
to raw pointer.

The only change in behavior after this CL is that the RenderFrame
will be destroyed prior to RenderView being destroyed as part of
WebView::close(). The destruction of both objects still stays in
the same task on the event loop, though.

BUG=357747

Review URL: https://codereview.chromium.org/1130233002

Cr-Commit-Position: refs/heads/master@{#329337}

content/renderer/render_frame_impl.cc

@@ -730,12 +730,18 @@ RenderFrameImpl::~RenderFrameImpl() {
     render_view_->UnregisterVideoHoleFrame(this);
 #endif
 
-  // RenderFrameProxy is only "owned" by RenderFrameImpl in the case it is the
-  // main frame. Ensure it is deleted along with this object.
-  if (!is_subframe_ && render_frame_proxy_) {
-    // The following method calls back into this object and clears
-    // |render_frame_proxy_|.
-    render_frame_proxy_->frameDetached();
+  if (!is_subframe_) {
+    // 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();
+    }
+
+    // Ensure the RenderView doesn't point to this object, once it is destroyed.
+    CHECK_EQ(render_view_->main_render_frame_, this);
+    render_view_->main_render_frame_ = nullptr;
   }
 
   render_view_->UnregisterRenderFrame(this);
@@ -2231,10 +2237,8 @@ void RenderFrameImpl::frameDetached(blink::WebFrame* frame) {
   frame->close();
   frame_ = nullptr;
 
-  if (is_subframe_) {
-    delete this;
-    // Object is invalid after this point.
-  }
+  delete this;
+  // Object is invalid after this point.
 }
 
 void RenderFrameImpl::frameFocused() {
@@ -2643,6 +2647,13 @@ void RenderFrameImpl::didCommitProvisionalLoad(
     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;
+    }
   }
 
   // When we perform a new navigation, we need to update the last committed

content/renderer/render_view_browsertest.cc

@@ -367,8 +367,9 @@ class DevToolsAgentTest : public RenderViewImplTest {
   }
 };
 
-// Test for https://crbug.com/461191.
-TEST_F(RenderViewImplTest, RenderFrameMessageAfterDetach) {
+// Ensure that the main RenderFrame is deleted and cleared from the RenderView
+// after closing it.
+TEST_F(RenderViewImplTest, RenderFrameClearedAfterClose) {
   // Create a new main frame RenderFrame so that we don't interfere with the
   // shutdown of frame() in RenderViewTest.TearDown.
   blink::WebURLRequest popup_request(GURL("http://foo.com"));
@@ -376,17 +377,10 @@ TEST_F(RenderViewImplTest, RenderFrameMessageAfterDetach) {
       GetMainFrame(), popup_request, blink::WebWindowFeatures(), "foo",
       blink::WebNavigationPolicyNewForegroundTab, false);
   RenderViewImpl* new_view = RenderViewImpl::FromWebView(new_web_view);
-  RenderFrameImpl* new_frame =
-      static_cast<RenderFrameImpl*>(new_view->GetMainRenderFrame());
 
-  // Detach the main frame.
+  // Close the view, causing the main RenderFrame to be detached and deleted.
   new_view->Close();
-
-  // Before the frame is asynchronously deleted, it may receive a message.
-  // We should not crash here, and the message should not be processed.
-  scoped_ptr<const IPC::Message> msg(
-      new FrameMsg_Stop(frame()->GetRoutingID()));
-  EXPECT_FALSE(new_frame->OnMessageReceived(*msg));
+  EXPECT_FALSE(new_view->GetMainRenderFrame());
 
   // Clean up after the new view so we don't leak it.
   new_view->Release();
@@ -2014,7 +2008,7 @@ TEST_F(RenderViewImplTest, NavigateSubframe) {
 // This test ensures that a RenderFrame object is created for the top level
 // frame in the RenderView.
 TEST_F(RenderViewImplTest, BasicRenderFrame) {
-  EXPECT_TRUE(view()->main_render_frame_.get());
+  EXPECT_TRUE(view()->main_render_frame_);
 }
 
 TEST_F(RenderViewImplTest, GetSSLStatusOfFrame) {

content/renderer/render_view_impl.cc

@@ -669,11 +669,11 @@ void RenderViewImpl::Initialize(const ViewMsg_New_Params& params,
   // Ensure we start with a valid next_page_id_ from the browser.
   DCHECK_GE(next_page_id_, 0);
 
-  main_render_frame_.reset(RenderFrameImpl::Create(
-      this, params.main_frame_routing_id));
+  main_render_frame_ = RenderFrameImpl::Create(
+      this, params.main_frame_routing_id);
   // The main frame WebLocalFrame object is closed by
   // RenderFrameImpl::frameDetached().
-  WebLocalFrame* web_frame = WebLocalFrame::create(main_render_frame_.get());
+  WebLocalFrame* web_frame = WebLocalFrame::create(main_render_frame_);
   main_render_frame_->SetWebFrame(web_frame);
 
   compositor_deps_ = compositor_deps;
@@ -734,7 +734,7 @@ void RenderViewImpl::Initialize(const ViewMsg_New_Params& params,
   if (params.proxy_routing_id != MSG_ROUTING_NONE) {
     CHECK(params.swapped_out);
     proxy = RenderFrameProxy::CreateProxyToReplaceFrame(
-        main_render_frame_.get(), params.proxy_routing_id);
+        main_render_frame_, params.proxy_routing_id);
     main_render_frame_->set_render_frame_proxy(proxy);
   }
 
@@ -2170,7 +2170,7 @@ bool RenderViewImpl::Send(IPC::Message* message) {
 }
 
 RenderFrameImpl* RenderViewImpl::GetMainRenderFrame() {
-  return main_render_frame_.get();
+  return main_render_frame_;
 }
 
 int RenderViewImpl::GetRoutingID() const {
@@ -3492,7 +3492,7 @@ blink::WebPageVisibilityState RenderViewImpl::visibilityState() const {
   blink::WebPageVisibilityState override_state = current_state;
   // TODO(jam): move this method to WebFrameClient.
   if (GetContentClient()->renderer()->
-          ShouldOverridePageVisibilityState(main_render_frame_.get(),
+          ShouldOverridePageVisibilityState(main_render_frame_,
                                             &override_state))
     return override_state;
   return current_state;

content/renderer/render_view_impl.h

@@ -557,6 +557,7 @@ class CONTENT_EXPORT RenderViewImpl
   FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest,
                            MessageOrderInDidChangeSelection);
   FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, SendCandidateWindowEvents);
+  FRIEND_TEST_ALL_PREFIXES(RenderViewImplTest, RenderFrameClearedAfterClose);
   FRIEND_TEST_ALL_PREFIXES(SuppressErrorPageTest, Suppresses);
   FRIEND_TEST_ALL_PREFIXES(SuppressErrorPageTest, DoesNotSuppress);
 
@@ -908,7 +909,7 @@ class CONTENT_EXPORT RenderViewImpl
 
   // Helper objects ------------------------------------------------------------
 
-  scoped_ptr<RenderFrameImpl> main_render_frame_;
+  RenderFrameImpl* main_render_frame_;
 
   // The next group of objects all implement RenderViewObserver, so are deleted
   // along with the RenderView automatically.  This is why we just store