Fix NotificationViewMD leak and restore unit tests.

NotificationViewMDTest were reverted due to LSan failure in
NotificationViewMD.
This CL fixes the leak and restores NotificationViewMDTest.
The leak was caused because we assumed a->RemoveChildView(b)
would delete the b object, but it was not true.
We should use DCHECK(a->Contains(b)); delete b; instead.

Original CL: https://codereview.chromium.org/2966343002/

BUG=733948,739356
TEST=
gn gen out/asan --args='is_asan=true is_lsan=true enable_nacl=false
is_debug=false' &&
ASAN_OPTIONS="detect_leaks=1 symbolize=1
external_symbolizer_path=`pwd`/third_party/llvm-build/
Release+Asserts/bin/llvm-symbolizer" out/asan/message_center_unittests

Change-Id: I81f1aefdab6fafa35fd00e69875b1aa426822c9e
Reviewed-on: https://chromium-review.googlesource.com/564859Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485591}

ui/message_center/BUILD.gn

@@ -268,6 +268,7 @@ test("message_center_unittests") {
         "views/message_center_view_unittest.cc",
         "views/message_list_view_unittest.cc",
         "views/message_popup_collection_unittest.cc",
+        "views/notification_view_md_unittest.cc",
         "views/notification_view_unittest.cc",
         "views/notifier_settings_view_unittest.cc",
       ]

ui/message_center/views/notification_view_md.cc

@@ -117,6 +117,8 @@ class ItemView : public views::View {
   explicit ItemView(const message_center::NotificationItem& item);
   ~ItemView() override;
 
+  const char* GetClassName() const override;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ItemView);
 };
