views: add WidgetDelegate::SetContentsView

This change adds two new methods to WidgetDelegate, both of which allow
setting the WidgetDelegate's contents view, thus removing the need to
override GetContentsView(). The two new methods are:

* SetContentsView(unique_ptr<View>), which takes ownership of the given
  View
* SetContentsView(View*), which doesn't

Each method can only be used with the appropriate ownership protocol, so
SetContentsView(unique_ptr<View>) will DCHECK if the provided view is
owned_by_client(), and vice versa for SetContentsView(View*). This
effectively forces clients into one of these two ownership protocols and
away from the "sometimes owning" pointer semantics of overriding
GetContentsView().

This is part 2 of the following 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*
as laid out in 1206dbae.

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

ui/views/BUILD.gn

@@ -1135,6 +1135,7 @@ test("views_unittests") {
     "widget/native_widget_unittest.cc",
     "widget/root_view_unittest.cc",
     "widget/unique_widget_ptr_unittest.cc",
+    "widget/widget_delegate_unittest.cc",
     "widget/widget_unittest.cc",
     "window/custom_frame_view_unittest.cc",
     "window/dialog_client_view_unittest.cc",

ui/views/widget/widget_delegate.cc

@@ -199,6 +199,8 @@ const Widget* WidgetDelegate::GetWidget() const {
 }
 
 View* WidgetDelegate::GetContentsView() {
+  if (unowned_contents_view_)
+    return unowned_contents_view_;
   if (!default_contents_view_)
     default_contents_view_ = new View;
   return default_contents_view_;
@@ -207,6 +209,8 @@ View* WidgetDelegate::GetContentsView() {
 View* WidgetDelegate::TransferOwnershipOfContentsView() {
   DCHECK(!contents_view_taken_);
   contents_view_taken_ = true;
+  if (owned_contents_view_)
+    owned_contents_view_.release();
   return GetContentsView();
 }
 
@@ -333,6 +337,15 @@ void WidgetDelegate::RegisterDeleteDelegateCallback(
   delete_delegate_callbacks_.emplace_back(std::move(callback));
 }
 
+void WidgetDelegate::SetContentsViewImpl(View* contents) {
+  // Note: DCHECKing the ownership of contents is done in the public setters,
+  // which are inlined in the header.
+  DCHECK(!unowned_contents_view_);
+  if (!contents->owned_by_client())
+    owned_contents_view_ = base::WrapUnique(contents);
+  unowned_contents_view_ = contents;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // WidgetDelegateView:
 

ui/views/widget/widget_delegate.h

@@ -314,6 +314,21 @@ class VIEWS_EXPORT WidgetDelegate {
   void SetCenterTitle(bool center_title);
 #endif
 
+  template <typename T>
+  T* SetContentsView(std::unique_ptr<T> contents) {
+    DCHECK(!contents->owned_by_client());
+    T* raw_contents = contents.get();
+    SetContentsViewImpl(contents.release());
+    return raw_contents;
+  }
+
+  template <typename T>
+  T* SetContentsView(T* contents) {
+    DCHECK(contents->owned_by_client());
+    SetContentsViewImpl(contents);
+    return contents;
+  }
+
   // A convenience wrapper that does all three of SetCanMaximize,
   // SetCanMinimize, and SetCanResize.
   void SetHasWindowSizeControls(bool has_controls);
@@ -338,6 +353,8 @@ class VIEWS_EXPORT WidgetDelegate {
  private:
   friend class Widget;
 
+  void SetContentsViewImpl(View* contents);
+
   // The Widget that was initialized with this instance as its WidgetDelegate,
   // if any.
   Widget* widget_ = nullptr;
@@ -347,6 +364,9 @@ class VIEWS_EXPORT WidgetDelegate {
   bool contents_view_taken_ = false;
   bool can_activate_ = true;
 
+  View* unowned_contents_view_ = nullptr;
+  std::unique_ptr<View> owned_contents_view_;
+
   // Managed by Widget. Ensures |this| outlives its Widget.
   bool can_delete_this_ = true;
 

ui/views/widget/widget_delegate_unittest.cc

+// Copyright 2020 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 "ui/views/widget/widget_delegate.h"
+
+#include <utility>
+
+#include "ui/views/test/views_test_base.h"
+#include "ui/views/view.h"
+#include "ui/views/view_tracker.h"
+
+namespace views {
+namespace {
+
+using WidgetDelegateTest = views::ViewsTestBase;
+
+TEST_F(WidgetDelegateTest, ClientOwnedContentsViewOwnershipNotHeld) {
+  std::unique_ptr<View> view = std::make_unique<View>();
+  view->set_owned_by_client();
+  ViewTracker tracker(view.get());
+
+  auto delegate = std::make_unique<WidgetDelegate>();
+  delegate->SetContentsView(view.get());
+  delegate.reset();
+
+  ASSERT_TRUE(tracker.view());
+  view.reset();
+  EXPECT_FALSE(tracker.view());
+}
+
+TEST_F(WidgetDelegateTest, ContentsViewOwnershipHeld) {
+  std::unique_ptr<View> view = std::make_unique<View>();
+  ViewTracker tracker(view.get());
+
+  auto delegate = std::make_unique<WidgetDelegate>();
+  delegate->SetContentsView(std::move(view));
+  delegate.reset();
+
+  EXPECT_FALSE(tracker.view());
+}
+
+TEST_F(WidgetDelegateTest, ContentsViewOwnershipTransferredToCaller) {
+  std::unique_ptr<View> view = std::make_unique<View>();
+  ViewTracker tracker(view.get());
+  std::unique_ptr<View> same_view;
+
+  auto delegate = std::make_unique<WidgetDelegate>();
+  delegate->SetContentsView(std::move(view));
+  same_view = base::WrapUnique(delegate->TransferOwnershipOfContentsView());
+  EXPECT_EQ(tracker.view(), same_view.get());
+  delegate.reset();
+
+  EXPECT_TRUE(tracker.view());
+}
+
+TEST_F(WidgetDelegateTest, GetContentsViewDoesNotTransferOwnership) {
+  std::unique_ptr<View> view = std::make_unique<View>();
+  ViewTracker tracker(view.get());
+
+  auto delegate = std::make_unique<WidgetDelegate>();
+  delegate->SetContentsView(std::move(view));
+  EXPECT_EQ(delegate->GetContentsView(), tracker.view());
+  delegate.reset();
+
+  EXPECT_FALSE(tracker.view());
+}
+
+}  // namespace
+}  // namespace views