GMC: Remove uses of set_owned_by_client

This CL removes uses of set_owned_by_client in the Global Media
Controls code.

Bug: 1009708
Change-Id: I08a0c49310b356a225a92f5522189f3e69621524
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1885499Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Tommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710499}

ash/media/media_notification_container_impl.cc

@@ -4,6 +4,7 @@
 
 #include "ash/media/media_notification_container_impl.h"
 
+#include "components/media_message_center/media_notification_view.h"
 #include "ui/message_center/message_center.h"
 #include "ui/message_center/public/cpp/message_center_constants.h"
 #include "ui/message_center/views/notification_control_buttons_view.h"
@@ -16,25 +17,19 @@ namespace ash {
 MediaNotificationContainerImpl::MediaNotificationContainerImpl(
     const message_center::Notification& notification,
     base::WeakPtr<media_message_center::MediaNotificationItem> item)
-    : message_center::MessageView(notification),
-      control_buttons_view_(
-          std::make_unique<message_center::NotificationControlButtonsView>(
-              this)),
-      view_(
-          this,
-          std::move(item),
-          control_buttons_view_.get(),
-          message_center::MessageCenter::Get()->GetSystemNotificationAppName(),
-          message_center::kNotificationWidth,
-          /*should_show_icon=*/true) {
+    : message_center::MessageView(notification) {
   SetLayoutManager(std::make_unique<views::FillLayout>());
 
-  // Since we own these, we don't want Views to destroy them when their parent
-  // is destroyed.
-  control_buttons_view_->set_owned_by_client();
-  view_.set_owned_by_client();
+  auto control_buttons_view =
+      std::make_unique<message_center::NotificationControlButtonsView>(this);
+  control_buttons_view_ = control_buttons_view.get();
 
-  AddChildView(&view_);
+  auto view = std::make_unique<media_message_center::MediaNotificationView>(
+      this, std::move(item), std::move(control_buttons_view),
+      message_center::MessageCenter::Get()->GetSystemNotificationAppName(),
+      message_center::kNotificationWidth,
+      /*should_show_icon=*/true);
+  view_ = AddChildView(std::move(view));
 
   SetBackground(views::CreateSolidBackground(SK_ColorTRANSPARENT));
 }
@@ -54,17 +49,17 @@ void MediaNotificationContainerImpl::UpdateWithNotification(
 
 message_center::NotificationControlButtonsView*
 MediaNotificationContainerImpl::GetControlButtonsView() const {
-  return control_buttons_view_.get();
+  return control_buttons_view_;
 }
 
 void MediaNotificationContainerImpl::SetExpanded(bool expanded) {
-  view_.SetExpanded(expanded);
+  view_->SetExpanded(expanded);
 }
 
 void MediaNotificationContainerImpl::UpdateCornerRadius(int top_radius,
                                                         int bottom_radius) {
   MessageView::SetCornerRadius(top_radius, bottom_radius);
-  view_.UpdateCornerRadius(top_radius, bottom_radius);
+  view_->UpdateCornerRadius(top_radius, bottom_radius);
 }
 
 void MediaNotificationContainerImpl::OnExpanded(bool expanded) {

ash/media/media_notification_container_impl.h

@@ -8,11 +8,11 @@
 #include "ash/ash_export.h"
 #include "base/containers/flat_set.h"
 #include "components/media_message_center/media_notification_container.h"
-#include "components/media_message_center/media_notification_view.h"
 #include "ui/message_center/views/message_view.h"
 
 namespace media_message_center {
 class MediaNotificationItem;
+class MediaNotificationView;
 }  // namespace media_message_center
 
 namespace ash {
@@ -57,10 +57,10 @@ class ASH_EXPORT MediaNotificationContainerImpl
       const message_center::Notification& notification);
 
   // View containing close and settings buttons.
-  std::unique_ptr<message_center::NotificationControlButtonsView>
-      control_buttons_view_;
+  message_center::NotificationControlButtonsView* control_buttons_view_ =
+      nullptr;
 
-  media_message_center::MediaNotificationView view_;
+  media_message_center::MediaNotificationView* view_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(MediaNotificationContainerImpl);
 };

chrome/browser/ui/views/global_media_controls/media_notification_container_impl_view.cc

@@ -68,26 +68,25 @@ MediaNotificationContainerImplView::MediaNotificationContainerImplView(
   SetTooltipText(
       l10n_util::GetStringUTF16(IDS_GLOBAL_MEDIA_CONTROLS_BACK_TO_TAB));
 
-  swipeable_container_ = std::make_unique<views::View>();
-  swipeable_container_->set_owned_by_client();
-  swipeable_container_->SetLayoutManager(std::make_unique<views::FillLayout>());
-  swipeable_container_->SetPaintToLayer();
-  swipeable_container_->layer()->SetFillsBoundsOpaquely(false);
-  AddChildView(swipeable_container_.get());
-
-  dismiss_button_placeholder_ = std::make_unique<views::View>();
-  dismiss_button_placeholder_->set_owned_by_client();
-  dismiss_button_placeholder_->SetPreferredSize(kDismissButtonSize);
-  dismiss_button_placeholder_->SetLayoutManager(
+  auto swipeable_container = std::make_unique<views::View>();
+  swipeable_container->SetLayoutManager(std::make_unique<views::FillLayout>());
+  swipeable_container->SetPaintToLayer();
+  swipeable_container->layer()->SetFillsBoundsOpaquely(false);
+  swipeable_container_ = AddChildView(std::move(swipeable_container));
+
+  auto dismiss_button_placeholder = std::make_unique<views::View>();
+  dismiss_button_placeholder->SetPreferredSize(kDismissButtonSize);
+  dismiss_button_placeholder->SetLayoutManager(
       std::make_unique<views::FillLayout>());
+  dismiss_button_placeholder_ = dismiss_button_placeholder.get();
 
-  dismiss_button_container_ = std::make_unique<views::View>();
-  dismiss_button_container_->set_owned_by_client();
-  dismiss_button_container_->SetPreferredSize(kDismissButtonSize);
-  dismiss_button_container_->SetLayoutManager(
+  auto dismiss_button_container = std::make_unique<views::View>();
+  dismiss_button_container->SetPreferredSize(kDismissButtonSize);
+  dismiss_button_container->SetLayoutManager(
       std::make_unique<views::FillLayout>());
-  dismiss_button_container_->SetVisible(false);
-  dismiss_button_placeholder_->AddChildView(dismiss_button_container_.get());
+  dismiss_button_container->SetVisible(false);
+  dismiss_button_container_ = dismiss_button_placeholder_->AddChildView(
+      std::move(dismiss_button_container));
 
   auto dismiss_button = std::make_unique<DismissButton>(this);
   dismiss_button->SetPreferredSize(kDismissButtonSize);
@@ -98,13 +97,12 @@ MediaNotificationContainerImplView::MediaNotificationContainerImplView(
       dismiss_button_container_->AddChildView(std::move(dismiss_button));
   UpdateDismissButtonIcon();
 
-  view_ = std::make_unique<media_message_center::MediaNotificationView>(
-      this, std::move(item), dismiss_button_placeholder_.get(),
+  auto view = std::make_unique<media_message_center::MediaNotificationView>(
+      this, std::move(item), std::move(dismiss_button_placeholder),
       base::string16(), kWidth, /*should_show_icon=*/false);
-  view_->set_owned_by_client();
-  ForceExpandedState();
+  view_ = swipeable_container_->AddChildView(std::move(view));
 
-  swipeable_container_->AddChildView(view_.get());
+  ForceExpandedState();
 
   slide_out_controller_ =
       std::make_unique<views::SlideOutController>(this, this);

chrome/browser/ui/views/global_media_controls/media_notification_container_impl_view.h

@@ -81,7 +81,7 @@ class MediaNotificationContainerImplView
   views::ImageButton* GetDismissButtonForTesting();
 
   media_message_center::MediaNotificationView* view_for_testing() {
-    return view_.get();
+    return view_;
   }
 
  private:
@@ -99,19 +99,19 @@ class MediaNotificationContainerImplView
   void ForceExpandedState();
 
   const std::string id_;
-  std::unique_ptr<views::View> swipeable_container_;
+  views::View* swipeable_container_ = nullptr;
 
   // Always "visible" so that it reserves space in the header so that the
   // dismiss button can appear without forcing things to shift.
-  std::unique_ptr<views::View> dismiss_button_placeholder_;
+  views::View* dismiss_button_placeholder_ = nullptr;
 
   // Shows the colored circle background behind the dismiss button to give it
   // proper contrast against the artwork. The background can't be on the dismiss
   // button itself because it messes up the ink drop.
-  std::unique_ptr<views::View> dismiss_button_container_;
+  views::View* dismiss_button_container_ = nullptr;
 
-  DismissButton* dismiss_button_;
-  std::unique_ptr<media_message_center::MediaNotificationView> view_;
+  DismissButton* dismiss_button_ = nullptr;
+  media_message_center::MediaNotificationView* view_ = nullptr;
 
   SkColor foreground_color_;
   SkColor background_color_;

components/media_message_center/media_notification_view.cc

@@ -99,13 +99,12 @@ const char MediaNotificationView::kMetadataHistogramName[] =
 MediaNotificationView::MediaNotificationView(
     MediaNotificationContainer* container,
     base::WeakPtr<MediaNotificationItem> item,
-    views::View* header_row_controls_view,
+    std::unique_ptr<views::View> header_row_controls_view,
     const base::string16& default_app_name,
     int notification_width,
     bool should_show_icon)
     : container_(container),
       item_(std::move(item)),
-      header_row_controls_view_(header_row_controls_view),
       default_app_name_(default_app_name),
       notification_width_(notification_width) {
   DCHECK(container_);
@@ -116,8 +115,10 @@ MediaNotificationView::MediaNotificationView(
   auto header_row =
       std::make_unique<message_center::NotificationHeaderView>(this);
 
-  if (header_row_controls_view_)
-    header_row->AddChildView(header_row_controls_view_);
+  if (header_row_controls_view) {
+    header_row_controls_view_ =
+        header_row->AddChildView(std::move(header_row_controls_view));
+  }
 
   header_row->SetAppName(default_app_name_);
 

components/media_message_center/media_notification_view.h

@@ -64,7 +64,7 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaNotificationView
 
   MediaNotificationView(MediaNotificationContainer* container,
                         base::WeakPtr<MediaNotificationItem> item,
-                        views::View* header_row_controls_view,
+                        std::unique_ptr<views::View> header_row_controls_view,
                         const base::string16& default_app_name,
                         int notification_width,
                         bool should_show_icon);
@@ -124,7 +124,7 @@ class COMPONENT_EXPORT(MEDIA_MESSAGE_CENTER) MediaNotificationView
 
   // Optional View that is put into the header row. E.g. in Ash we show
   // notification control buttons.
-  views::View* header_row_controls_view_;
+  views::View* header_row_controls_view_ = nullptr;
 
   // String to set as the app name of the header when there is no source title.
   base::string16 default_app_name_;