Add SetClockForTesting to the UserModifiableProvider class

To support adjustments to the clock used by a UserModifiableProvider
during testing, the SetClockForTesting function is elevated to be a
virtual member of the class. Most subclasses provided this functionality
by an identical function, with NotificationChannelsProviderAndroid
providing it via custom constructors.

This CL refactors NotificationChannelsProviderAndroid local clock to align
with other existing subclasses and updates associated test code.

Additional changes to the SetClockForTesting on the
HostContentSettingsMap allow it to set the clock on all user_providers
it is aware of.

Bug: 1032584
Change-Id: I068b7179375c0e6ce668ae2f2e4d8c0fcfc71cb7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2000028
Commit-Queue: Theodore Olsauskas-Warren <sauski@google.com>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732422}

chrome/browser/content_settings/host_content_settings_map_unittest.cc

@@ -82,6 +82,8 @@ class MockUserModifiableProvider
                           const ContentSettingsPattern&,
                           ContentSettingsType,
                           const content_settings::ResourceIdentifier&));
+
+  MOCK_METHOD1(SetClockForTesting, void(base::Clock*));
 };
 
 class HostContentSettingsMapTest : public testing::Test {

chrome/browser/notifications/notification_channels_provider_android.cc

@@ -187,15 +187,13 @@ NotificationChannel::NotificationChannel(const NotificationChannel& other) =
 
 NotificationChannelsProviderAndroid::NotificationChannelsProviderAndroid()
     : NotificationChannelsProviderAndroid(
-          std::make_unique<NotificationChannelsBridgeImpl>(),
-          std::make_unique<base::DefaultClock>()) {}
+          std::make_unique<NotificationChannelsBridgeImpl>()) {}
 
 NotificationChannelsProviderAndroid::NotificationChannelsProviderAndroid(
-    std::unique_ptr<NotificationChannelsBridge> bridge,
-    std::unique_ptr<base::Clock> clock)
+    std::unique_ptr<NotificationChannelsBridge> bridge)
     : bridge_(std::move(bridge)),
       platform_supports_channels_(bridge_->ShouldUseChannelSettings()),
-      clock_(std::move(clock)),
+      clock_(base::DefaultClock::GetInstance()),
       initialized_cached_channels_(false) {}
 
 NotificationChannelsProviderAndroid::~NotificationChannelsProviderAndroid() =
@@ -394,6 +392,11 @@ base::Time NotificationChannelsProviderAndroid::GetWebsiteSettingLastModified(
   return channel_entry->second.timestamp;
 }
 
+void NotificationChannelsProviderAndroid::SetClockForTesting(
+    base::Clock* clock) {
+  clock_ = clock;
+}
+
 // InitCachedChannels() must be called prior to calling this method.
 void NotificationChannelsProviderAndroid::CreateChannelIfRequired(
     const std::string& origin_string,

chrome/browser/notifications/notification_channels_provider_android.h

@@ -100,11 +100,11 @@ class NotificationChannelsProviderAndroid
       const ContentSettingsPattern& secondary_pattern,
       ContentSettingsType content_type,
       const content_settings::ResourceIdentifier& resource_identifier) override;
+  void SetClockForTesting(base::Clock* clock) override;
 
  private:
   explicit NotificationChannelsProviderAndroid(
-      std::unique_ptr<NotificationChannelsBridge> bridge,
-      std::unique_ptr<base::Clock> clock);
+      std::unique_ptr<NotificationChannelsBridge> bridge);
   friend class NotificationChannelsProviderAndroidTest;
 
   std::vector<NotificationChannel> UpdateCachedChannels() const;
@@ -120,7 +120,7 @@ class NotificationChannelsProviderAndroid
 
   bool platform_supports_channels_;
 
-  std::unique_ptr<base::Clock> clock_;
+  base::Clock* clock_;
 
   // Flag to keep track of whether |cached_channels_| has been initialized yet.
   bool initialized_cached_channels_;

chrome/browser/notifications/notification_channels_provider_android_unittest.cc

