CHECK() if a WidgetDelegate would be deleted before the Widget it initialized is deleted.

WidgetDelegates must outlive their Widget.

Crashes suggest situations where they may not, but can't pinpoint the actual
client code. The crash is instead attributed to a UAF on the widget delegate
either in a layout call (during an animation), or when
Widget::OnNativeWidgetDestroyed() tries to delete the delegate (again).

This CL adds a CHECK() in ~WidgetDelegate which should track down the
misbehaving code.

Bug: 825091, 814821, 808318
Change-Id: I9490ff1682ee236436edf075f7c237ccf910c44c
Reviewed-on: https://chromium-review.googlesource.com/977244Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545873}

ui/views/widget/widget.cc

@@ -325,6 +325,9 @@ void Widget::Init(const InitParams& in_params) {
       params.delegate : new DefaultWidgetDelegate(this);
   widget_delegate_->set_can_activate(can_activate);
 
+  // Henceforth, ensure the delegate outlives the Widget.
+  widget_delegate_->can_delete_this_ = false;
+
   ownership_ = params.ownership;
   native_widget_ = CreateNativeWidget(params, this)->AsNativeWidgetPrivate();
   root_view_.reset(CreateRootView());
@@ -1092,6 +1095,7 @@ void Widget::OnNativeWidgetDestroying() {
 void Widget::OnNativeWidgetDestroyed() {
   for (WidgetObserver& observer : observers_)
     observer.OnWidgetDestroyed(this);
+  widget_delegate_->can_delete_this_ = true;
   widget_delegate_->DeleteDelegate();
   widget_delegate_ = NULL;
   native_widget_destroyed_ = true;

ui/views/widget/widget_delegate.cc

@@ -4,6 +4,7 @@
 
 #include "ui/views/widget/widget_delegate.h"
 
+#include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "services/ui/public/interfaces/window_manager_constants.mojom.h"
 #include "ui/gfx/image/image_skia.h"
@@ -17,9 +18,9 @@ namespace views {
 ////////////////////////////////////////////////////////////////////////////////
 // WidgetDelegate:
 
-WidgetDelegate::WidgetDelegate()
-    : default_contents_view_(NULL),
-      can_activate_(true) {
+WidgetDelegate::WidgetDelegate() = default;
+WidgetDelegate::~WidgetDelegate() {
+  CHECK(can_delete_this_) << "A WidgetDelegate must outlive its Widget";
 }
 
 void WidgetDelegate::OnWidgetMove() {

ui/views/widget/widget_delegate.h

@@ -184,12 +184,16 @@ class VIEWS_EXPORT WidgetDelegate {
   virtual void GetAccessiblePanes(std::vector<View*>* panes) {}
 
  protected:
-  virtual ~WidgetDelegate() {}
+  virtual ~WidgetDelegate();
 
  private:
-  View* default_contents_view_;
+  friend class Widget;
 
-  bool can_activate_;
+  View* default_contents_view_ = nullptr;
+  bool can_activate_ = true;
+
+  // Managed by Widget. Ensures |this| outlives its Widget.
+  bool can_delete_this_ = true;
 
   DISALLOW_COPY_AND_ASSIGN(WidgetDelegate);
 };