Separate RenderViewHost from RenderWidgetHost, part 10: shutdown.

content/browser/renderer_host/render_widget_host_impl.cc

 // Copyright (c) 2012 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/browser/renderer_host/render_widget_host_impl.h"
 
 #include <math.h>
 #include <set>
 #include <utility>
 
@@ skipping 160 matching lines @@
 
 }  // namespace
 
 ///////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHostImpl
 
 RenderWidgetHostImpl::RenderWidgetHostImpl(RenderWidgetHostDelegate* delegate,
                                            RenderProcessHost* process,
                                            int32_t routing_id,
                                            bool hidden)
-    : view_(NULL),
-      renderer_initialized_(false),
+    : renderer_initialized_(false),
+      destroyed_(false),
       delegate_(delegate),
       owner_delegate_(nullptr),
       process_(process),
       routing_id_(routing_id),
       is_loading_(false),
       is_hidden_(hidden),
       repaint_ack_pending_(false),
       resize_ack_pending_(false),
       color_profile_out_of_date_(false),
       auto_resize_enabled_(false),
@@ skipping 48 matching lines @@
         base::Bind(&RenderWidgetHostImpl::RendererIsUnresponsive,
                    weak_factory_.GetWeakPtr())));
   }
 
   new_content_rendering_timeout_.reset(new TimeoutMonitor(
       base::Bind(&RenderWidgetHostImpl::ClearDisplayedGraphics,
                  weak_factory_.GetWeakPtr())));
 }
 
 RenderWidgetHostImpl::~RenderWidgetHostImpl() {
-  if (view_weak_)
-    view_weak_->RenderWidgetHostGone();
-  SetView(NULL);
-
-  process_->RemoveRoute(routing_id_);
-  g_routing_id_widget_map.Get().erase(
-      RenderWidgetHostID(process_->GetID(), routing_id_));
-
-  if (delegate_)
-    delegate_->RenderWidgetDeleted(this);
+  if (!destroyed_)
+    Destroy(false);

[Avi (use Gerrit), 2015/11/20 20:10:40]

Perhaps instead DCHECK that Destroy() was called?

[ncarter (slow), 2015/11/30 21:32:33]

You can do the DCHECK, but you'll have to fix up at least the following
unittests:

https://code.google.com/p/chromium/codesearch#search/&q=scoped_ptr.*%5C%3C(Re...

I think the DCHECK makes sense, because otherwise it seems we open a way for the
callers to skip the Send() of the Shutdown IPC messages, and as far as I can
tell we don't need to expose a "Destroy without Shutdown" method to outside this
class (unless RVH will need it)?

 }
 
 // static
 RenderWidgetHost* RenderWidgetHost::FromID(
     int32_t process_id,
     int32_t routing_id) {
   return RenderWidgetHostImpl::FromID(process_id, routing_id);
 }
 
 // static
@@ skipping 39 matching lines @@
   return hosts.Pass();
 }
 
 // static
 RenderWidgetHostImpl* RenderWidgetHostImpl::From(RenderWidgetHost* rwh) {
   return static_cast<RenderWidgetHostImpl*>(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.
   if (view_ && renderer_initialized_) {
     Send(new ViewMsg_SetSurfaceIdNamespace(routing_id_,
                                            view_->GetSurfaceIdNamespace()));
   }
 
   synthetic_gesture_controller_.reset();
 }
 
 RenderProcessHost* RenderWidgetHostImpl::GetProcess() const {
   return process_;
 }
 
 int RenderWidgetHostImpl::GetRoutingID() const {
   return routing_id_;
 }
 
 RenderWidgetHostViewBase* RenderWidgetHostImpl::GetView() const {
-  return view_;
+  return view_.get();
 }
 
 gfx::NativeViewId RenderWidgetHostImpl::GetNativeViewId() const {
   if (view_)
     return view_->GetNativeViewId();
   return 0;
 }
 
 void RenderWidgetHostImpl::ResetSizeAndRepaintPendingFlags() {
   resize_ack_pending_ = false;
@@ skipping 62 matching lines @@
 
   if (owner_delegate_)
     owner_delegate_->RenderWidgetDidInit();
 }
 
 void RenderWidgetHostImpl::InitForFrame() {
   DCHECK(process_->HasConnection());
   renderer_initialized_ = true;
 }
 
-void RenderWidgetHostImpl::Shutdown() {
+void RenderWidgetHostImpl::ShutdownAndDestroyWidget(bool also_delete) {
   RejectMouseLockOrUnlockIfNecessary();
 
   if (process_->HasConnection()) {
     // Tell the renderer object to close.
     bool rv = Send(new ViewMsg_Close(routing_id_));
     DCHECK(rv);
   }
 
-  Destroy();
+  Destroy(also_delete);
 }
 
 bool RenderWidgetHostImpl::IsLoading() const {
   return is_loading_;
 }
 
 bool RenderWidgetHostImpl::OnMessageReceived(const IPC::Message &msg) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(RenderWidgetHostImpl, msg)
     IPC_MESSAGE_HANDLER(FrameHostMsg_RenderProcessGone, OnRenderProcessGone)
@@ skipping 914 matching lines @@
     process_->WidgetHidden();
     is_hidden_ = true;
   }
 
   // Reset this to ensure the hung renderer mechanism is working properly.
   in_flight_event_count_ = 0;
   StopHangMonitorTimeout();
 
   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
   // renderer. Otherwise it may be stuck waiting for the old renderer to ack an
   // event. (In particular, the above call to view_->RenderProcessGone will
   // destroy the aura window, which may dispatch a synthetic mouse move.)
   input_router_.reset(new InputRouterImpl(
       process_, this, this, routing_id_, GetInputRouterConfigForPlatform()));
 
   synthetic_gesture_controller_.reset();
@@ skipping 55 matching lines @@
 }
 
 void RenderWidgetHostImpl::SetAutoResize(bool enable,
                                          const gfx::Size& min_size,
                                          const gfx::Size& max_size) {
   auto_resize_enabled_ = enable;
   min_size_for_auto_resize_ = min_size;
   max_size_for_auto_resize_ = max_size;
 }
 