@@ -131,18 +131,12 @@ class NotificationChannelsProviderAndroidTest : public testing::Test {
 
  protected:
   void InitChannelsProvider(bool should_use_channels) {
-    InitChannelsProviderWithClock(should_use_channels,
-                                  std::make_unique<base::DefaultClock>());
-  }
-
-  void InitChannelsProviderWithClock(bool should_use_channels,
-                                     std::unique_ptr<base::Clock> clock) {
     fake_bridge_ = new FakeNotificationChannelsBridge(should_use_channels);
 
     // Can't use std::make_unique because the provider's constructor is private.
     channels_provider_ =
         base::WrapUnique(new NotificationChannelsProviderAndroid(
-            base::WrapUnique(fake_bridge_), std::move(clock)));
+            base::WrapUnique(fake_bridge_)));
   }
 
   ContentSettingsPattern GetTestPattern() {
@@ -481,12 +475,11 @@ TEST_F(NotificationChannelsProviderAndroidTest,
 
 TEST_F(NotificationChannelsProviderAndroidTest,
        GetWebsiteSettingLastModifiedReturnsMostRecentTimestamp) {
-  auto test_clock = std::make_unique<base::SimpleTestClock>();
+  base::SimpleTestClock clock;
   base::Time t1 = base::Time::Now();
-  test_clock->SetNow(t1);
-  base::SimpleTestClock* clock = test_clock.get();
-  InitChannelsProviderWithClock(true /* should_use_channels */,
-                                std::move(test_clock));
+  clock.SetNow(t1);
+  InitChannelsProvider(true /* should_use_channels */);
+  channels_provider_->SetClockForTesting(&clock);
 
   // Create channel and check last-modified time is the creation time.
   std::string first_origin = "https://example.com";
@@ -494,7 +487,7 @@ TEST_F(NotificationChannelsProviderAndroidTest,
       ContentSettingsPattern::FromString(first_origin),
       ContentSettingsPattern(), ContentSettingsType::NOTIFICATIONS,
       std::string(), std::make_unique<base::Value>(CONTENT_SETTING_ALLOW));
-  clock->Advance(base::TimeDelta::FromSeconds(1));
+  clock.Advance(base::TimeDelta::FromSeconds(1));
 
   base::Time last_modified = channels_provider_->GetWebsiteSettingLastModified(
       ContentSettingsPattern::FromString(first_origin),
@@ -504,8 +497,8 @@ TEST_F(NotificationChannelsProviderAndroidTest,
 
   // Delete and recreate the same channel after some time has passed.
   // This simulates the user clearing data and regranting permisison.
-  clock->Advance(base::TimeDelta::FromSeconds(3));
-  base::Time t2 = clock->Now();
+  clock.Advance(base::TimeDelta::FromSeconds(3));
+  base::Time t2 = clock.Now();
   channels_provider_->SetWebsiteSetting(
       ContentSettingsPattern::FromString(first_origin),
       ContentSettingsPattern(), ContentSettingsType::NOTIFICATIONS,
@@ -523,7 +516,7 @@ TEST_F(NotificationChannelsProviderAndroidTest,
   EXPECT_EQ(last_modified, t2);
 
   // Create an unrelated channel after some more time has passed.
-  clock->Advance(base::TimeDelta::FromSeconds(5));
+  clock.Advance(base::TimeDelta::FromSeconds(5));
   std::string second_origin = "https://other.com";
   channels_provider_->SetWebsiteSetting(
       ContentSettingsPattern::FromString(second_origin),

components/content_settings/core/browser/content_settings_ephemeral_provider.h

@@ -40,8 +40,8 @@ class EphemeralProvider : public UserModifiableProvider {
       const ContentSettingsPattern& secondary_pattern,
       ContentSettingsType content_type,
       const ResourceIdentifier& resource_identifier) override;
+  void SetClockForTesting(base::Clock* clock) override;
 
-  void SetClockForTesting(base::Clock* clock);
   void SetSupportedTypesForTesting(
       std::set<ContentSettingsType>& supported_types) {
     supported_types_ = supported_types;

components/content_settings/core/browser/content_settings_pref_provider.h

@@ -58,13 +58,12 @@ class PrefProvider : public UserModifiableProvider {
       const ContentSettingsPattern& secondary_pattern,
       ContentSettingsType content_type,
       const ResourceIdentifier& resource_identifier) override;
+  void SetClockForTesting(base::Clock* clock) override;
 
   void ClearPrefs();
 
   ContentSettingsPref* GetPref(ContentSettingsType type) const;
 
-  void SetClockForTesting(base::Clock* clock);
-
  private:
   friend class DeadlockCheckerObserver;  // For testing.
 

components/content_settings/core/browser/host_content_settings_map.cc

@@ -624,7 +624,8 @@ base::WeakPtr<HostContentSettingsMap> HostContentSettingsMap::GetWeakPtr() {
 }
 
 void HostContentSettingsMap::SetClockForTesting(base::Clock* clock) {
-  pref_provider_->SetClockForTesting(clock);
+  for (auto* provider : user_modifiable_providers_)
+    provider->SetClockForTesting(clock);
 }
 
 void HostContentSettingsMap::RecordExceptionMetrics() {

components/content_settings/core/browser/user_modifiable_provider.h

@@ -8,6 +8,10 @@
 #include "components/content_settings/core/browser/content_settings_observable_provider.h"
 #include "components/content_settings/core/common/content_settings.h"
 
+namespace base {
+class Clock;
+}
+
 class ContentSettingsPattern;
 
 namespace content_settings {
@@ -23,6 +27,8 @@ class UserModifiableProvider : public ObservableProvider {
       const ContentSettingsPattern& secondary_pattern,
       ContentSettingsType content_type,
       const ResourceIdentifier& resource_identifier) = 0;
+  // Sets the providers internal clock for testing purposes.
+  virtual void SetClockForTesting(base::Clock* clock) = 0;
 };
 
 }  // namespace content_settings