Don't conditionally try use undead RenderWidget when we know the state

When DetachWebFrameWidget() is called, its is because there was a main
frame, and thus a non-undead RenderWidget. We can mark it undead at
that point if we're not destroying.

When Destroy() finishes, if it did not have a main frame, then it did
have an undead RenderWidget.

R=avi@chromium.org

Bug: 419087
Change-Id: I14a18994a980d0a08a3dd27233fcb0da6871dd27
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848904
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704391}

content/renderer/render_frame_impl.cc

@@ -4456,7 +4456,6 @@ void RenderFrameImpl::FrameDetached(DetachType type) {
     // this "swap out", the pointer is moved off to the side until it is
     // swapped back in. The renderer is then told that the WebFrameWidget is
     // dropped which should remove all reference to this object.
-    render_view_->MakeMainFrameRenderWidgetUndead();
     render_view_->DetachWebFrameWidget();
   } else if (render_widget_) {
     DCHECK(owned_render_widget_);

content/renderer/render_view_impl.cc

@@ -1073,13 +1073,8 @@ void RenderViewImpl::Destroy() {
   // a main frame. So it should not be able to see this happening when there is
   // no local main frame.
   if (close_render_widget_here) {
-    if (undead_render_widget_) {
-      RenderWidget* closing_widget = undead_render_widget_.get();
-      closing_widget->CloseForFrame(std::move(undead_render_widget_));
-    } else {
-      RenderWidget* closing_widget = render_widget_.get();
-      closing_widget->CloseForFrame(std::move(render_widget_));
-    }
+    RenderWidget* closing_widget = undead_render_widget_.get();
+    closing_widget->CloseForFrame(std::move(undead_render_widget_));
   }
 
   delete this;
@@ -1572,30 +1567,29 @@ void RenderViewImpl::DetachWebFrameWidget() {
     // The RenderWidget will be closed, and it will close the WebWidget stored
     // in |frame_widget_|. We just want to drop raw pointer here.
     frame_widget_ = nullptr;
-    if (undead_render_widget_) {
-      RenderWidget* closing_widget = undead_render_widget_.get();
-      closing_widget->CloseForFrame(std::move(undead_render_widget_));
-    } else {
-      RenderWidget* closing_widget = render_widget_.get();
-      closing_widget->CloseForFrame(std::move(render_widget_));
-    }
+
+    // We pass ownership of |render_widget_| to itself. Grab a raw pointer to
+    // call the Close() method on so we don't have to be a C++ expert to know
+    // whether we will end up with a nullptr where we didn't intend due to order
+    // of execution.
+    RenderWidget* closing_widget = render_widget_.get();
+    closing_widget->CloseForFrame(std::move(render_widget_));
   } else {
     // We are not inside RenderViewImpl::Destroy(), the main frame is being
     // detached and replaced with a remote frame proxy. We can't close the
     // RenderWidget, and it is marked undead instead, but we do need to close
     // the WebFrameWidget and remove it from the RenderWidget.
 
-    RenderWidget* render_widget = render_widget_.get();
-    if (!render_widget)
-      render_widget = undead_render_widget_.get();
+    render_widget_->SetIsUndead(true);
 
-    DCHECK(render_widget->IsUndeadOrProvisional());
     // The WebWidget needs to be closed even though the RenderWidget won't be
     // here (since it is marked undead instead).
     frame_widget_->Close();
     frame_widget_ = nullptr;
     // This just clears the webwidget_internal_ member from RenderWidget.
-    render_widget->SetWebWidgetInternal(nullptr);
+    render_widget_->SetWebWidgetInternal(nullptr);
+
+    undead_render_widget_ = std::move(render_widget_);
   }
 }
 
@@ -1943,11 +1937,6 @@ void RenderViewImpl::ApplyPageHidden(bool hidden, bool initial_setting) {
   // does not change when tests override the visibility of the Page.
 }
 
-void RenderViewImpl::MakeMainFrameRenderWidgetUndead() {
-  render_widget_->SetIsUndead(true);
-  undead_render_widget_ = std::move(render_widget_);
-}
-
 void RenderViewImpl::ReviveUndeadMainFrameRenderWidget() {
   render_widget_ = std::move(undead_render_widget_);
   render_widget_->SetIsUndead(false);

content/renderer/render_view_impl.h

@@ -326,10 +326,8 @@ class CONTENT_EXPORT RenderViewImpl : public blink::WebViewClient,
   // be able to specify |initial_setting| where IPC handlers do not.
   void ApplyPageHidden(bool hidden, bool initial_setting);
 
-  // These functions take the main frame's RenderWidget and to make it
-  // inaccessible for out-of-process main frames and then brings it back
-  // if the main frame comes back into the current process.
-  void MakeMainFrameRenderWidgetUndead();
+  // Instead of creating a new RenderWidget, this revives the undead
+  // RenderWidget for use with a new local main frame.
   void ReviveUndeadMainFrameRenderWidget();
 
  private: