From f44467004569a9c1c9743c93543ef356c2634665 Mon Sep 17 00:00:00 2001
From: isherman <isherman@chromium.org>
Date: Thu, 22 Jan 2015 22:33:14 -0800
Subject: [PATCH] [Clean-up] Replace the final straggler use of
 MockTimeProvider with Clock.

Clock is the newer interface; MockTimeProvider is deprecated.

BUG=none
TEST=unit_tests
R=pkasting@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#312786}
---
 base/base.gyp                                      |  2 -
 base/test/BUILD.gn                                 |  2 -
 base/test/mock_time_provider.cc                    | 31 ----------
 base/test/mock_time_provider.h                     | 69 ----------------------
 components/search_engines/template_url_service.cc  |  7 ++-
 components/search_engines/template_url_service.h   | 13 ++--
 .../template_url_service_unittest.cc               | 43 +++++++++-----
 7 files changed, 36 insertions(+), 131 deletions(-)
 delete mode 100644 base/test/mock_time_provider.cc
 delete mode 100644 base/test/mock_time_provider.h

diff --git a/base/base.gyp b/base/base.gyp
index 114cad64ac16..6b7d0dae4b9a 100644
--- a/base/base.gyp
+++ b/base/base.gyp
@@ -929,8 +929,6 @@
         'test/mock_chrome_application_mac.mm',
         'test/mock_devices_changed_observer.cc',
         'test/mock_devices_changed_observer.h',
-        'test/mock_time_provider.cc',
-        'test/mock_time_provider.h',
         'test/multiprocess_test.cc',
         'test/multiprocess_test.h',
         'test/multiprocess_test_android.cc',
