Add workaround for MessagePopupCollection bug

This CL adds workaround to MessageView::SlideOut() for a bug that it did
not work with MessagePopupCollection.

If |by_user| of RemoveNotification is true in SlideOut(),
IncrementDeferCounter() in MessagePopupCollection::OnNotificationRemoved
is called but corresponding DecrementDeferCounter() won't be called.

MessagePopupCollection will be completely rewritten.

TEST=message_center_unittests
BUG=805208

Change-Id: I1a637cef8bdcb25be45647f487f1d29ab3952583
Reviewed-on: https://chromium-review.googlesource.com/952822Reviewed-by: Evan Stade <estade@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542686}

ui/message_center/views/message_popup_collection_unittest.cc

@@ -18,6 +18,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/compositor/scoped_animation_duration_scale_mode.h"
 #include "ui/display/display.h"
 #include "ui/events/event.h"
 #include "ui/events/event_constants.h"
@@ -29,6 +30,8 @@
 #include "ui/message_center/fake_message_center.h"
 #include "ui/message_center/public/cpp/message_center_constants.h"
 #include "ui/message_center/views/desktop_popup_alignment_delegate.h"
+#include "ui/message_center/views/message_view.h"
+#include "ui/message_center/views/slide_out_controller.h"
 #include "ui/message_center/views/toast_contents_view.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
