views: introduce WidgetDelegate::TransferOwnershipOfContentsView

This change is the first step in rectifying the confusion around
WidgetDelegate's ownership of its contents view, which is documented
at length in WidgetDelegate's class definition. Specifically, this
change makes it clear when it is *intended* that ownership of the
contents view is taken from the WidgetDelegate.

This is step 1 of this plan:
1) Introduce WidgetDelegate::TransferOwnershipOfContentsView() and
   migrate to it in places that are trying to take ownership
2) Introduce WidgetDelegate::SetContentsView(), which will either:
   a. Store the provided view in a unique_ptr, if it is not owned
      by client, or
   b. Store the provided view in a raw pointer, if it is owned by
      client
3) Replace all existing overrides of GetContentsView() with uses of
   SetContentsView()
4) Require that !owned_by_client() in SetContentsView(), which will
   likely require intense surgery of client classes
5) Have SetContentsView() take and TransferOwnershipOfContentsView()
   return a unique_ptr<View> rather than a View*

This multi-step plan avoids ever storing a view with owned_by_client
in a unique_ptr, which is an extremely simple route to an accidental
double-free.

Bug: 1075649
Change-Id: I3ee55423ba5e4e7b48a50a9f4e04299f0721d410
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2376449
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804322}

ash/hud_display/hud_display.cc

@@ -173,7 +173,7 @@ void HUDDisplayView::SetDisplayMode(DisplayMode display_mode) {
 }
 
 views::ClientView* HUDDisplayView::CreateClientView(views::Widget* widget) {
-  return new HTClientView(this, widget, GetContentsView());
+  return new HTClientView(this, widget, TransferOwnershipOfContentsView());
 }
 
 void HUDDisplayView::OnWidgetInitialized() {

ui/views/widget/widget.cc

@@ -364,7 +364,7 @@ void Widget::Init(InitParams params) {
       saved_show_state_ = ui::SHOW_STATE_MINIMIZED;
     }
   } else if (delegate) {
-    SetContentsView(delegate->GetContentsView());
+    SetContentsView(delegate->TransferOwnershipOfContentsView());
     SetInitialBoundsForFramelessWindow(bounds);
   }
 

ui/views/widget/widget_delegate.cc

@@ -200,8 +200,14 @@ View* WidgetDelegate::GetContentsView() {
   return default_contents_view_;
 }
 
+View* WidgetDelegate::TransferOwnershipOfContentsView() {
+  DCHECK(!contents_view_taken_);
+  contents_view_taken_ = true;
+  return GetContentsView();
+}
+
 ClientView* WidgetDelegate::CreateClientView(Widget* widget) {
-  return new ClientView(widget, GetContentsView());
+  return new ClientView(widget, TransferOwnershipOfContentsView());
 }
 
 std::unique_ptr<NonClientFrameView> WidgetDelegate::CreateNonClientFrameView(

ui/views/widget/widget_delegate.h

@@ -242,6 +242,21 @@ class VIEWS_EXPORT WidgetDelegate {
   // replace it.
   virtual View* GetContentsView();
 
+  // Returns ownership of the contents view, which means something similar to
+  // but not the same as C++ ownership in the unique_ptr sense. The caller
+  // takes on responsibility for either destroying the returned View (if it
+  // is !owned_by_client()) or not (if it is owned_by_client()). Since this
+  // returns a raw pointer, this method serves only as a declaration of intent
+  // by the caller.
+  //
+  // It is only legal to call this method one time on a given WidgetDelegate
+  // instance.
+  //
+  // In future, this method will begin returning a unique_ptr<View> instead,
+  // and will eventually be renamed to TakeContentsView() once WidgetDelegate
+  // no longer retains any reference to the contents view internally.
+  View* TransferOwnershipOfContentsView();
+
   // Called by the Widget to create the Client View used to host the contents
   // of the widget.
   virtual ClientView* CreateClientView(Widget* widget);
@@ -322,6 +337,7 @@ class VIEWS_EXPORT WidgetDelegate {
   Params params_;
 
   View* default_contents_view_ = nullptr;
+  bool contents_view_taken_ = false;
   bool can_activate_ = true;
 
   // Managed by Widget. Ensures |this| outlives its Widget.

ui/views/window/dialog_delegate.cc

@@ -192,7 +192,7 @@ DialogDelegate* DialogDelegate::AsDialogDelegate() {
 }
 
 ClientView* DialogDelegate::CreateClientView(Widget* widget) {
-  return new DialogClientView(widget, GetContentsView());
+  return new DialogClientView(widget, TransferOwnershipOfContentsView());
 }
 
 std::unique_ptr<NonClientFrameView> DialogDelegate::CreateNonClientFrameView(