Refactor BaseTimer to avoid spamming the MessageLoop with orphaned tasks.

This change maintains the same API promises*, but instead of orphaning tasks when they are stopped, the BaseTimer_Helper class holds on to the task until either (1) it expires or (2) the user requests a delay that would arrive earlier than the pending task. If the user requests a longer delay than the pending task, a followup task will be posted when the pending task fires to span the remaining time.

* The one change of usage is related to threading. The threading requirements are now more strict. It is not allowed to destruct a timer on a different thread than the one used to post tasks. A thread ID DCHECK is now in place that will help catch misuse. Some existing instances are changed as part of this CL.

A side effect of this change is that the BaseTimer and DelayTimer are simplified to use features of BaseTimer_Helper (which is now called Timer).

As suggested in timer.h, I ran the disabled TimerTest tests from linux, and they pass consistently. I also added some new tests to verify correct run states.

BUG=117451,103667,119714,119750

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128412

Reverted: http://src.chromium.org/viewvc/chrome?view=rev&revision=128506

Review URL: https://chromiumcodereview.appspot.com/9655006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@128993 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 123 insertions, 0 deletions

diff --git a/base/timer_unittest.cc b/base/timer_unittest.cc
index 929bd38..71a752e 100644
--- a/base/timer_unittest.cc
+++ b/base/timer_unittest.cc
@@ -352,3 +352,126 @@ TEST(TimerTest, MessageLoopShutdown) {
 
   EXPECT_FALSE(did_run);
 }
+
+void TimerTestCallback() {
+}
+
+TEST(TimerTest, NonRepeatIsRunning) {
+  {
+    MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+    base::Timer timer(false, false);
+    EXPECT_FALSE(timer.IsRunning());
+    timer.Start(FROM_HERE, TimeDelta::FromDays(1),
+                base::Bind(&TimerTestCallback));
+    EXPECT_TRUE(timer.IsRunning());
+    timer.Stop();
+    EXPECT_FALSE(timer.IsRunning());
+    EXPECT_TRUE(timer.user_task().is_null());
+  }
+
+  {
+    base::Timer timer(true, false);
+    MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+    EXPECT_FALSE(timer.IsRunning());
+    timer.Start(FROM_HERE, TimeDelta::FromDays(1),
+                base::Bind(&TimerTestCallback));
+    EXPECT_TRUE(timer.IsRunning());
+    timer.Stop();
+    EXPECT_FALSE(timer.IsRunning());
+    ASSERT_FALSE(timer.user_task().is_null());
+    timer.Reset();
+    EXPECT_TRUE(timer.IsRunning());
+  }
+}
+
+TEST(TimerTest, NonRepeatMessageLoopDeath) {
+  base::Timer timer(false, false);
+  {
+    MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+    EXPECT_FALSE(timer.IsRunning());
+    timer.Start(FROM_HERE, TimeDelta::FromDays(1),
+                base::Bind(&TimerTestCallback));
+    EXPECT_TRUE(timer.IsRunning());
+  }
+  EXPECT_FALSE(timer.IsRunning());
+  EXPECT_TRUE(timer.user_task().is_null());
+}
+
+TEST(TimerTest, RetainRepeatIsRunning) {
+  MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+  base::Timer timer(FROM_HERE, TimeDelta::FromDays(1),
+                    base::Bind(&TimerTestCallback), true);
+  EXPECT_FALSE(timer.IsRunning());
+  timer.Reset();
+  EXPECT_TRUE(timer.IsRunning());
+  timer.Stop();
+  EXPECT_FALSE(timer.IsRunning());
+  timer.Reset();
+  EXPECT_TRUE(timer.IsRunning());
+}
+
+TEST(TimerTest, RetainNonRepeatIsRunning) {
+  MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+  base::Timer timer(FROM_HERE, TimeDelta::FromDays(1),
+                    base::Bind(&TimerTestCallback), false);
+  EXPECT_FALSE(timer.IsRunning());
+  timer.Reset();
+  EXPECT_TRUE(timer.IsRunning());
+  timer.Stop();
+  EXPECT_FALSE(timer.IsRunning());
+  timer.Reset();
+  EXPECT_TRUE(timer.IsRunning());
+}
+
+namespace {
+
+bool g_callback_happened1 = false;
+bool g_callback_happened2 = false;
+
+void ClearAllCallbackHappened() {
+  g_callback_happened1 = false;
+  g_callback_happened2 = false;
+}
+
+void SetCallbackHappened1() {
+  g_callback_happened1 = true;
+  MessageLoop::current()->Quit();
+}
+
+void SetCallbackHappened2() {
+  g_callback_happened2 = true;
+  MessageLoop::current()->Quit();
+}
+
+TEST(TimerTest, ContinuationStopStart) {
+  {
+    ClearAllCallbackHappened();
+    MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+    base::Timer timer(false, false);
+    timer.Start(FROM_HERE, TimeDelta::FromMilliseconds(10),
+                base::Bind(&SetCallbackHappened1));
+    timer.Stop();
+    timer.Start(FROM_HERE, TimeDelta::FromMilliseconds(40),
+                base::Bind(&SetCallbackHappened2));
+    MessageLoop::current()->Run();
+    EXPECT_FALSE(g_callback_happened1);
+    EXPECT_TRUE(g_callback_happened2);
+  }
+}
+
+TEST(TimerTest, ContinuationReset) {
+  {
+    ClearAllCallbackHappened();
+    MessageLoop loop(MessageLoop::TYPE_DEFAULT);
+    base::Timer timer(false, false);
+    timer.Start(FROM_HERE, TimeDelta::FromMilliseconds(10),
+                base::Bind(&SetCallbackHappened1));
+    timer.Reset();
+    // Since Reset happened before task ran, the user_task must not be cleared:
+    ASSERT_FALSE(timer.user_task().is_null());
+    MessageLoop::current()->Run();
+    EXPECT_TRUE(g_callback_happened1);
+  }
+}
+
+}  // namespace