Attempt at making HandleWatcher tests less flakey

HandleWatcher was using a TickClock to get now but not
MessagePumpMojo. This meant if a test set a TickClock both places
would not use it, resulting in two different times being used.

BUG=none
TEST=none
R=viettrungluu@chromium.org

Review URL: https://codereview.chromium.org/196473024

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

6 files changed, 80 insertions, 31 deletions

diff --git a/mojo/common/handle_watcher.cc b/mojo/common/handle_watcher.cc
index 2e8f083..150305c 100644
--- a/mojo/common/handle_watcher.cc
+++ b/mojo/common/handle_watcher.cc
@@ -13,10 +13,10 @@
 #include "base/message_loop/message_loop.h"
 #include "base/message_loop/message_loop_proxy.h"
 #include "base/threading/thread.h"
-#include "base/time/tick_clock.h"
 #include "base/time/time.h"
 #include "mojo/common/message_pump_mojo.h"
 #include "mojo/common/message_pump_mojo_handler.h"
+#include "mojo/common/time_helper.h"
 
 namespace mojo {
 namespace common {
@@ -35,6 +35,11 @@ scoped_ptr<base::MessagePump> CreateMessagePumpMojo() {
   return scoped_ptr<base::MessagePump>(message_pump_mojo).Pass();
 }
 
+base::TimeTicks MojoDeadlineToTimeTicks(MojoDeadline deadline) {
+  return deadline == MOJO_DEADLINE_INDEFINITE ? base::TimeTicks() :
+      internal::NowTicks() + base::TimeDelta::FromMicroseconds(deadline);
+}
+
 // Tracks the data for a single call to Start().
 struct WatchData {
   WatchData()
@@ -257,9 +262,6 @@ struct HandleWatcher::StartState {
 
 // HandleWatcher ---------------------------------------------------------------
 
-// static
-base::TickClock* HandleWatcher::tick_clock_ = NULL;
-
 HandleWatcher::HandleWatcher() {
 }
 
@@ -303,16 +305,5 @@ void HandleWatcher::OnHandleReady(MojoResult result) {
   // NOTE: We may have been deleted during callback execution.
 }
 
-// static
-base::TimeTicks HandleWatcher::NowTicks() {
-  return tick_clock_ ? tick_clock_->NowTicks() : base::TimeTicks::Now();
-}
-
-// static
-base::TimeTicks HandleWatcher::MojoDeadlineToTimeTicks(MojoDeadline deadline) {
-  return deadline == MOJO_DEADLINE_INDEFINITE ? base::TimeTicks() :
-      NowTicks() + base::TimeDelta::FromMicroseconds(deadline);
-}
-
 }  // namespace common
 }  // namespace mojo
diff --git a/mojo/common/handle_watcher.h b/mojo/common/handle_watcher.h
index 6e77dbd1..115ac4a 100644
--- a/mojo/common/handle_watcher.h
+++ b/mojo/common/handle_watcher.h
@@ -8,13 +8,12 @@
 #include "base/basictypes.h"
 #include "base/callback_forward.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/run_loop.h"
 #include "mojo/common/mojo_common_export.h"
 #include "mojo/public/system/core_cpp.h"
 
 namespace base {
 class Thread;
-class TickClock;
-class TimeTicks;
 }
 
 namespace mojo {
@@ -43,14 +42,7 @@ class MOJO_COMMON_EXPORT HandleWatcher {
   // Stops listening. Does nothing if not in the process of listening.
   void Stop();
 
-  // Returns now. Used internally; generally not useful.
-  static base::TimeTicks NowTicks();
-
-  // Converts a MojoDeadline into a TimeTicks.
-  static base::TimeTicks MojoDeadlineToTimeTicks(MojoDeadline deadline);
-
  private:
-  friend class test::HandleWatcherTest;
   struct StartState;
 
   // See description of |StartState::weak_factory| for details.
@@ -59,9 +51,6 @@ class MOJO_COMMON_EXPORT HandleWatcher {
   // If non-NULL Start() has been invoked.
   scoped_ptr<StartState> start_state_;
 
-  // Used for getting the time. Only set by tests.
-  static base::TickClock* tick_clock_;
-
   DISALLOW_COPY_AND_ASSIGN(HandleWatcher);
 };
 
diff --git a/mojo/common/handle_watcher_unittest.cc b/mojo/common/handle_watcher_unittest.cc
index ba71ca4..4fc03b1 100644
--- a/mojo/common/handle_watcher_unittest.cc
+++ b/mojo/common/handle_watcher_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/run_loop.h"
 #include "base/test/simple_test_tick_clock.h"
+#include "mojo/common/time_helper.h"
 #include "mojo/public/system/core_cpp.h"
 #include "mojo/public/tests/test_utils.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -96,12 +97,12 @@ class HandleWatcherTest : public testing::Test {
  public:
   HandleWatcherTest() {}
   virtual ~HandleWatcherTest() {
-    HandleWatcher::tick_clock_ = NULL;
+    test::SetTickClockForTest(NULL);
   }
 
  protected:
   void InstallTickClock() {
-    HandleWatcher::tick_clock_ = &tick_clock_;
+    test::SetTickClockForTest(&tick_clock_);
   }
 
   base::SimpleTestTickClock tick_clock_;
diff --git a/mojo/common/message_pump_mojo.cc b/mojo/common/message_pump_mojo.cc
index 437be7f..2b920f9 100644
--- a/mojo/common/message_pump_mojo.cc
+++ b/mojo/common/message_pump_mojo.cc
@@ -10,6 +10,7 @@
 #include "base/logging.h"
 #include "base/time/time.h"
 #include "mojo/common/message_pump_mojo_handler.h"
+#include "mojo/common/time_helper.h"
 
 namespace mojo {
 namespace common {
@@ -147,7 +148,7 @@ void MessagePumpMojo::DoInternalWork(bool block) {
   // Notify and remove any handlers whose time has expired. Make a copy in case
   // someone tries to add/remove new handlers from notification.
   const HandleToHandler cloned_handlers(handlers_);
-  const base::TimeTicks now(base::TimeTicks::Now());
+  const base::TimeTicks now(internal::NowTicks());
   for (HandleToHandler::const_iterator i = cloned_handlers.begin();
        i != cloned_handlers.end(); ++i) {
     // Since we're iterating over a clone of the handlers, verify the handler is
@@ -213,7 +214,7 @@ MojoDeadline MessagePumpMojo::GetDeadlineForWait() const {
   return min_time.is_null() ? MOJO_DEADLINE_INDEFINITE :
       std::max(static_cast<MojoDeadline>(0),
                static_cast<MojoDeadline>(
-                   (min_time - base::TimeTicks::Now()).InMicroseconds()));
+                   (min_time - internal::NowTicks()).InMicroseconds()));
 }
 
 }  // namespace common
diff --git a/mojo/common/time_helper.cc b/mojo/common/time_helper.cc
new file mode 100644
index 0000000..36fd087
--- /dev/null
+++ b/mojo/common/time_helper.cc
@@ -0,0 +1,33 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "mojo/common/time_helper.h"
+
+#include "base/time/tick_clock.h"
+
+namespace mojo {
+namespace common {
+
+namespace {
+
+base::TickClock* tick_clock = NULL;
+
+}  // namespace
+
+namespace test {
+
+void SetTickClockForTest(base::TickClock* clock) {
+  tick_clock = clock;
+}
+}  // namespace test
+
+namespace internal {
+
+base::TimeTicks NowTicks() {
+  return tick_clock ? tick_clock->NowTicks() : base::TimeTicks::Now();
+}
+
+}  // namespace internal
+}  // namespace common
+}  // namespace mojo
diff --git a/mojo/common/time_helper.h b/mojo/common/time_helper.h
new file mode 100644
index 0000000..365ae04
--- /dev/null
+++ b/mojo/common/time_helper.h
@@ -0,0 +1,34 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef MOJO_COMMON_TIME_HELPER_H_
+#define MOJO_COMMON_TIME_HELPER_H_
+
+#include "base/time/time.h"
+#include "mojo/common/mojo_common_export.h"
+
+namespace base {
+class TickClock;
+}
+
+namespace mojo {
+namespace common {
+namespace test {
+
+// Sets the TickClock used for getting TimeTicks::Now(). This is currently used
+// by both HandleWatcher and MessagePumpMojo.
+MOJO_COMMON_EXPORT void SetTickClockForTest(base::TickClock* clock);
+
+}  // namespace test
+
+namespace internal {
+
+// Returns now. Used internally; generally not useful.
+MOJO_COMMON_EXPORT base::TimeTicks NowTicks();
+
+}  // namespace internal
+}  // namespace common
+}  // namespace mojo
+
+#endif  // MOJO_COMMON_TIME_HELPER_H_