diff --git a/base/test/BUILD.gn b/base/test/BUILD.gn
index b4987d9d5583..cca72dd25e1b 100644
--- a/base/test/BUILD.gn
+++ b/base/test/BUILD.gn
@@ -36,8 +36,6 @@ source_set("test_support") {
     "mock_chrome_application_mac.mm",
     "mock_devices_changed_observer.cc",
     "mock_devices_changed_observer.h",
-    "mock_time_provider.cc",
-    "mock_time_provider.h",
     "multiprocess_test.cc",
     "multiprocess_test.h",
     "multiprocess_test_android.cc",
diff --git a/base/test/mock_time_provider.cc b/base/test/mock_time_provider.cc
deleted file mode 100644
index 9e5547fcd941..000000000000
--- a/base/test/mock_time_provider.cc
+++ /dev/null
@@ -1,31 +0,0 @@
-// Copyright (c) 2011 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 "base/logging.h"
-#include "base/test/mock_time_provider.h"
-
-using ::testing::DefaultValue;
-
-namespace base {
-
-MockTimeProvider* MockTimeProvider::instance_ = NULL;
-
-MockTimeProvider::MockTimeProvider() {
-  DCHECK(!instance_) << "Only one instance of MockTimeProvider can exist";
-  DCHECK(!DefaultValue<Time>::IsSet());
-  instance_ = this;
-  DefaultValue<Time>::Set(Time::FromInternalValue(0));
-}
-
-MockTimeProvider::~MockTimeProvider() {
-  instance_ = NULL;
-  DefaultValue<Time>::Clear();
-}
-
-// static
-Time MockTimeProvider::StaticNow() {
-  return instance_->Now();
-}
-
-}  // namespace base
diff --git a/base/test/mock_time_provider.h b/base/test/mock_time_provider.h
deleted file mode 100644
index 7c58648ecd22..000000000000
--- a/base/test/mock_time_provider.h
+++ /dev/null
@@ -1,69 +0,0 @@
-// Copyright (c) 2011 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.
-
-// TODO(akalin): Change all users of this class to use SimpleTestClock
-// or SimpleTestTickClock and remove this class.
-
-// A helper class used to mock out calls to the static method base::Time::Now.
-//
-// Example usage:
-//
-// typedef base::Time(TimeProvider)();
-// class StopWatch {
-//  public:
-//   StopWatch(TimeProvider* time_provider);
-//   void Start();
-//   base::TimeDelta Stop();
-//  private:
-//   TimeProvider* time_provider_;
-//   ...
-// }
-//
-// Normally, you would instantiate a StopWatch with the real Now function:
-//
-// StopWatch watch(&base::Time::Now);
-//
-// But when testing, you want to instantiate it with
-// MockTimeProvider::StaticNow, which calls an internally mocked out member.
-// This allows you to set expectations on the Now method. For example:
-//
-// TEST_F(StopWatchTest, BasicTest) {
-//   InSequence s;
-//   StrictMock<MockTimeProvider> mock_time;
-//   EXPECT_CALL(mock_time, Now())
-//       .WillOnce(Return(Time::FromDoubleT(4)));
-//   EXPECT_CALL(mock_time, Now())
-//       .WillOnce(Return(Time::FromDoubleT(10)));
-//
-//   StopWatch sw(&MockTimeProvider::StaticNow);
-//   sw.Start();  // First call to Now.
-//   TimeDelta elapsed = sw.stop();  // Second call to Now.
-//   ASSERT_EQ(elapsed, TimeDelta::FromSeconds(6));
-// }
-
-#ifndef BASE_TEST_MOCK_TIME_PROVIDER_H_
-#define BASE_TEST_MOCK_TIME_PROVIDER_H_
-
-#include "base/time/time.h"
-#include "testing/gmock/include/gmock/gmock.h"
-
-namespace base {
-
-class MockTimeProvider {
- public:
-  MockTimeProvider();
-  ~MockTimeProvider();
-
-  MOCK_METHOD0(Now, Time());
-
-  static Time StaticNow();
-
- private:
-  static MockTimeProvider* instance_;
-  DISALLOW_COPY_AND_ASSIGN(MockTimeProvider);
-};
-
-}  // namespace base
-
-#endif  // BASE_TEST_MOCK_TIME_PROVIDER_H_
diff --git a/components/search_engines/template_url_service.cc b/components/search_engines/template_url_service.cc
index 86f6ceeaebdb..46c53fe2fa5a 100644
--- a/components/search_engines/template_url_service.cc
+++ b/components/search_engines/template_url_service.cc
@@ -20,6 +20,7 @@
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/time/default_clock.h"
 #include "base/time/time.h"
 #include "components/rappor/rappor_service.h"
 #include "components/search_engines/search_engines_pref_names.h"
@@ -198,7 +199,7 @@ TemplateURLService::TemplateURLService(
       load_handle_(0),
       default_search_provider_(NULL),
       next_id_(kInvalidTemplateURLID + 1),
-      time_provider_(&base::Time::Now),
+      clock_(new base::DefaultClock),
       models_associated_(false),
       processing_syncer_changes_(false),
       dsp_change_origin_(DSP_CHANGE_OTHER),
@@ -223,7 +224,7 @@ TemplateURLService::TemplateURLService(const Initializer* initializers,
       load_handle_(0),
       default_search_provider_(NULL),
       next_id_(kInvalidTemplateURLID + 1),
-      time_provider_(&base::Time::Now),
+      clock_(new base::DefaultClock),
       models_associated_(false),
       processing_syncer_changes_(false),
       dsp_change_origin_(DSP_CHANGE_OTHER),
@@ -2067,7 +2068,7 @@ bool TemplateURLService::ResetTemplateURLNoNotify(
     data.favicon_url = GURL();
   }
   data.safe_for_autoreplace = false;
-  data.last_modified = time_provider_();
+  data.last_modified = clock_->Now();
   return UpdateNoNotify(url, TemplateURL(data));
 }
 
diff --git a/components/search_engines/template_url_service.h b/components/search_engines/template_url_service.h
index dda5de4f3dae..f93000f5af07 100644
--- a/components/search_engines/template_url_service.h
+++ b/components/search_engines/template_url_service.h
@@ -16,6 +16,7 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/observer_list.h"
 #include "base/prefs/pref_change_registrar.h"
+#include "base/time/clock.h"
 #include "components/google/core/browser/google_url_tracker.h"
 #include "components/keyed_service/core/keyed_service.h"
 #include "components/search_engines/default_search_manager.h"
@@ -69,8 +70,6 @@ class TemplateURLService : public WebDataServiceConsumer,
  public:
   typedef std::map<std::string, std::string> QueryTerms;
   typedef std::vector<TemplateURL*> TemplateURLVector;
-  // Type for a static function pointer that acts as a time source.
-  typedef base::Time(TimeProvider)();
   typedef std::map<std::string, syncer::SyncData> SyncDataMap;
   typedef base::CallbackList<void(void)>::Subscription Subscription;
 
@@ -366,11 +365,7 @@ class TemplateURLService : public WebDataServiceConsumer,
       const syncer::SyncDataList& sync_data);
 
 #if defined(UNIT_TEST)
-  // Sets a different time provider function, such as
-  // base::MockTimeProvider::StaticNow, for testing calls to base::Time::Now.
-  void set_time_provider(TimeProvider* time_provider) {
-    time_provider_ = time_provider;
-  }
+  void set_clock(scoped_ptr<base::Clock> clock) { clock_ = clock.Pass(); }
 #endif
 
  private:
@@ -706,8 +701,8 @@ class TemplateURLService : public WebDataServiceConsumer,
   // increasing integer that is initialized from the database.
   TemplateURLID next_id_;
 
-  // Function returning current time in base::Time units.
-  TimeProvider* time_provider_;
+  // Used to retrieve the current time, in base::Time units.
+  scoped_ptr<base::Clock> clock_;
 
   // Do we have an active association between the TemplateURLs and sync models?
   // Set in MergeDataAndStartSyncing, reset in StopSyncing. While this is not
diff --git a/components/search_engines/template_url_service_unittest.cc b/components/search_engines/template_url_service_unittest.cc
index ad474e7745ee..368496cc7fc2 100644
--- a/components/search_engines/template_url_service_unittest.cc
+++ b/components/search_engines/template_url_service_unittest.cc
@@ -2,17 +2,20 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "components/search_engines/template_url_service.h"
+
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/run_loop.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/cancelable_task_tracker.h"
-#include "base/test/mock_time_provider.h"
+#include "base/test/simple_test_clock.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
 #include "chrome/browser/history/history_service.h"
@@ -24,15 +27,12 @@
 #include "components/search_engines/search_terms_data.h"
 #include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_prepopulate_data.h"
-#include "components/search_engines/template_url_service.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::ASCIIToUTF16;
 using base::Time;
 using base::TimeDelta;
-using ::testing::Return;
-using ::testing::StrictMock;
 
 namespace {
 
@@ -150,6 +150,10 @@ class TemplateURLServiceTest : public testing::Test {
   // Verifies the two TemplateURLs are equal.
   void AssertEquals(const TemplateURL& expected, const TemplateURL& actual);
 
+  // Verifies the two timestamps are equal, within the expected degree of
+  // precision.
+  void AssertTimesEqual(const base::Time& expected, const base::Time& actual);
+
   // Create an URL that appears to have been prepopulated, but won't be in the
   // current data. The caller owns the returned TemplateURL*.
   TemplateURL* CreatePreloadedTemplateURL(bool safe_for_autoreplace,
@@ -227,12 +231,20 @@ void TemplateURLServiceTest::AssertEquals(const TemplateURL& expected,
   ASSERT_EQ(expected.input_encodings(), actual.input_encodings());
   ASSERT_EQ(expected.id(), actual.id());
   ASSERT_EQ(expected.date_created(), actual.date_created());
-  ASSERT_EQ(expected.last_modified(), actual.last_modified());
+  AssertTimesEqual(expected.last_modified(), actual.last_modified());
   ASSERT_EQ(expected.sync_guid(), actual.sync_guid());
   ASSERT_EQ(expected.search_terms_replacement_key(),
             actual.search_terms_replacement_key());
 }
 
+void TemplateURLServiceTest::AssertTimesEqual(const base::Time& expected,
+                                              const base::Time& actual) {
+  // Because times are stored with a granularity of one second, there is a loss
+  // of precision when serializing and deserializing the timestamps. Hence, only
+  // expect timestamps to be equal to within one second of one another.
+  ASSERT_LT((expected - actual).magnitude(), base::TimeDelta::FromSeconds(1));
+}
+
 TemplateURL* TemplateURLServiceTest::CreatePreloadedTemplateURL(
     bool safe_for_autoreplace,
     int prepopulate_id) {
@@ -305,11 +317,11 @@ TEST_F(TemplateURLServiceTest, AddUpdateRemove) {
                                          NULL));
 
   // We expect the last_modified time to be updated to the present time on an
-  // explicit reset. We have to set up the expectation here because ResetModel
-  // resets the TimeProvider in the TemplateURLService.
-  StrictMock<base::MockTimeProvider> mock_time;
-  model()->set_time_provider(&base::MockTimeProvider::StaticNow);
-  EXPECT_CALL(mock_time, Now()).WillOnce(Return(base::Time::FromDoubleT(1337)));
+  // explicit reset.
+  base::Time now = base::Time::Now();
+  scoped_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock);
+  clock->SetNow(now);
+  model()->set_clock(clock.Pass());
 
   // Mutate an element and verify it succeeded.
   model()->ResetTemplateURL(loaded_url, ASCIIToUTF16("a"), ASCIIToUTF16("b"),
@@ -330,7 +342,7 @@ TEST_F(TemplateURLServiceTest, AddUpdateRemove) {
   AssertEquals(*cloned_url, *loaded_url);
   // We changed a TemplateURL in the service, so ensure that the time was
   // updated.
-  ASSERT_EQ(base::Time::FromDoubleT(1337), loaded_url->last_modified());
+  AssertTimesEqual(now, loaded_url->last_modified());
 
   // Remove an element and verify it succeeded.
   model()->Remove(loaded_url);
@@ -620,9 +632,10 @@ TEST_F(TemplateURLServiceTest, Reset) {
   VerifyObserverCount(1);
   base::RunLoop().RunUntilIdle();
 
-  StrictMock<base::MockTimeProvider> mock_time;
-  model()->set_time_provider(&base::MockTimeProvider::StaticNow);
-  EXPECT_CALL(mock_time, Now()).WillOnce(Return(base::Time::FromDoubleT(1337)));
+  base::Time now = base::Time::Now();
+  scoped_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock);
+  clock->SetNow(now);
+  model()->set_clock(clock.Pass());
 
   // Reset the short name, keyword, url and make sure it takes.
   const base::string16 new_short_name(ASCIIToUTF16("a"));
@@ -646,7 +659,7 @@ TEST_F(TemplateURLServiceTest, Reset) {
   const TemplateURL* read_url = model()->GetTemplateURLForKeyword(new_keyword);
   ASSERT_TRUE(read_url);
   AssertEquals(*cloned_url, *read_url);
-  ASSERT_EQ(base::Time::FromDoubleT(1337), read_url->last_modified());
+  AssertTimesEqual(now, read_url->last_modified());
 }
 
 TEST_F(TemplateURLServiceTest, DefaultSearchProvider) {
-- 
2.11.4.GIT