-void RenderWidgetHostImpl::Destroy() {
+void RenderWidgetHostImpl::Destroy(bool also_delete) {
+  DCHECK(!destroyed_);
+  destroyed_ = true;
+
   NotificationService::current()->Notify(
-      NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
-      Source<RenderWidgetHost>(this),
+      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;
+    view_.reset();
   }
 
-  delete this;
+  process_->RemoveRoute(routing_id_);
+  g_routing_id_widget_map.Get().erase(
+      RenderWidgetHostID(process_->GetID(), routing_id_));
+
+  if (delegate_)
+    delegate_->RenderWidgetDeleted(this);
+
+  if (also_delete)
+    delete this;
 }
 
 void RenderWidgetHostImpl::RendererIsUnresponsive() {
   NotificationService::current()->Notify(
       NOTIFICATION_RENDER_WIDGET_HOST_HANG,
       Source<RenderWidgetHost>(this),
       NotificationService::NoDetails());
   is_unresponsive_ = true;
   if (delegate_)
     delegate_->RendererUnresponsive(this);
@@ skipping 13 matching lines @@
     view_->ClearCompositorFrame();
 }
 
 void RenderWidgetHostImpl::OnRenderProcessGone(int status, int exit_code) {
   // RenderFrameHost owns a RenderWidgetHost when it needs one, in which case
   // it handles destruction.
   if (!owned_by_render_frame_host_) {
     // TODO(evanm): This synchronously ends up calling "delete this".
     // Is that really what we want in response to this message?  I'm matching
     // previous behavior of the code here.
-    Destroy();
+    Destroy(true);
   } else {
     RendererExited(static_cast<base::TerminationStatus>(status), exit_code);
   }
 }
 
 void RenderWidgetHostImpl::OnClose() {
-  Shutdown();
+  ShutdownAndDestroyWidget(true);
 }
 
 void RenderWidgetHostImpl::OnSetTooltipText(
     const base::string16& tooltip_text,
     WebTextDirection text_direction_hint) {
   // First, add directionality marks around tooltip text if necessary.
   // A naive solution would be to simply always wrap the text. However, on
   // windows, Unicode directional embedding characters can't be displayed on
   // systems that lack RTL fonts and are instead displayed as empty squares.
   //
@@ skipping 209 matching lines @@
 
 void RenderWidgetHostImpl::OnSetCursor(const WebCursor& cursor) {
   SetCursor(cursor);
 }
 
 void RenderWidgetHostImpl::SetTouchEventEmulationEnabled(
     bool enabled, ui::GestureProviderConfigType config_type) {
   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 {
     if (touch_emulator_)
       touch_emulator_->Disable();
   }
 }
 
 void RenderWidgetHostImpl::OnTextInputStateChanged(
     const ViewHostMsg_TextInputState_Params& params) {
@@ skipping 496 matching lines @@
 }
 
 #if defined(OS_WIN)
 gfx::NativeViewAccessible
     RenderWidgetHostImpl::GetParentNativeViewAccessible() {
   return delegate_ ? delegate_->GetParentNativeViewAccessible() : NULL;
 }
 #endif
 
 }  // namespace content