Separate RenderViewHost from RenderWidgetHost, part 10: shutdown.

content/browser/renderer_host/render_widget_host_impl.cc

Index: content/browser/renderer_host/render_widget_host_impl.cc
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 7f6269c923c404ca5f53c67ff33d66b1faf06928..d5974f3e45d9b51adb685f59f98eae492658cdf7 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -178,8 +178,7 @@ RenderWidgetHostImpl::RenderWidgetHostImpl(RenderWidgetHostDelegate* delegate,
                                            RenderProcessHost* process,
                                            int32_t routing_id,
                                            bool hidden)
-    : view_(NULL),
-      renderer_initialized_(false),
+    : renderer_initialized_(false),
       delegate_(delegate),
       owner_delegate_(nullptr),
       process_(process),
@@ -248,9 +247,18 @@ RenderWidgetHostImpl::RenderWidgetHostImpl(RenderWidgetHostDelegate* delegate,
 }
 
 RenderWidgetHostImpl::~RenderWidgetHostImpl() {
-  if (view_weak_)
-    view_weak_->RenderWidgetHostGone();
-  SetView(NULL);
+  NotificationService::current()->Notify(
+      NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED, Source<RenderWidgetHost>(this),

[ncarter (slow), 2015/11/17 20:59:57]

We gotta worry about stuff like this:

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/fil...

That won't be safe after this transformation, I think, because ~RVH has already
run. It might be safe (but gross) after we move to composition, since at that
point it'll become possible to run the ~RWH dtor while RVH::widget_ is still a
valid pointer.

[Avi (use Gerrit), 2015/11/18 00:29:59]

Sigh. Awesomeness all around. But that should call through a null pointer, which
should be easy to catch.

+      NotificationService::NoDetails());
+
+  // Tell the view to die.
+  // Note that in the process of the view shutting down, it can call a ton
+  // of other messages on us.  So if you do any other deinitialization here,
+  // do it after this call to view_->Destroy().
+  if (view_) {
+    view_->Destroy();
+    view_.reset();
+  }
 
   process_->RemoveRoute(routing_id_);
   g_routing_id_widget_map.Get().erase(
@@ -317,10 +325,9 @@ RenderWidgetHostImpl* RenderWidgetHostImpl::From(RenderWidgetHost* rwh) {
 
 void RenderWidgetHostImpl::SetView(RenderWidgetHostViewBase* view) {
   if (view)
-    view_weak_ = view->GetWeakPtr();
+    view_ = view->GetWeakPtr();
   else
-    view_weak_.reset();
-  view_ = view;
+    view_.reset();
 
   // If the renderer has not yet been initialized, then the surface ID
   // namespace will be sent during initialization.
@@ -341,7 +348,7 @@ int RenderWidgetHostImpl::GetRoutingID() const {
 }
 
 RenderWidgetHostViewBase* RenderWidgetHostImpl::GetView() const {
-  return view_;
+  return view_.get();
 }
 
 gfx::NativeViewId RenderWidgetHostImpl::GetNativeViewId() const {
@@ -424,7 +431,7 @@ void RenderWidgetHostImpl::InitForFrame() {
   renderer_initialized_ = true;
 }
 
-void RenderWidgetHostImpl::Shutdown() {
+void RenderWidgetHostImpl::ShutdownWidget(bool destroy) {
   RejectMouseLockOrUnlockIfNecessary();
 
   if (process_->HasConnection()) {
@@ -433,7 +440,8 @@ void RenderWidgetHostImpl::Shutdown() {
     DCHECK(rv);
   }
 
-  Destroy();
+  if (destroy)
+    Destroy();
 }
 
 bool RenderWidgetHostImpl::IsLoading() const {
@@ -1354,8 +1362,7 @@ void RenderWidgetHostImpl::RendererExited(base::TerminationStatus status,
 
   if (view_) {
     view_->RenderProcessGone(status, exit_code);
-    view_ = nullptr;  // The View should be deleted by RenderProcessGone.
-    view_weak_.reset();
+    view_.reset();  // The View should be deleted by RenderProcessGone.
   }
 
   // Reconstruct the input router to ensure that it has fresh state for a new
@@ -1432,20 +1439,7 @@ void RenderWidgetHostImpl::SetAutoResize(bool enable,
 }
 
 void RenderWidgetHostImpl::Destroy() {
-  NotificationService::current()->Notify(
-      NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
-      Source<RenderWidgetHost>(this),
-      NotificationService::NoDetails());
-
-  // Tell the view to die.
-  // Note that in the process of the view shutting down, it can call a ton
-  // of other messages on us.  So if you do any other deinitialization here,
-  // do it after this call to view_->Destroy().
-  if (view_) {
-    view_->Destroy();
-    view_ = nullptr;
-  }
-
+  DCHECK(!owner_delegate_);
   delete this;
 }
 
@@ -1487,7 +1481,7 @@ void RenderWidgetHostImpl::OnRenderProcessGone(int status, int exit_code) {
 }
 
 void RenderWidgetHostImpl::OnClose() {
-  Shutdown();
+  ShutdownWidget(true);
 }
 
 void RenderWidgetHostImpl::OnSetTooltipText(
@@ -1717,7 +1711,8 @@ void RenderWidgetHostImpl::SetTouchEventEmulationEnabled(
   if (enabled) {
     if (!touch_emulator_) {
       touch_emulator_.reset(new TouchEmulator(
-          this, view_ ? content::GetScaleFactorForView(view_) : 1.0f));
+          this,
+          view_.get() ? content::GetScaleFactorForView(view_.get()) : 1.0f));
     }
     touch_emulator_->Enable(config_type);
   } else {