@@ -143,6 +145,10 @@ ItemView::ItemView(const message_center::NotificationItem& item) {
 
 ItemView::~ItemView() = default;
 
+const char* ItemView::GetClassName() const {
+  return "ItemView";
+}
+
 // CompactTitleMessageView /////////////////////////////////////////////////////
 
 // CompactTitleMessageView shows notification title and message in a single
@@ -152,6 +158,8 @@ class CompactTitleMessageView : public views::View {
   explicit CompactTitleMessageView();
   ~CompactTitleMessageView() override;
 
+  const char* GetClassName() const override;
+
   void OnPaint(gfx::Canvas* canvas) override;
 
   void set_title(const base::string16& title) { title_ = title; }
@@ -167,7 +175,11 @@ class CompactTitleMessageView : public views::View {
   views::Label* message_view_ = nullptr;
 };
 
-CompactTitleMessageView::~CompactTitleMessageView() {}
+CompactTitleMessageView::~CompactTitleMessageView() = default;
+
+const char* CompactTitleMessageView::GetClassName() const {
+  return "CompactTitleMessageView";
+}
 
 CompactTitleMessageView::CompactTitleMessageView() {
   SetLayoutManager(new views::FillLayout());
@@ -230,6 +242,7 @@ class NotificationButtonMD : public views::LabelButton {
   ~NotificationButtonMD() override;
 
   void SetText(const base::string16& text) override;
+  const char* GetClassName() const override;
 
   std::unique_ptr<views::InkDropHighlight> CreateInkDropHighlight()
       const override;
@@ -260,6 +273,10 @@ void NotificationButtonMD::SetText(const base::string16& text) {
   views::LabelButton::SetText(base::i18n::ToUpper(text));
 }
 
+const char* NotificationButtonMD::GetClassName() const {
+  return "NotificationButtonMD";
+}
+
 std::unique_ptr<views::InkDropHighlight>
 NotificationButtonMD::CreateInkDropHighlight() const {
   std::unique_ptr<views::InkDropHighlight> highlight =
@@ -315,10 +332,12 @@ void NotificationViewMD::CreateOrUpdateViews(const Notification& notification) {
   CreateOrUpdateIconView(notification);
   CreateOrUpdateSmallIconView(notification);
   CreateOrUpdateImageView(notification);
-  CreateOrUpdateActionButtonViews(notification);
   CreateOrUpdateCloseButtonView(notification);
   CreateOrUpdateSettingsButtonView(notification);
   UpdateViewForExpandedState(expanded_);
+  // Should be called at the last because SynthesizeMouseMoveEvent() requires
+  // everything is in the right location when called.
+  CreateOrUpdateActionButtonViews(notification);
 }
 
 NotificationViewMD::NotificationViewMD(MessageCenterController* controller,
@@ -483,8 +502,10 @@ void NotificationViewMD::CreateOrUpdateContextTitleView(
 
 void NotificationViewMD::CreateOrUpdateTitleView(
     const Notification& notification) {
-  if (notification.type() == NOTIFICATION_TYPE_PROGRESS) {
-    left_content_->RemoveChildView(title_view_);
+  if (notification.title().empty() ||
+      notification.type() == NOTIFICATION_TYPE_PROGRESS) {
+    DCHECK(!title_view_ || left_content_->Contains(title_view_));
+    delete title_view_;
     title_view_ = nullptr;
     return;
   }
@@ -539,7 +560,9 @@ void NotificationViewMD::CreateOrUpdateMessageView(
 void NotificationViewMD::CreateOrUpdateCompactTitleMessageView(
     const Notification& notification) {
   if (notification.type() != NOTIFICATION_TYPE_PROGRESS) {
-    left_content_->RemoveChildView(compact_title_message_view_);
+    DCHECK(!compact_title_message_view_ ||
+           left_content_->Contains(compact_title_message_view_));
+    delete compact_title_message_view_;
     compact_title_message_view_ = nullptr;
     return;
   }
@@ -556,7 +579,8 @@ void NotificationViewMD::CreateOrUpdateCompactTitleMessageView(
 void NotificationViewMD::CreateOrUpdateProgressBarView(
     const Notification& notification) {
   if (notification.type() != NOTIFICATION_TYPE_PROGRESS) {
-    left_content_->RemoveChildView(progress_bar_view_);
+    DCHECK(!progress_bar_view_ || left_content_->Contains(progress_bar_view_));
+    delete progress_bar_view_;
     progress_bar_view_ = nullptr;
     header_row_->ClearProgress();
     return;
@@ -604,7 +628,8 @@ void NotificationViewMD::CreateOrUpdateIconView(
     const Notification& notification) {
   if (notification.type() == NOTIFICATION_TYPE_PROGRESS ||
       notification.type() == NOTIFICATION_TYPE_MULTIPLE) {
-    right_content_->RemoveChildView(icon_view_);
+    DCHECK(!icon_view_ || right_content_->Contains(icon_view_));
+    delete icon_view_;
     icon_view_ = nullptr;
     return;
   }
@@ -637,8 +662,8 @@ void NotificationViewMD::CreateOrUpdateImageView(
   if (notification.image().IsEmpty()) {
     if (image_container_) {
       DCHECK(image_view_);
-
-      left_content_->RemoveChildView(image_container_);
+      DCHECK(Contains(image_container_));
+      delete image_container_;
       image_container_ = NULL;
       image_view_ = NULL;
     } else {
@@ -696,11 +721,14 @@ void NotificationViewMD::CreateOrUpdateActionButtonViews(
     }
   }
 
-  if (new_buttons) {
-    // TODO(fukino): Investigate if this Layout() is necessary.
-    Layout();
+  // Inherit mouse hover state when action button views reset.
+  // If the view is not expanded, there should be no hover state.
+  if (new_buttons && expanded_) {
     views::Widget* widget = GetWidget();
-    if (widget != NULL) {
+    if (widget) {
+      // This Layout() is needed because button should be in the right location
+      // in the view hierarchy when SynthesizeMouseMoveEvent() is called.
+      Layout();
       widget->SetSize(widget->GetContentsView()->GetPreferredSize());
       GetWidget()->SynthesizeMouseMoveEvent();
     }

ui/message_center/views/notification_view_md.h

@@ -64,6 +64,13 @@ class MESSAGE_CENTER_EXPORT NotificationViewMD
   views::View* TargetForRect(views::View* root, const gfx::Rect& rect) override;
 
  private:
+  FRIEND_TEST_ALL_PREFIXES(NotificationViewMDTest, CreateOrUpdateTest);
+  FRIEND_TEST_ALL_PREFIXES(NotificationViewMDTest, TestIconSizing);
+  FRIEND_TEST_ALL_PREFIXES(NotificationViewMDTest, UpdateButtonsStateTest);
+  FRIEND_TEST_ALL_PREFIXES(NotificationViewMDTest, UpdateButtonCountTest);
+
+  friend class NotificationViewMDTest;
+
   void CreateOrUpdateViews(const Notification& notification);
 
   void CreateOrUpdateContextTitleView(const Notification& notification);