Reset popup dismissal timers when replacing a notification.

Replacing a notification should reset the popup dismissal timer. Right
now the timer of the original notification continues to run, which means
that a popup might disappear immediately after being updated.

Also removes some methods that were tested, but not used.

BUG=

Review-Url: https://codereview.chromium.org/2315513002
Cr-Commit-Position: refs/heads/master@{#417993}

ui/message_center/message_center_impl_unittest.cc

@@ -14,12 +14,14 @@
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/test_mock_time_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/message_center/message_center.h"
+#include "ui/message_center/message_center_style.h"
 #include "ui/message_center/message_center_types.h"
 #include "ui/message_center/notification_blocker.h"
 #include "ui/message_center/notification_types.h"
@@ -262,7 +264,6 @@ TEST_F(MessageCenterImplTest, PopupTimersEmptyController) {
   popup_timers_controller->StartAll();
   popup_timers_controller->CancelAll();
   popup_timers_controller->TimerFinished("unknown");
-  popup_timers_controller->PauseTimer("unknown");
   popup_timers_controller->CancelTimer("unknown");
 }
 
@@ -275,17 +276,6 @@ TEST_F(MessageCenterImplTest, PopupTimersControllerStartTimer) {
   EXPECT_TRUE(popup_timers_controller->timer_finished());
 }
 
-TEST_F(MessageCenterImplTest, PopupTimersControllerPauseTimer) {
-  std::unique_ptr<MockPopupTimersController> popup_timers_controller =
-      base::MakeUnique<MockPopupTimersController>(message_center(), closure());
-  popup_timers_controller->StartTimer("test",
-                                      base::TimeDelta::FromMilliseconds(1));
-  popup_timers_controller->PauseTimer("test");
-  run_loop()->RunUntilIdle();
-
-  EXPECT_FALSE(popup_timers_controller->timer_finished());
-}
-
 TEST_F(MessageCenterImplTest, PopupTimersControllerCancelTimer) {
   std::unique_ptr<MockPopupTimersController> popup_timers_controller =
       base::MakeUnique<MockPopupTimersController>(message_center(), closure());
@@ -337,40 +327,50 @@ TEST_F(MessageCenterImplTest, PopupTimersControllerStartMultipleTimers) {
   EXPECT_TRUE(popup_timers_controller->timer_finished());
 }
 
-TEST_F(MessageCenterImplTest, PopupTimersControllerStartMultipleTimersPause) {
-  std::unique_ptr<MockPopupTimersController> popup_timers_controller =
-      base::MakeUnique<MockPopupTimersController>(message_center(), closure());
-  popup_timers_controller->StartTimer("test",
-                                      base::TimeDelta::FromMilliseconds(5));
-  popup_timers_controller->StartTimer("test2",
-                                      base::TimeDelta::FromMilliseconds(1));
-  popup_timers_controller->StartTimer("test3",
-                                      base::TimeDelta::FromMilliseconds(3));
-  popup_timers_controller->PauseTimer("test2");
+TEST_F(MessageCenterImplTest, PopupTimersControllerRestartOnUpdate) {
+  scoped_refptr<base::SingleThreadTaskRunner> old_task_runner =
+      base::ThreadTaskRunnerHandle::Get();
 
-  run_loop()->Run();
+  scoped_refptr<base::TestMockTimeTaskRunner> task_runner(
+      new base::TestMockTimeTaskRunner(base::Time::Now(),
+                                       base::TimeTicks::Now()));
+  base::MessageLoop::current()->SetTaskRunner(task_runner);
 
-  EXPECT_EQ(popup_timers_controller->last_id(), "test3");
-  EXPECT_TRUE(popup_timers_controller->timer_finished());
-}
+  NotifierId notifier_id(GURL("https://example.com"));
+
+  message_center()->AddNotification(std::unique_ptr<Notification>(
+      new Notification(NOTIFICATION_TYPE_SIMPLE, "id1", UTF8ToUTF16("title"),
+                       UTF8ToUTF16("message"), gfx::Image() /* icon */,
+                       base::string16() /* display_source */, GURL(),
+                       notifier_id, RichNotificationData(), NULL)));
 
-TEST_F(MessageCenterImplTest, PopupTimersControllerResetTimer) {
   std::unique_ptr<MockPopupTimersController> popup_timers_controller =
       base::MakeUnique<MockPopupTimersController>(message_center(), closure());
-  popup_timers_controller->StartTimer("test",
-                                      base::TimeDelta::FromMilliseconds(5));
-  popup_timers_controller->StartTimer("test2",
-                                      base::TimeDelta::FromMilliseconds(1));
-  popup_timers_controller->StartTimer("test3",
-                                      base::TimeDelta::FromMilliseconds(3));
-  popup_timers_controller->PauseTimer("test2");
-  popup_timers_controller->ResetTimer("test",
-                                      base::TimeDelta::FromMilliseconds(2));
 
-  run_loop()->Run();
+  popup_timers_controller->OnNotificationDisplayed("id1", DISPLAY_SOURCE_POPUP);
+  ASSERT_FALSE(popup_timers_controller->timer_finished());
 
-  EXPECT_EQ(popup_timers_controller->last_id(), "test");
-  EXPECT_TRUE(popup_timers_controller->timer_finished());
+  // Fast forward the |task_runner| by one second less than the auto-close timer
+  // frequency for Web Notifications. (As set by the |notifier_id|.)
+  task_runner->FastForwardBy(
+      base::TimeDelta::FromSeconds(kAutocloseWebPageDelaySeconds - 1));
+  ASSERT_FALSE(popup_timers_controller->timer_finished());
+
+  // Trigger a replacement of the notification in the timer controller.
+  popup_timers_controller->OnNotificationUpdated("id1");
+
+  // Fast forward the |task_runner| by one second less than the auto-close timer
+  // frequency for Web Notifications again. It should have been reset.
+  task_runner->FastForwardBy(
+      base::TimeDelta::FromSeconds(kAutocloseWebPageDelaySeconds - 1));
+  ASSERT_FALSE(popup_timers_controller->timer_finished());
+
+  // Now fast forward the |task_runner| by two seconds (to avoid flakiness),
+  // after which the timer should have fired.
+  task_runner->FastForwardBy(base::TimeDelta::FromSeconds(2));
+  ASSERT_TRUE(popup_timers_controller->timer_finished());
+
+  base::MessageLoop::current()->SetTaskRunner(old_task_runner);
 }
 
 TEST_F(MessageCenterImplTest, NotificationBlocker) {

ui/message_center/popup_timer.cc

@@ -12,41 +12,34 @@ PopupTimer::PopupTimer(const std::string& id,
     : id_(id),
       timeout_(timeout),
       timer_delegate_(delegate),
-      timer_(new base::OneShotTimer) {}
+      timer_(new base::OneShotTimer) {
+  DCHECK(timer_delegate_);
+}
 
 PopupTimer::~PopupTimer() {
-  if (!timer_)
-    return;
-
   if (timer_->IsRunning())
     timer_->Stop();
 }
 
 void PopupTimer::Start() {
   if (timer_->IsRunning())
     return;
+
   base::TimeDelta timeout_to_close =
       timeout_ <= passed_ ? base::TimeDelta() : timeout_ - passed_;
   start_time_ = base::Time::Now();
 
-  DCHECK(timer_delegate_);
   timer_->Start(
       FROM_HERE, timeout_to_close,
       base::Bind(&Delegate::TimerFinished, timer_delegate_, id_));
 }
 
 void PopupTimer::Pause() {
-  if (!timer_ || !timer_->IsRunning())
+  if (!timer_->IsRunning())
     return;
 
   timer_->Stop();
   passed_ += base::Time::Now() - start_time_;
 }
 
-void PopupTimer::Reset() {
-  if (timer_)
-    timer_->Stop();
-  passed_ = base::TimeDelta();
-}
-
 }  // namespace message_center

ui/message_center/popup_timer.h

@@ -41,12 +41,6 @@ class PopupTimer {
   // subsequent calls to Start the timer will continue where it left off.
   void Pause();
 
-  // Stops the timer, and resets the amount of time that has passed so that
-  // calling Start results in a timeout equal to the initial timeout setting.
-  void Reset();
-
-  base::TimeDelta get_timeout() const { return timeout_; }
-
  private:
   // Notification ID for which this timer applies.
   const std::string id_;

ui/message_center/popup_timers_controller.cc

@@ -52,19 +52,6 @@ void PopupTimersController::StartAll() {
     iter.second->Start();
 }
 
-void PopupTimersController::ResetTimer(const std::string& id,
-                                       const base::TimeDelta& timeout) {
-  CancelTimer(id);
-  StartTimer(id, timeout);
-}
-
-void PopupTimersController::PauseTimer(const std::string& id) {
-  PopupTimerCollection::const_iterator iter = popup_timers_.find(id);
-  if (iter == popup_timers_.end())
-    return;
-  iter->second->Pause();
-}
-
 void PopupTimersController::PauseAll() {
   for (const auto& iter : popup_timers_)
     iter.second->Pause();
@@ -113,9 +100,8 @@ void PopupTimersController::OnNotificationUpdated(const std::string& id) {
     return;
   }
 
-  // Start the timer if not yet.
-  if (popup_timers_.find(id) == popup_timers_.end())
-    StartTimer(id, GetTimeoutForNotification(*iter));
+  CancelTimer(id);
+  StartTimer(id, GetTimeoutForNotification(*iter));
 }
 
 void PopupTimersController::OnNotificationRemoved(const std::string& id,

ui/message_center/popup_timers_controller.h

@@ -53,14 +53,6 @@ class MESSAGE_CENTER_EXPORT PopupTimersController
   void StartTimer(const std::string& id,
                   const base::TimeDelta& timeout_in_seconds);
 
-  // Stops a single timer, reverts it to a new timeout, and restarts it.
-  void ResetTimer(const std::string& id,
-                  const base::TimeDelta& timeout_in_seconds);
-
-  // Pauses a single timer, such that it will continue where it left off after a
-  // call to StartAll or StartTimer.
-  void PauseTimer(const std::string& id);
-
   // Removes and cancels a single popup timer, if it exists.
   void CancelTimer(const std::string& id);