@@ -99,6 +102,8 @@ class MessagePopupCollectionTest : public views::ViewsTestBase {
  protected:
   MessagePopupCollection* collection() { return collection_.get(); }
 
+  int GetDeferCounter() { return collection_->defer_counter_; }
+
   size_t GetToastCounts() {
     return collection_->toasts_.size();
   }
@@ -139,6 +144,15 @@ class MessagePopupCollectionTest : public views::ViewsTestBase {
     return NULL;
   }
 
+  void SlideOutToast(const std::string& id) {
+    ui::ScopedAnimationDurationScaleMode zero_duration_scope(
+        ui::ScopedAnimationDurationScaleMode::ZERO_DURATION);
+    ui::GestureEventDetails details(ui::ET_SCROLL_FLING_START, 1000.0f, 0.f);
+    ui::GestureEvent event(0, 0, ui::EF_NONE, ui::EventTimeForNow(), details);
+    static_cast<MessageView*>(GetToast(id)->child_at(0))
+        ->slide_out_controller_.OnGestureEvent(&event);
+  }
+
   std::string AddNotification() {
     std::string id = base::IntToString(id_++);
     std::unique_ptr<Notification> notification = std::make_unique<Notification>(
@@ -822,5 +836,18 @@ TEST_F(MessagePopupCollectionTest, AddedAndRemovedAtSameTime) {
 }
 #endif
 
+TEST_F(MessagePopupCollectionTest, RemoveBySlide) {
+  std::string id = AddNotification();
+  WaitForTransitionsDone();
+  EXPECT_EQ(0, GetDeferCounter());
+  EXPECT_EQ(1u, GetToastCounts());
+
+  SlideOutToast(id);
+  WaitForTransitionsDone();
+
+  EXPECT_EQ(0, GetDeferCounter());
+  EXPECT_EQ(0u, GetToastCounts());
+}
+
 }  // namespace test
 }  // namespace message_center

ui/message_center/views/message_view.cc

@@ -276,8 +276,17 @@ ui::Layer* MessageView::GetSlideOutLayer() {
 void MessageView::OnSlideChanged() {}
 
 void MessageView::OnSlideOut() {
-  MessageCenter::Get()->RemoveNotification(notification_id_,
-                                           true /* by_user */);
+  // As a workaround for a MessagePopupCollection bug https://crbug.com/805208,
+  // pass false to by_user although it is triggered by user.
+  // TODO(tetsui): Rewrite MessagePopupCollection and remove this hack.
+  if (pinned_) {
+    // Also a workaround to not break notification pinning.
+    MessageCenter::Get()->MarkSinglePopupAsShown(
+        notification_id_, true /* mark_notification_as_read */);
+  } else {
+    MessageCenter::Get()->RemoveNotification(notification_id_,
+                                             false /* by_user */);
+  }
 }
 
 bool MessageView::GetPinned() const {

ui/message_center/views/message_view.h

@@ -28,14 +28,17 @@ class ScrollView;
 
 namespace message_center {
 
+namespace test {
+class MessagePopupCollectionTest;
+}
+
 class Notification;
 class NotificationControlButtonsView;
 
 // An base class for a notification entry. Contains background and other
 // elements shared by derived notification views.
-class MESSAGE_CENTER_EXPORT MessageView
-    : public views::InkDropHostView,
-      public views::SlideOutController::Delegate {
+class MESSAGE_CENTER_EXPORT MessageView : public views::InkDropHostView,
+                                          public SlideOutController::Delegate {
  public:
   static const char kViewClassName[];
 
@@ -88,7 +91,7 @@ class MESSAGE_CENTER_EXPORT MessageView
   const char* GetClassName() const override;
   void OnGestureEvent(ui::GestureEvent* event) override;
 
-  // views::SlideOutController::Delegate
+  // message_center::SlideOutController::Delegate
   ui::Layer* GetSlideOutLayer() override;
   void OnSlideChanged() override;
   void OnSlideOut() override;
@@ -114,6 +117,8 @@ class MESSAGE_CENTER_EXPORT MessageView
   bool is_nested() const { return is_nested_; }
 
  private:
+  friend class test::MessagePopupCollectionTest;
+
   std::string notification_id_;
   views::View* background_view_ = nullptr;  // Owned by views hierarchy.
   views::ScrollView* scroller_ = nullptr;
@@ -125,7 +130,7 @@ class MESSAGE_CENTER_EXPORT MessageView
 
   std::unique_ptr<views::Painter> focus_painter_;
 
-  views::SlideOutController slide_out_controller_;
+  message_center::SlideOutController slide_out_controller_;
 
   // True if |this| is embedded in another view. Equivalent to |!top_level| in
   // MessageViewFactory parlance.

ui/message_center/views/slide_out_controller.cc

@@ -8,7 +8,7 @@
 #include "ui/compositor/scoped_layer_animation_settings.h"
 #include "ui/gfx/transform.h"
 
-namespace views {
+namespace message_center {
 
 SlideOutController::SlideOutController(ui::EventTarget* target,
                                        Delegate* delegate)
@@ -108,4 +108,4 @@ void SlideOutController::OnImplicitAnimationsCompleted() {
   delegate_->OnSlideOut();
 }
 
-}  // namespace views
+}  // namespace message_center

ui/message_center/views/slide_out_controller.h

@@ -8,15 +8,16 @@
 #include "base/macros.h"
 #include "ui/compositor/layer_animation_observer.h"
 #include "ui/events/scoped_target_handler.h"
+#include "ui/message_center/message_center_export.h"
 #include "ui/views/view.h"
-#include "ui/views/views_export.h"
 
-namespace views {
+namespace message_center {
 
 // This class contains logic to control sliding out of a layer in response to
 // swipes, i.e. gesture scroll events.
-class SlideOutController : public ui::EventHandler,
-                           public ui::ImplicitAnimationObserver {
+class MESSAGE_CENTER_EXPORT SlideOutController
+    : public ui::EventHandler,
+      public ui::ImplicitAnimationObserver {
  public:
   class Delegate {
    public:
@@ -59,6 +60,6 @@ class SlideOutController : public ui::EventHandler,
   DISALLOW_COPY_AND_ASSIGN(SlideOutController);
 };
 
-}  // namespace views
+}  // namespace message_center
 
 #endif  // UI_MESSAGE_CENTER_VIEWS_SLIDE_OUT_CONTROLLER_H_