Use the shared instance of base::Default{,Tick}Clock in previews

This CL changes the ownership of base::Clock and base::TickClock from
injectee-owned to injecter-owned. Before this CL, these instances are
owned by the owner of the injectee or one of the injectees themselves.
That makes the ownership complex.

After this CL, the clock instances are always un-owned by injectees.
Instead, an injecter owned one is used on testing, and the shared
instance is used on production path.

Bug: 789079
Change-Id: Ic91e3cbb16c8d66131936b6b4c6e74ed212602c2
Reviewed-on: https://chromium-review.googlesource.com/803154Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521191}

chrome/browser/net/spdyproxy/data_reduction_proxy_settings_unittest_android.cc

@@ -70,7 +70,7 @@ class TestDataReductionProxySettingsAndroid
 
 template <class C>
 void data_reduction_proxy::DataReductionProxySettingsTestBase::ResetSettings(
-    std::unique_ptr<base::Clock> clock) {
+    base::Clock* clock) {
   MockDataReductionProxySettings<C>* settings =
       new MockDataReductionProxySettings<C>();
   settings->config_ = test_context_->config();
@@ -86,9 +86,8 @@ void data_reduction_proxy::DataReductionProxySettingsTestBase::ResetSettings(
       test_context_->CreateDataReductionProxyService(settings_.get());
 }
 
-template void
-data_reduction_proxy::DataReductionProxySettingsTestBase::ResetSettings<
-    DataReductionProxyChromeSettings>(std::unique_ptr<base::Clock> clock);
+template void data_reduction_proxy::DataReductionProxySettingsTestBase::
+    ResetSettings<DataReductionProxyChromeSettings>(base::Clock* clock);
 
 namespace {
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc

@@ -43,7 +43,7 @@ DataReductionProxyDelegate::DataReductionProxyDelegate(
       event_creator_(event_creator),
       bypass_stats_(bypass_stats),
       alternative_proxies_broken_(false),
-      tick_clock_(new base::DefaultTickClock()),
+      tick_clock_(base::DefaultTickClock::GetInstance()),
       first_data_saver_request_recorded_(false),
       io_data_(nullptr),
       net_log_(net_log) {
@@ -154,8 +154,8 @@ void DataReductionProxyDelegate::OnTunnelHeadersReceived(
 }
 
 void DataReductionProxyDelegate::SetTickClockForTesting(
-    std::unique_ptr<base::TickClock> tick_clock) {
-  tick_clock_ = std::move(tick_clock);
+    base::TickClock* tick_clock) {
+  tick_clock_ = tick_clock;
   // Update |last_network_change_time_| to the provided tick clock's current
   // time for testing.
   last_network_change_time_ = tick_clock_->NowTicks();

components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.h

@@ -70,7 +70,7 @@ class DataReductionProxyDelegate
       const net::HostPortPair& proxy_server,
       const net::HttpResponseHeaders& response_headers) override;
 
-  void SetTickClockForTesting(std::unique_ptr<base::TickClock> tick_clock);
+  void SetTickClockForTesting(base::TickClock* tick_clock);
 
  protected:
   // Protected so that these methods are accessible for testing.
@@ -112,7 +112,7 @@ class DataReductionProxyDelegate
   bool alternative_proxies_broken_;
 
   // Tick clock used for obtaining the current time.
-  std::unique_ptr<base::TickClock> tick_clock_;
+  base::TickClock* tick_clock_;
 
   // True if the metrics related to the first request whose resolved proxy was a
   // data saver proxy has been recorded. |first_data_saver_request_recorded_| is

components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc

@@ -838,10 +838,8 @@ TEST_F(DataReductionProxyDelegateTest, OnCompletedSizeFor200) {
 }
 
 TEST_F(DataReductionProxyDelegateTest, TimeToFirstHttpDataSaverRequest) {
-  std::unique_ptr<base::SimpleTestTickClock> tick_clock(
-      new base::SimpleTestTickClock());
-  base::SimpleTestTickClock* tick_clock_ptr = tick_clock.get();
-  proxy_delegate()->SetTickClockForTesting(std::move(tick_clock));
+  base::SimpleTestTickClock tick_clock;
+  proxy_delegate()->SetTickClockForTesting(&tick_clock);
 
   const char kResponseHeaders[] =
       "HTTP/1.1 200 OK\r\n"
@@ -852,7 +850,7 @@ TEST_F(DataReductionProxyDelegateTest, TimeToFirstHttpDataSaverRequest) {
   {
     base::HistogramTester histogram_tester;
     base::TimeDelta advance_time(base::TimeDelta::FromSeconds(1));
-    tick_clock_ptr->Advance(advance_time);
+    tick_clock.Advance(advance_time);
 
     FetchURLRequest(GURL("http://example.com/path/"), nullptr, kResponseHeaders,
                     10);
@@ -874,7 +872,7 @@ TEST_F(DataReductionProxyDelegateTest, TimeToFirstHttpDataSaverRequest) {
     net::NetworkChangeNotifier::NotifyObserversOfIPAddressChangeForTests();
     base::RunLoop().RunUntilIdle();
 
-    tick_clock_ptr->Advance(advance_time);
+    tick_clock.Advance(advance_time);
     FetchURLRequest(GURL("http://example.com/path/"), nullptr, kResponseHeaders,
                     10);
     histogram_tester.ExpectUniqueSample(

components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.cc

@@ -54,7 +54,7 @@ DataReductionProxySettings::DataReductionProxySettings()
       data_reduction_proxy_enabled_pref_name_(),
       prefs_(nullptr),
       config_(nullptr),
-      clock_(new base::DefaultClock()) {}
+      clock_(base::DefaultClock::GetInstance()) {}
 
 DataReductionProxySettings::~DataReductionProxySettings() {
   spdy_proxy_auth_enabled_.Destroy();

components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h

@@ -265,7 +265,7 @@ class DataReductionProxySettings : public DataReductionProxyServiceObserver {
   SyntheticFieldTrialRegistrationCallback register_synthetic_field_trial_;
 
   // Should not be null.
-  std::unique_ptr<base::Clock> clock_;
+  base::Clock* clock_;
 
   base::ThreadChecker thread_checker_;
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_test_utils.cc

@@ -72,8 +72,7 @@ void DataReductionProxySettingsTestBase::SetUp() {
 }
 
 template <class C>
-void DataReductionProxySettingsTestBase::ResetSettings(
-    std::unique_ptr<base::Clock> clock) {
+void DataReductionProxySettingsTestBase::ResetSettings(base::Clock* clock) {
   MockDataReductionProxySettings<C>* settings =
       new MockDataReductionProxySettings<C>();
   settings->config_ = test_context_->config();
@@ -81,7 +80,7 @@ void DataReductionProxySettingsTestBase::ResetSettings(
   settings->data_reduction_proxy_service_ =
       test_context_->CreateDataReductionProxyService(settings);
   if (clock)
-    settings->clock_ = std::move(clock);
+    settings->clock_ = clock;
   EXPECT_CALL(*settings, GetOriginalProfilePrefs())
       .Times(AnyNumber())
       .WillRepeatedly(Return(test_context_->pref_service()));
@@ -93,7 +92,7 @@ void DataReductionProxySettingsTestBase::ResetSettings(
 
 // Explicitly generate required instantiations.
 template void DataReductionProxySettingsTestBase::ResetSettings<
-    DataReductionProxySettings>(std::unique_ptr<base::Clock> clock);
+    DataReductionProxySettings>(base::Clock* clock);
 
 void DataReductionProxySettingsTestBase::ExpectSetProxyPrefs(
     bool expected_enabled,

components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_test_utils.h

@@ -50,8 +50,8 @@ class DataReductionProxySettingsTestBase : public testing::Test {
   void SetUp() override;
 
   template <class C>
-  void ResetSettings(std::unique_ptr<base::Clock> clock);
-  virtual void ResetSettings(std::unique_ptr<base::Clock> clock) = 0;
+  void ResetSettings(base::Clock* clock);
+  virtual void ResetSettings(base::Clock* clock) = 0;
 
   void ExpectSetProxyPrefs(bool expected_enabled,
                            bool expected_at_startup);
@@ -84,9 +84,8 @@ class ConcreteDataReductionProxySettingsTest
     : public DataReductionProxySettingsTestBase {
  public:
   typedef MockDataReductionProxySettings<C> MockSettings;
-  void ResetSettings(std::unique_ptr<base::Clock> clock) override {
-    return DataReductionProxySettingsTestBase::ResetSettings<C>(
-        std::move(clock));
+  void ResetSettings(base::Clock* clock) override {
+    return DataReductionProxySettingsTestBase::ResetSettings<C>(clock);
   }
 };
 

components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_unittest.cc

@@ -385,12 +385,11 @@ TEST_F(DataReductionProxySettingsTest, TestSettingsEnabledStateHistograms) {
 // enables the data reduction proxy.
 TEST_F(DataReductionProxySettingsTest, TestDaysSinceEnabledWithTestClock) {
   const char kUMAEnabledState[] = "DataReductionProxy.DaysSinceEnabled";
-  std::unique_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock());
-  base::SimpleTestClock* clock_ptr = clock.get();
-  clock_ptr->Advance(base::TimeDelta::FromDays(1));
-  ResetSettings(std::move(clock));
+  base::SimpleTestClock clock;
+  clock.Advance(base::TimeDelta::FromDays(1));
+  ResetSettings(&clock);
 
-  base::Time last_enabled_time = clock_ptr->Now();
+  base::Time last_enabled_time = clock.Now();
 
   InitPrefMembers();
   {
@@ -405,7 +404,7 @@ TEST_F(DataReductionProxySettingsTest, TestDaysSinceEnabledWithTestClock) {
     settings_->SetDataReductionProxyEnabled(true /* enabled */);
     test_context_->RunUntilIdle();
 
-    last_enabled_time = clock_ptr->Now();
+    last_enabled_time = clock.Now();
 
     EXPECT_EQ(
         last_enabled_time,
@@ -418,9 +417,9 @@ TEST_F(DataReductionProxySettingsTest, TestDaysSinceEnabledWithTestClock) {
     // Simulate turning off and on of data reduction proxy while Chromium is
     // running.
     settings_->SetDataReductionProxyEnabled(false /* enabled */);
-    clock_ptr->Advance(base::TimeDelta::FromDays(1));
+    clock.Advance(base::TimeDelta::FromDays(1));
     base::HistogramTester histogram_tester;
-    last_enabled_time = clock_ptr->Now();
+    last_enabled_time = clock.Now();
 
     settings_->spdy_proxy_auth_enabled_.SetValue(true);
     settings_->MaybeActivateDataReductionProxy(false);
@@ -435,7 +434,7 @@ TEST_F(DataReductionProxySettingsTest, TestDaysSinceEnabledWithTestClock) {
   {
     // Advance clock by a random number of days.
     int advance_clock_days = 42;
-    clock_ptr->Advance(base::TimeDelta::FromDays(advance_clock_days));
+    clock.Advance(base::TimeDelta::FromDays(advance_clock_days));
     base::HistogramTester histogram_tester;
     // Simulate Chromium start up. Data reduction proxy was enabled
     // |advance_clock_days| ago.
@@ -469,22 +468,21 @@ TEST_F(DataReductionProxySettingsTest, TestDaysSinceEnabledExistingUser) {
 }
 
 TEST_F(DataReductionProxySettingsTest, TestDaysSinceSavingsCleared) {
-  std::unique_ptr<base::SimpleTestClock> clock(new base::SimpleTestClock());
-  base::SimpleTestClock* clock_ptr = clock.get();
-  clock_ptr->Advance(base::TimeDelta::FromDays(1));
-  ResetSettings(std::move(clock));
+  base::SimpleTestClock clock;
+  clock.Advance(base::TimeDelta::FromDays(1));
+  ResetSettings(&clock);
 
   InitPrefMembers();
   base::HistogramTester histogram_tester;
   test_context_->pref_service()->SetInt64(
       prefs::kDataReductionProxySavingsClearedNegativeSystemClock,
-      clock_ptr->Now().ToInternalValue());
+      clock.Now().ToInternalValue());
 
   settings_->data_reduction_proxy_service_->SetIOData(
       test_context_->io_data()->GetWeakPtr());
   test_context_->RunUntilIdle();
 
-  clock_ptr->Advance(base::TimeDelta::FromDays(100));
+  clock.Advance(base::TimeDelta::FromDays(100));
 
   // Simulate Chromium startup with data reduction proxy already enabled.
   settings_->spdy_proxy_auth_enabled_.SetValue(true);

components/previews/content/previews_io_data.cc

@@ -133,7 +133,7 @@ void PreviewsIOData::InitializeOnIOThread(
   DCHECK(io_task_runner_->BelongsToCurrentThread());
   previews_black_list_.reset(
       new PreviewsBlackList(std::move(previews_opt_out_store),
-                            base::MakeUnique<base::DefaultClock>(), this));
+                            base::DefaultClock::GetInstance(), this));
   ui_task_runner_->PostTask(
       FROM_HERE, base::Bind(&PreviewsUIService::SetIOData, previews_ui_service_,
                             weak_factory_.GetWeakPtr()));

components/previews/content/previews_io_data_unittest.cc

@@ -77,7 +77,7 @@ class TestPreviewsBlackList : public PreviewsBlackList {
   TestPreviewsBlackList(PreviewsEligibilityReason status,
                         PreviewsBlacklistDelegate* blacklist_delegate)
       : PreviewsBlackList(nullptr,
-                          base::MakeUnique<base::DefaultClock>(),
+                          base::DefaultClock::GetInstance(),
                           blacklist_delegate),
         status_(status) {}
   ~TestPreviewsBlackList() override {}

components/previews/core/previews_black_list.cc

@@ -54,11 +54,11 @@ PreviewsBlackListItem* GetBlackListItemFromMap(
 
 PreviewsBlackList::PreviewsBlackList(
     std::unique_ptr<PreviewsOptOutStore> opt_out_store,
-    std::unique_ptr<base::Clock> clock,
+    base::Clock* clock,
     PreviewsBlacklistDelegate* blacklist_delegate)
     : loaded_(false),
       opt_out_store_(std::move(opt_out_store)),
-      clock_(std::move(clock)),
+      clock_(clock),
       blacklist_delegate_(blacklist_delegate),
       weak_factory_(this) {
   DCHECK(blacklist_delegate_);

components/previews/core/previews_black_list.h

@@ -82,7 +82,7 @@ class PreviewsBlackList {
   // |blacklist_delegate| is a single object listening for blacklist events, and
   // it is guaranteed to overlive the life time of |this|.
   PreviewsBlackList(std::unique_ptr<PreviewsOptOutStore> opt_out_store,
-                    std::unique_ptr<base::Clock> clock,
+                    base::Clock* clock,
                     PreviewsBlacklistDelegate* blacklist_delegate);
   virtual ~PreviewsBlackList();
 
@@ -165,7 +165,7 @@ class PreviewsBlackList {
   // completed.
   base::queue<base::Closure> pending_callbacks_;
 
-  std::unique_ptr<base::Clock> clock_;
+  base::Clock* clock_;
 
   // The delegate listening to this blacklist. |blacklist_delegate_| lifetime is
   // guaranteed to overlive |this|.