Revert "Removed use of set_owned_by_client()."

This reverts commit 6cba64bc.

Reason for revert: Seems to be causing crash failures in NotificationMenuViewTest tests:
https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-dbg/18756

CL landed in a previous build https://ci.chromium.org/p/chromium/builders/ci/linux-chromeos-dbg/18755

but the error was obscured by a different compile failure.

This CL looked most suspect, so apologies if this was a bad revert.

Original change's description:
> Removed use of set_owned_by_client().
> 
> NotificationMenuView
> 
> Bug: 1044687
> Change-Id: Ie7b3c57bef5d55d306e5f083d709cc29a8bb0444
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2209493
> Reviewed-by: Alex Newcomer <newcomer@chromium.org>
> Commit-Queue: Allen Bauer <kylixrd@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#770303}

TBR=kylixrd@chromium.org,newcomer@chromium.org

Change-Id: I8b7c4193038da689a99ba77369b33ee1e3e65ef0
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1044687
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2210170Reviewed-by: Darren Shen <shend@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770472}

ash/app_menu/notification_menu_view.cc

@@ -27,14 +27,16 @@ NotificationMenuView::NotificationMenuView(
     : app_id_(app_id),
       notification_item_view_delegate_(notification_item_view_delegate),
       slide_out_controller_delegate_(slide_out_controller_delegate),
-      double_separator_(AddChildView(std::make_unique<views::MenuSeparator>(
-          ui::MenuSeparatorType::DOUBLE_SEPARATOR))),
-      header_view_(
-          AddChildView(std::make_unique<NotificationMenuHeaderView>())) {
+      double_separator_(
+          new views::MenuSeparator(ui::MenuSeparatorType::DOUBLE_SEPARATOR)),
+      header_view_(new NotificationMenuHeaderView()) {
   DCHECK(notification_item_view_delegate_);
   DCHECK(slide_out_controller_delegate_);
   DCHECK(!app_id_.empty())
       << "Only context menus for applications can show notifications.";
+
+  AddChildView(double_separator_);
+  AddChildView(header_view_);
 }
 
 NotificationMenuView::~NotificationMenuView() = default;
@@ -84,8 +86,9 @@ void NotificationMenuView::AddNotificationItemView(
       notification_item_view_delegate_, slide_out_controller_delegate_,
       notification.title(), notification.message(), notification.icon(),
       notification.id());
-  notification_item_views_.push_front(
-      AddChildView(std::move(notification_view)));
+  notification_view->set_owned_by_client();
+  AddChildView(notification_view.get());
+  notification_item_views_.push_front(std::move(notification_view));
 
   header_view_->UpdateCounter(notification_item_views_.size());
 
@@ -93,12 +96,14 @@ void NotificationMenuView::AddNotificationItemView(
     return;
 
   // Push |old_displayed_notification_item_view| to |overflow_view_|.
-  old_displayed_item->SetVisible(false);
+  RemoveChildView(old_displayed_item);
 
   const bool overflow_view_created = !overflow_view_;
-  if (!overflow_view_)
-    overflow_view_ = AddChildView(std::make_unique<NotificationOverflowView>());
-
+  if (!overflow_view_) {
+    overflow_view_ = std::make_unique<NotificationOverflowView>();
+    overflow_view_->set_owned_by_client();
+    AddChildView(overflow_view_.get());
+  }
   overflow_view_->AddIcon(old_displayed_item->proportional_image_view(),
                           old_displayed_item->notification_id());
 
@@ -110,7 +115,6 @@ void NotificationMenuView::AddNotificationItemView(
   }
   Layout();
 }
-
 void NotificationMenuView::UpdateNotificationItemView(
     const message_center::Notification& notification) {
   // Find the view which corresponds to |notification|.
@@ -129,9 +133,8 @@ void NotificationMenuView::OnNotificationRemoved(
   if (i == notification_item_views_.end())
     return;
   const bool removed_displayed_notification =
-      *i == GetDisplayedNotificationItemView();
+      i->get() == GetDisplayedNotificationItemView();
 
-  RemoveChildViewT(*i);
   notification_item_views_.erase(i);
   header_view_->UpdateCounter(notification_item_views_.size());
 
@@ -139,7 +142,7 @@ void NotificationMenuView::OnNotificationRemoved(
     // Display the next notification.
     auto* item = GetDisplayedNotificationItemView();
     if (item) {
-      item->SetVisible(true);
+      AddChildView(item);
       if (overflow_view_)
         overflow_view_->RemoveIcon(item->notification_id());
     }
@@ -148,9 +151,7 @@ void NotificationMenuView::OnNotificationRemoved(
   }
 
   if (overflow_view_ && overflow_view_->is_empty()) {
-    // Remove and delete |overflow_view_|.
-    RemoveChildViewT(overflow_view_);
-    overflow_view_ = nullptr;
+    overflow_view_.reset();
     PreferredSizeChanged();
     notification_item_view_delegate_->OnOverflowAddedOrRemoved();
   }
@@ -163,8 +164,9 @@ ui::Layer* NotificationMenuView::GetSlideOutLayer() {
 
 const NotificationItemView*
 NotificationMenuView::GetDisplayedNotificationItemView() const {
-  return notification_item_views_.empty() ? nullptr
-                                          : notification_item_views_.front();
+  return notification_item_views_.empty()
+             ? nullptr
+             : notification_item_views_.front().get();
 }
 
 const std::string& NotificationMenuView::GetDisplayedNotificationID() const {

ash/app_menu/notification_menu_view.h

@@ -89,7 +89,8 @@ class APP_MENU_EXPORT NotificationMenuView : public views::View {
  private:
   friend class NotificationMenuViewTestAPI;
 
-  using NotificationItemViews = std::deque<NotificationItemView*>;
+  using NotificationItemViews =
+      std::deque<std::unique_ptr<NotificationItemView>>;
 
   // Returns an iterator to the notification matching the supplied ID, or
   // notification_item_views_.end() if none.
@@ -106,18 +107,18 @@ class APP_MENU_EXPORT NotificationMenuView : public views::View {
 
   // The deque of NotificationItemViews. The front item in the deque is the view
   // which is shown.
-  NotificationItemViews notification_item_views_;
+  std::deque<std::unique_ptr<NotificationItemView>> notification_item_views_;
 
   // A double separator used to distinguish notifications from context menu
   // options. Owned by views hierarchy.
   views::MenuSeparator* double_separator_;
 
   // Holds the header and counter texts. Owned by views hierarchy.
-  NotificationMenuHeaderView* header_view_;
+  NotificationMenuHeaderView* const header_view_;
 
   // A view that shows icons of notifications for this app that are not being
   // shown.
-  NotificationOverflowView* overflow_view_;
+  std::unique_ptr<NotificationOverflowView> overflow_view_;
 
   DISALLOW_COPY_AND_ASSIGN(NotificationMenuView);
 };

ash/app_menu/notification_menu_view_test_api.cc

@@ -28,7 +28,7 @@ int NotificationMenuViewTestAPI::GetItemViewCount() const {
 }
 
 NotificationOverflowView* NotificationMenuViewTestAPI::GetOverflowView() const {
-  return notification_menu_view_->overflow_view_;
+  return notification_menu_view_->overflow_view_.get();
 }
 
 }  // namespace ash