C++ readability review

components/timers/alarm_timer_unittest.cc

Index: components/timers/alarm_timer_unittest.cc
diff --git a/components/timers/alarm_timer_unittest.cc b/components/timers/alarm_timer_unittest.cc
index efa88d4229e813c28e5b991b915cc4492836a99a..3e11e868d611c4f217e5549c23d8ddfbd4c16461 100644
--- a/components/timers/alarm_timer_unittest.cc
+++ b/components/timers/alarm_timer_unittest.cc
@@ -9,7 +9,7 @@
 #include "base/run_loop.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
-#include "components/timers/alarm_timer.h"
+#include "components/timers/alarm_timer_chromeos.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 // Most of these tests have been lifted right out of timer_unittest.cc with only
@@ -24,10 +24,10 @@ namespace timers {
 namespace {
 // The message loops on which each timer should be tested.
 const base::MessageLoop::Type testing_message_loops[] = {
-  base::MessageLoop::TYPE_DEFAULT,
-  base::MessageLoop::TYPE_IO,
+    base::MessageLoop::TYPE_DEFAULT,
+    base::MessageLoop::TYPE_IO,
 #if !defined(OS_IOS)  // iOS does not allow direct running of the UI loop.
-  base::MessageLoop::TYPE_UI,
+    base::MessageLoop::TYPE_UI,
 #endif
 };
 
@@ -39,13 +39,10 @@ class OneShotAlarmTimerTester {
   OneShotAlarmTimerTester(bool* did_run, base::TimeDelta delay)
       : did_run_(did_run),
         delay_(delay),
-        timer_(new timers::AlarmTimer(false, false)) {
-  }
+        timer_(new timers::OneShotAlarmTimer()) {}
   void Start() {
-    timer_->Start(FROM_HERE,
-                  delay_,
-                  base::Bind(&OneShotAlarmTimerTester::Run,
-                             base::Unretained(this)));
+    timer_->Start(FROM_HERE, delay_, base::Bind(&OneShotAlarmTimerTester::Run,
+                                                base::Unretained(this)));
   }
 
  private:
@@ -58,7 +55,7 @@ class OneShotAlarmTimerTester {
 
   bool* did_run_;
   const base::TimeDelta delay_;
-  scoped_ptr<timers::AlarmTimer> timer_;
+  scoped_ptr<timers::OneShotAlarmTimer> timer_;
 
   DISALLOW_COPY_AND_ASSIGN(OneShotAlarmTimerTester);
 };
@@ -68,11 +65,9 @@ class OneShotSelfDeletingAlarmTimerTester {
   OneShotSelfDeletingAlarmTimerTester(bool* did_run, base::TimeDelta delay)
       : did_run_(did_run),
         delay_(delay),
-        timer_(new timers::AlarmTimer(false, false)) {
-  }
+        timer_(new timers::OneShotAlarmTimer()) {}
   void Start() {
-    timer_->Start(FROM_HERE,
-                  delay_,
+    timer_->Start(FROM_HERE, delay_,
                   base::Bind(&OneShotSelfDeletingAlarmTimerTester::Run,
                              base::Unretained(this)));
   }
@@ -88,7 +83,7 @@ class OneShotSelfDeletingAlarmTimerTester {
 
   bool* did_run_;
   const base::TimeDelta delay_;
-  scoped_ptr<timers::AlarmTimer> timer_;
+  scoped_ptr<timers::OneShotAlarmTimer> timer_;
 
   DISALLOW_COPY_AND_ASSIGN(OneShotSelfDeletingAlarmTimerTester);
 };
@@ -99,13 +94,10 @@ class RepeatingAlarmTimerTester {
       : did_run_(did_run),
         delay_(delay),
         counter_(10),
-        timer_(new timers::AlarmTimer(true, true)) {
-  }
+        timer_(new timers::RepeatingAlarmTimer()) {}
   void Start() {
-    timer_->Start(FROM_HERE,
-                  delay_,
-                  base::Bind(&RepeatingAlarmTimerTester::Run,
-                             base::Unretained(this)));
+    timer_->Start(FROM_HERE, delay_, base::Bind(&RepeatingAlarmTimerTester::Run,
+                                                base::Unretained(this)));
   }
 
  private:
@@ -122,148 +114,147 @@ class RepeatingAlarmTimerTester {
   bool* did_run_;
   const base::TimeDelta delay_;
   int counter_;
-  scoped_ptr<timers::AlarmTimer> timer_;
+  scoped_ptr<timers::RepeatingAlarmTimer> timer_;
 
   DISALLOW_COPY_AND_ASSIGN(RepeatingAlarmTimerTester);
 };
 
-void RunTest_OneShotAlarmTimer(base::MessageLoop::Type message_loop_type) {
-  base::MessageLoop loop(message_loop_type);
+}  // namespace
 
-  bool did_run = false;
-  OneShotAlarmTimerTester f(&did_run, kTenMilliseconds);
-  f.Start();
+//-----------------------------------------------------------------------------
+// Each test is run against each type of MessageLoop.  That way we are sure
+// that timers work properly in all configurations.
 
-  base::RunLoop().Run();
+TEST(AlarmTimerTest, OneShotAlarmTimer) {
+  for (int i = 0; i < kNumTestingMessageLoops; i++) {
+    base::MessageLoop loop(testing_message_loops[i]);
+
+    bool did_run = false;
+    OneShotAlarmTimerTester f(&did_run, kTenMilliseconds);
+    f.Start();
 
-  EXPECT_TRUE(did_run);
+    base::RunLoop().Run();
+
+    EXPECT_TRUE(did_run);
+  }
 }
 
-void RunTest_OneShotAlarmTimer_Cancel(
-    base::MessageLoop::Type message_loop_type) {
-  base::MessageLoop loop(message_loop_type);
+TEST(AlarmTimerTest, OneShotAlarmTimer_Cancel) {
+  for (int i = 0; i < kNumTestingMessageLoops; i++) {
+    base::MessageLoop loop(testing_message_loops[i]);
 
-  bool did_run_a = false;
-  OneShotAlarmTimerTester* a = new OneShotAlarmTimerTester(&did_run_a,
-                                                           kTenMilliseconds);
+    bool did_run_a = false;
+    OneShotAlarmTimerTester* a =
+        new OneShotAlarmTimerTester(&did_run_a, kTenMilliseconds);
 
-  // This should run before the timer expires.
-  base::MessageLoop::current()->DeleteSoon(FROM_HERE, a);
+    // This should run before the timer expires.
+    base::MessageLoop::current()->DeleteSoon(FROM_HERE, a);
 
-  // Now start the timer.
-  a->Start();
+    // Now start the timer.
+    a->Start();
 
-  bool did_run_b = false;
-  OneShotAlarmTimerTester b(&did_run_b, kTenMilliseconds);
-  b.Start();
+    bool did_run_b = false;
+    OneShotAlarmTimerTester b(&did_run_b, kTenMilliseconds);
+    b.Start();
 
-  base::RunLoop().Run();
+    base::RunLoop().Run();
 
-  EXPECT_FALSE(did_run_a);
-  EXPECT_TRUE(did_run_b);
+    EXPECT_FALSE(did_run_a);
+    EXPECT_TRUE(did_run_b);
+  }
 }
 
-void RunTest_OneShotSelfDeletingAlarmTimer(
-    base::MessageLoop::Type message_loop_type) {
-  base::MessageLoop loop(message_loop_type);
+// If underlying timer does not handle this properly, we will crash or fail
+// in full page heap environment.
+TEST(AlarmTimerTest, OneShotSelfDeletingAlarmTimer) {
+  for (int i = 0; i < kNumTestingMessageLoops; i++) {
+    base::MessageLoop loop(testing_message_loops[i]);
 
-  bool did_run = false;
-  OneShotSelfDeletingAlarmTimerTester f(&did_run, kTenMilliseconds);
-  f.Start();
+    bool did_run = false;
+    OneShotSelfDeletingAlarmTimerTester f(&did_run, kTenMilliseconds);
+    f.Start();
 
-  base::RunLoop().Run();
+    base::RunLoop().Run();
 
-  EXPECT_TRUE(did_run);
+    EXPECT_TRUE(did_run);
+  }
 }
 
-void RunTest_RepeatingAlarmTimer(base::MessageLoop::Type message_loop_type,
-                                 const base::TimeDelta& delay) {
-  base::MessageLoop loop(message_loop_type);
+TEST(AlarmTimerTest, RepeatingAlarmTimer) {
+  for (int i = 0; i < kNumTestingMessageLoops; i++) {
+    base::MessageLoop loop(testing_message_loops[i]);
 
-  bool did_run = false;
-  RepeatingAlarmTimerTester f(&did_run, delay);
-  f.Start();
+    bool did_run = false;
+    RepeatingAlarmTimerTester f(&did_run, kTenMilliseconds);
+    f.Start();
 
-  base::RunLoop().Run();
+    base::RunLoop().Run();
 
-  EXPECT_TRUE(did_run);
+    EXPECT_TRUE(did_run);
+  }
 }
 
-void RunTest_RepeatingAlarmTimer_Cancel(
-    base::MessageLoop::Type message_loop_type, const base::TimeDelta& delay) {
-  base::MessageLoop loop(message_loop_type);
+TEST(AlarmTimerTest, RepeatingAlarmTimer_Cancel) {
+  for (int i = 0; i < kNumTestingMessageLoops; i++) {
+    base::MessageLoop loop(testing_message_loops[i]);
 
-  bool did_run_a = false;
-  RepeatingAlarmTimerTester* a = new RepeatingAlarmTimerTester(&did_run_a,
-                                                               delay);
+    bool did_run_a = false;
+    RepeatingAlarmTimerTester* a =
+        new RepeatingAlarmTimerTester(&did_run_a, kTenMilliseconds);
 
-  // This should run before the timer expires.
-  base::MessageLoop::current()->DeleteSoon(FROM_HERE, a);
+    // This should run before the timer expires.
+    base::MessageLoop::current()->DeleteSoon(FROM_HERE, a);
 
-  // Now start the timer.
-  a->Start();
+    // Now start the timer.
+    a->Start();
 
-  bool did_run_b = false;
-  RepeatingAlarmTimerTester b(&did_run_b, delay);
-  b.Start();
+    bool did_run_b = false;
+    RepeatingAlarmTimerTester b(&did_run_b, kTenMilliseconds);
+    b.Start();
 
-  base::RunLoop().Run();
+    base::RunLoop().Run();
 
-  EXPECT_FALSE(did_run_a);
-  EXPECT_TRUE(did_run_b);
+    EXPECT_FALSE(did_run_a);
+    EXPECT_TRUE(did_run_b);
+  }
 }
 
-}  // namespace
+TEST(AlarmTimerTest, RepeatingAlarmTimerZeroDelay) {
+  for (int i = 0; i < kNumTestingMessageLoops; i++) {
+    base::MessageLoop loop(testing_message_loops[i]);
 
-//-----------------------------------------------------------------------------
-// Each test is run against each type of MessageLoop.  That way we are sure
-// that timers work properly in all configurations.
+    bool did_run = false;
+    RepeatingAlarmTimerTester f(&did_run, base::TimeDelta());
+    f.Start();
 
-TEST(AlarmTimerTest, OneShotAlarmTimer) {
-  for (int i = 0; i < kNumTestingMessageLoops; i++) {
-    RunTest_OneShotAlarmTimer(testing_message_loops[i]);
-  }
-}
+    base::RunLoop().Run();
 
-TEST(AlarmTimerTest, OneShotAlarmTimer_Cancel) {
-  for (int i = 0; i < kNumTestingMessageLoops; i++) {
-    RunTest_OneShotAlarmTimer_Cancel(testing_message_loops[i]);
+    EXPECT_TRUE(did_run);
   }
 }
 
-// If underlying timer does not handle this properly, we will crash or fail
-// in full page heap environment.
-TEST(AlarmTimerTest, OneShotSelfDeletingAlarmTimer) {
+TEST(AlarmTimerTest, RepeatingAlarmTimerZeroDelay_Cancel) {
   for (int i = 0; i < kNumTestingMessageLoops; i++) {
-    RunTest_OneShotSelfDeletingAlarmTimer(testing_message_loops[i]);
-  }
-}
+    base::MessageLoop loop(testing_message_loops[i]);
 
-TEST(AlarmTimerTest, RepeatingAlarmTimer) {
-  for (int i = 0; i < kNumTestingMessageLoops; i++) {
-    RunTest_RepeatingAlarmTimer(testing_message_loops[i],
-                                kTenMilliseconds);
-  }
-}
+    bool did_run_a = false;
+    RepeatingAlarmTimerTester* a =
+        new RepeatingAlarmTimerTester(&did_run_a, base::TimeDelta());
 
-TEST(AlarmTimerTest, RepeatingAlarmTimer_Cancel) {
-  for (int i = 0; i < kNumTestingMessageLoops; i++) {
-    RunTest_RepeatingAlarmTimer_Cancel(testing_message_loops[i],
-                                       kTenMilliseconds);
-  }
-}
+    // This should run before the timer expires.
+    base::MessageLoop::current()->DeleteSoon(FROM_HERE, a);
 
-TEST(AlarmTimerTest, RepeatingAlarmTimerZeroDelay) {
-  for (int i = 0; i < kNumTestingMessageLoops; i++) {
-    RunTest_RepeatingAlarmTimer(testing_message_loops[i],
-                                base::TimeDelta::FromMilliseconds(0));
-  }
-}
+    // Now start the timer.
+    a->Start();
 
-TEST(AlarmTimerTest, RepeatingAlarmTimerZeroDelay_Cancel) {
-  for (int i = 0; i < kNumTestingMessageLoops; i++) {
-    RunTest_RepeatingAlarmTimer_Cancel(testing_message_loops[i],
-                                       base::TimeDelta::FromMilliseconds(0));
+    bool did_run_b = false;
+    RepeatingAlarmTimerTester b(&did_run_b, base::TimeDelta());
+    b.Start();
+
+    base::RunLoop().Run();
+
+    EXPECT_FALSE(did_run_a);
+    EXPECT_TRUE(did_run_b);
   }
 }
 
@@ -283,7 +274,7 @@ TEST(AlarmTimerTest, MessageLoopShutdown) {
       a.Start();
       b.Start();
     }  // MessageLoop destructs by falling out of scope.
-  }  // OneShotTimers destruct.  SHOULD NOT CRASH, of course.
+  }    // OneShotTimers destruct.  SHOULD NOT CRASH, of course.
 
   EXPECT_FALSE(did_run);
 }
@@ -291,7 +282,7 @@ TEST(AlarmTimerTest, MessageLoopShutdown) {
 TEST(AlarmTimerTest, NonRepeatIsRunning) {
   {
     base::MessageLoop loop;
-    timers::AlarmTimer timer(false, false);
+    timers::OneShotAlarmTimer timer;
     EXPECT_FALSE(timer.IsRunning());
     timer.Start(FROM_HERE, base::TimeDelta::FromDays(1),
                 base::Bind(&base::DoNothing));
@@ -302,7 +293,7 @@ TEST(AlarmTimerTest, NonRepeatIsRunning) {
   }
 
   {
-    timers::AlarmTimer timer(true, false);
+    timers::SimpleAlarmTimer timer;
     base::MessageLoop loop;
     EXPECT_FALSE(timer.IsRunning());
     timer.Start(FROM_HERE, base::TimeDelta::FromDays(1),
@@ -317,7 +308,7 @@ TEST(AlarmTimerTest, NonRepeatIsRunning) {
 }
 
 TEST(AlarmTimerTest, NonRepeatMessageLoopDeath) {
-  timers::AlarmTimer timer(false, false);
+  timers::OneShotAlarmTimer timer;
   {
     base::MessageLoop loop;
     EXPECT_FALSE(timer.IsRunning());
@@ -331,8 +322,8 @@ TEST(AlarmTimerTest, NonRepeatMessageLoopDeath) {
 
 TEST(AlarmTimerTest, RetainRepeatIsRunning) {
   base::MessageLoop loop;
-  timers::AlarmTimer timer(FROM_HERE, base::TimeDelta::FromDays(1),
-                          base::Bind(&base::DoNothing), true);
+  timers::RepeatingAlarmTimer timer(FROM_HERE, base::TimeDelta::FromDays(1),
+                                    base::Bind(&base::DoNothing));
   EXPECT_FALSE(timer.IsRunning());
   timer.Reset();
   EXPECT_TRUE(timer.IsRunning());
@@ -344,8 +335,8 @@ TEST(AlarmTimerTest, RetainRepeatIsRunning) {
 
 TEST(AlarmTimerTest, RetainNonRepeatIsRunning) {
   base::MessageLoop loop;
-  timers::AlarmTimer timer(FROM_HERE, base::TimeDelta::FromDays(1),
-                          base::Bind(&base::DoNothing), false);
+  timers::SimpleAlarmTimer timer(FROM_HERE, base::TimeDelta::FromDays(1),
+                                 base::Bind(&base::DoNothing));
   EXPECT_FALSE(timer.IsRunning());
   timer.Reset();
   EXPECT_TRUE(timer.IsRunning());
@@ -381,7 +372,7 @@ TEST(AlarmTimerTest, ContinuationStopStart) {
   {
     ClearAllCallbackHappened();
     base::MessageLoop loop;
-    timers::AlarmTimer timer(false, false);
+    timers::OneShotAlarmTimer timer;
     timer.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(10),
                 base::Bind(&SetCallbackHappened1));
     timer.Stop();
@@ -397,7 +388,7 @@ TEST(AlarmTimerTest, ContinuationReset) {
   {
     ClearAllCallbackHappened();
     base::MessageLoop loop;
-    timers::AlarmTimer timer(false, false);
+    timers::OneShotAlarmTimer timer;
     timer.Start(FROM_HERE, base::TimeDelta::FromMilliseconds(10),
                 base::Bind(&SetCallbackHappened1));
     timer.Reset();
@@ -410,7 +401,6 @@ TEST(AlarmTimerTest, ContinuationReset) {
 
 }  // namespace
 
-
 namespace {
 void TimerRanCallback(bool* did_run) {
   *did_run = true;
@@ -419,75 +409,71 @@ void TimerRanCallback(bool* did_run) {
       FROM_HERE, base::MessageLoop::QuitWhenIdleClosure());
 }
 
-void RunTest_OneShotTimerConcurrentResetAndTimerFired(
-    base::MessageLoop::Type message_loop_type) {
-  base::MessageLoop loop(message_loop_type);
+TEST(AlarmTimerTest, OneShotTimerConcurrentResetAndTimerFired) {
+  for (int i = 0; i < kNumTestingMessageLoops; i++) {
+    base::MessageLoop loop(testing_message_loops[i]);
+
+    timers::OneShotAlarmTimer timer;
+    bool did_run = false;
 
-  timers::AlarmTimer timer(false, false);
-  bool did_run = false;
+    base::RunLoop run_loop;
 
-  timer.Start(
-      FROM_HERE, kTenMilliseconds, base::Bind(&TimerRanCallback, &did_run));
+    timer.SetTimerFiredCallbackForTest(run_loop.QuitClosure());
+    timer.Start(FROM_HERE, kTenMilliseconds,
+                base::Bind(&TimerRanCallback, &did_run));
 
-  // Sleep twice as long as the timer to ensure that the timer task gets queued.
-  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(20));
+    // Wait until the timer has fired and a task has been queue in the
+    // MessageLoop.
+    run_loop.Run();
 
-  // Now reset the timer.  This is attempting to simulate the timer firing and
-  // being reset at the same time.  The previously queued task should be
-  // removed.
-  timer.Reset();
+    // Now reset the timer.  This is attempting to simulate the timer firing and
+    // being reset at the same time.  The previously queued task should be
+    // removed.
+    timer.Reset();
 
-  base::RunLoop().RunUntilIdle();
-  EXPECT_FALSE(did_run);
+    base::RunLoop().RunUntilIdle();
+    EXPECT_FALSE(did_run);
 
-  // If the previous check failed, running the message loop again will hang the
-  // test so we only do it if the callback has not run yet.
-  if (!did_run) {
-    base::RunLoop().Run();
-    EXPECT_TRUE(did_run);
+    // If the previous check failed, running the message loop again will hang
+    // the test so we only do it if the callback has not run yet.
+    if (!did_run) {
+      base::RunLoop().Run();
+      EXPECT_TRUE(did_run);
+    }
   }
 }
 
-void RunTest_RepeatingTimerConcurrentResetAndTimerFired(
-    base::MessageLoop::Type message_loop_type) {
-  base::MessageLoop loop(message_loop_type);
-
-  timers::AlarmTimer timer(true, true);
-  bool did_run = false;
+TEST(AlarmTimerTest, RepeatingTimerConcurrentResetAndTimerFired) {
+  for (int i = 0; i < kNumTestingMessageLoops; i++) {
+    base::MessageLoop loop(testing_message_loops[i]);
 
-  timer.Start(
-      FROM_HERE, kTenMilliseconds, base::Bind(&TimerRanCallback, &did_run));
+    timers::RepeatingAlarmTimer timer;
+    bool did_run = false;
 
-  // Sleep more that three times as long as the timer duration to ensure that
-  // multiple tasks get queued.
-  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(35));
+    base::RunLoop run_loop;
 
-  // Now reset the timer.  This is attempting to simulate a very busy message
-  // loop where multiple tasks get queued but the timer gets reset before any of
-  // them have a chance to run.
-  timer.Reset();
+    timer.SetTimerFiredCallbackForTest(run_loop.QuitClosure());
+    timer.Start(FROM_HERE, kTenMilliseconds,
+                base::Bind(&TimerRanCallback, &did_run));
 
-  base::RunLoop().RunUntilIdle();
-  EXPECT_FALSE(did_run);
+    // Wait until the timer has fired and a task has been queue in the
+    // MessageLoop.
+    run_loop.Run();
 
-  // If the previous check failed, running the message loop again will hang the
-  // test so we only do it if the callback has not run yet.
-  if (!did_run) {
-    base::RunLoop().Run();
-    EXPECT_TRUE(did_run);
-  }
-}
+    // Now reset the timer.  This is attempting to simulate the timer firing and
+    // being reset at the same time.  The previously queued task should be
+    // removed.
+    timer.Reset();
 
-TEST(AlarmTimerTest, OneShotTimerConcurrentResetAndTimerFired) {
-  for (int i = 0; i < kNumTestingMessageLoops; i++) {
-    RunTest_OneShotTimerConcurrentResetAndTimerFired(testing_message_loops[i]);
-  }
-}
+    base::RunLoop().RunUntilIdle();
+    EXPECT_FALSE(did_run);
 
-TEST(AlarmTimerTest, RepeatingTimerConcurrentResetAndTimerFired) {
-  for (int i = 0; i < kNumTestingMessageLoops; i++) {
-    RunTest_RepeatingTimerConcurrentResetAndTimerFired(
-        testing_message_loops[i]);
+    // If the previous check failed, running the message loop again will hang
+    // the test so we only do it if the callback has not run yet.
+    if (!did_run) {
+      base::RunLoop().Run();
+      EXPECT_TRUE(did_run);
+    }
   }
 }