Prepare explore_sites unittests for imminent switching on of the...

Prepare explore_sites unittests for imminent switching on of the HistoryServiceUsesTaskScheduler experiment.

The feature will soon be flipped to ENABLED_BY_DEFAULT. This will make HistoryServiceBackend's task runner
dispatch to the ThreadPool instead of a dedicated base::Thread. The tests need to be
prepared to stay functional when that happens.

This is done as the last mile of the chrome-wide effort to coalesce as much work as possible
to the ThreadPool.

Bug: 661143
Change-Id: I7e08e21c212c2852891d30d0db70b946d932dd4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1865595Reviewed-by: Justin DeWitt <dewittj@chromium.org>
Commit-Queue: Oliver Li <olivierli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#707832}

chrome/browser/android/explore_sites/explore_sites_service_factory.cc

@@ -85,7 +85,7 @@ KeyedService* ExploreSitesServiceFactory::BuildServiceInstanceFor(
       HistoryServiceFactory::GetForProfile(profile,
                                            ServiceAccessType::EXPLICIT_ACCESS);
   auto history_stats_reporter = std::make_unique<HistoryStatisticsReporter>(
-      history_service, profile->GetPrefs(), base::DefaultClock::GetInstance());
+      history_service, profile->GetPrefs());
 
   return new ExploreSitesServiceImpl(std::move(explore_sites_store),
                                      std::move(url_loader_factory_getter),

chrome/browser/android/explore_sites/explore_sites_service_impl_unittest.cc

@@ -48,7 +48,7 @@ class ExploreSitesServiceImplTest : public testing::Test {
         std::make_unique<ExploreSitesStore>(
             task_environment_.GetMainThreadTaskRunner());
     auto history_stats_reporter =
-        std::make_unique<HistoryStatisticsReporter>(nullptr, nullptr, nullptr);
+        std::make_unique<HistoryStatisticsReporter>(nullptr, nullptr);
     service_ = std::make_unique<ExploreSitesServiceImpl>(
         std::move(store),
         std::make_unique<TestURLLoaderFactoryGetter>(

chrome/browser/android/explore_sites/history_statistics_reporter.cc

@@ -9,14 +9,12 @@
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "base/time/time.h"
 #include "chrome/common/pref_names.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/history/core/browser/history_types.h"
 
 namespace {
-// Delay between the scheduling and actual computing/reporting of stats.
-const int kComputeStatisticsDelaySeconds = 5;
-
 // Pref name for the persistent timestamp of the last stats reporting.
 const char kWeeklyStatsReportingTimestamp[] =
     "explore_sites.weekly_stats_reporting_timestamp";
@@ -33,11 +31,9 @@ void HistoryStatisticsReporter::RegisterPrefs(PrefRegistrySimple* registry) {
 // from HistoryService::CountUniqueHostsVisitedLastMonth)
 HistoryStatisticsReporter::HistoryStatisticsReporter(
     history::HistoryService* history_service,
-    PrefService* prefs,
-    base::Clock* clock)
+    PrefService* prefs)
     : history_service_(history_service),
       prefs_(prefs),
-      clock_(clock),
       history_service_observer_(this) {}
 
 HistoryStatisticsReporter::~HistoryStatisticsReporter() {
@@ -52,17 +48,14 @@ void HistoryStatisticsReporter::ScheduleReportStatistics() {
 
   // If we've already reported metrics during last week, bail out.
   base::Time last_report_time = prefs_->GetTime(kWeeklyStatsReportingTimestamp);
-  if (last_report_time > clock_->Now() - base::TimeDelta::FromDays(7))
+  if (last_report_time > base::Time::Now() - base::TimeDelta::FromDays(7))
     return;
 
-  base::TimeDelta computeStatisticsDelay =
-      base::TimeDelta::FromSeconds(kComputeStatisticsDelaySeconds);
-
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
       base::BindOnce(&HistoryStatisticsReporter::MaybeReportStatistics,
                      weak_ptr_factory_.GetWeakPtr()),
-      computeStatisticsDelay);
+      kComputeStatisticsDelay);
 }
 
 void HistoryStatisticsReporter::OnHistoryServiceLoaded(
@@ -101,6 +94,6 @@ void HistoryStatisticsReporter::ReportStatistics(
     return;
   UMA_HISTOGRAM_COUNTS_1000("ExploreSites.MonthlyHostCount", result.count);
   // Remember when stats were reported to skip attempts until next week.
-  prefs_->SetTime(kWeeklyStatsReportingTimestamp, clock_->Now());
+  prefs_->SetTime(kWeeklyStatsReportingTimestamp, base::Time::Now());
 }
 }  // namespace explore_sites

chrome/browser/android/explore_sites/history_statistics_reporter.h

@@ -11,7 +11,6 @@
 #include "base/memory/weak_ptr.h"
 #include "base/scoped_observer.h"
 #include "base/task/cancelable_task_tracker.h"
-#include "base/time/clock.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/history/core/browser/history_service_observer.h"
 #include "components/prefs/pref_registry_simple.h"
@@ -20,11 +19,14 @@
 namespace explore_sites {
 class HistoryStatisticsReporter : public history::HistoryServiceObserver {
  public:
+  // Delay between the scheduling and actual computing/reporting of stats.
+  static constexpr base::TimeDelta kComputeStatisticsDelay =
+      base::TimeDelta::FromSeconds(5);
+
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
   HistoryStatisticsReporter(history::HistoryService* history_service,
-                            PrefService* prefs,
-                            base::Clock* clock);
+                            PrefService* prefs);
   ~HistoryStatisticsReporter() override;
 
   // Schedules delayed task to compute/report history statistics.
@@ -46,7 +48,6 @@ class HistoryStatisticsReporter : public history::HistoryServiceObserver {
 
   history::HistoryService* const history_service_;
   PrefService* prefs_;
-  base::Clock* clock_;
 
   base::CancelableTaskTracker cancelable_task_tracker_;
   ScopedObserver<history::HistoryService, history::HistoryServiceObserver>

chrome/browser/android/explore_sites/history_statistics_reporter_unittest.cc

@@ -8,7 +8,10 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/test/bind_test_util.h"
 #include "base/test/metrics/histogram_tester.h"
-#include "base/test/test_mock_time_task_runner.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/test/task_environment.h"
+#include "base/time/clock.h"
+#include "base/time/time.h"
 #include "components/history/core/browser/history_database_params.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/history/core/test/history_service_test_util.h"
@@ -30,30 +33,34 @@ namespace explore_sites {
 class HistoryStatisticsReporterTest : public testing::Test {
  public:
   HistoryStatisticsReporterTest()
-      : task_runner_(new base::TestMockTimeTaskRunner(
-            base::TestMockTimeTaskRunner::Type::kBoundToThread)) {}
+      : task_environment_(
+            base::test::SingleThreadTaskEnvironment::TimeSource::MOCK_TIME) {}
   ~HistoryStatisticsReporterTest() override {}
 
   void SetUp() override {
+    feature_list_.InitWithFeatures(
+        {history::HistoryService::kHistoryServiceUsesTaskScheduler}, {});
+
     HistoryStatisticsReporter::RegisterPrefs(pref_service_.registry());
     ASSERT_TRUE(history_dir_.CreateUniqueTempDir());
     // Creates HistoryService, but does not load it yet. Use LoadHistory() from
     // tests to control loading of HistoryService.
     history_service_ = std::make_unique<history::HistoryService>();
-    reporter_ = std::make_unique<HistoryStatisticsReporter>(
-        history_service(), &pref_service_, task_runner_->GetMockClock());
+    reporter_ = std::make_unique<HistoryStatisticsReporter>(history_service(),
+                                                            &pref_service_);
   }
 
   // Wait for separate background task runner in HistoryService to complete
   // all tasks and then all the tasks on the current one to complete as well.
   void RunUntilIdle() {
     history::BlockUntilHistoryProcessesPendingRequests(history_service());
-    task_runner_->RunUntilIdle();
+    task_environment_.RunUntilIdle();
   }
 
   void ScheduleReportAndRunUntilIdle() {
     reporter()->ScheduleReportStatistics();
-    task_runner()->FastForwardUntilNoTasksRemain();
+    task_environment_.FastForwardBy(
+        HistoryStatisticsReporter::kComputeStatisticsDelay);
     RunUntilIdle();
   }
 
@@ -68,11 +75,13 @@ class HistoryStatisticsReporterTest : public testing::Test {
   HistoryStatisticsReporter* reporter() const { return reporter_.get(); }
   const base::HistogramTester& histograms() const { return histogram_tester_; }
   history::HistoryService* history_service() { return history_service_.get(); }
-  base::TestMockTimeTaskRunner* task_runner() { return task_runner_.get(); }
   TestingPrefServiceSimple* prefs() { return &pref_service_; }
 
+ protected:
+  base::test::TaskEnvironment task_environment_;
+
  private:
-  scoped_refptr<base::TestMockTimeTaskRunner> task_runner_;
+  base::test::ScopedFeatureList feature_list_;
   base::ScopedTempDir history_dir_;
   TestingPrefServiceSimple pref_service_;
   base::HistogramTester histogram_tester_;
@@ -87,7 +96,8 @@ TEST_F(HistoryStatisticsReporterTest, HistoryNotLoaded) {
   reporter()->ScheduleReportStatistics();
 
   // Move past initial delay of reporter.
-  task_runner()->FastForwardUntilNoTasksRemain();
+  task_environment_.FastForwardBy(
+      HistoryStatisticsReporter::kComputeStatisticsDelay);
 
   // Since History is not yet loaded, there should be no histograms.
   histograms().ExpectTotalCount("History.DatabaseMonthlyHostCountTime", 0);
@@ -108,7 +118,8 @@ TEST_F(HistoryStatisticsReporterTest, HistoryLoaded) {
 
   reporter()->ScheduleReportStatistics();
   // Move past initial delay of reporter.
-  task_runner()->FastForwardUntilNoTasksRemain();
+  task_environment_.FastForwardBy(
+      HistoryStatisticsReporter::kComputeStatisticsDelay);
 
   RunUntilIdle();
   // Since History is already loaded, there should be a sample reported.
@@ -126,7 +137,8 @@ TEST_F(HistoryStatisticsReporterTest, HistoryLoadedTimeDelay) {
   histograms().ExpectTotalCount("History.DatabaseMonthlyHostCountTime", 0);
 
   // Move past initial delay of reporter.
-  task_runner()->FastForwardUntilNoTasksRemain();
+  task_environment_.FastForwardBy(
+      HistoryStatisticsReporter::kComputeStatisticsDelay);
 
   RunUntilIdle();
   // Since History is already loaded, there should be a sample reported.
@@ -200,15 +212,14 @@ TEST_F(HistoryStatisticsReporterTest, OneRunPerWeekSaveTimestamp) {
   histograms().ExpectTotalCount("History.DatabaseMonthlyHostCountTime", 1);
 
   // Reporter should have left the time of request in Prefs.
-  base::Time time_now = task_runner()->GetMockClock()->Now();
-  EXPECT_EQ(time_now, prefs()->GetTime(kWeeklyStatsReportingTimestamp));
+  EXPECT_EQ(base::Time::Now(),
+            prefs()->GetTime(kWeeklyStatsReportingTimestamp));
 }
 
 TEST_F(HistoryStatisticsReporterTest, OneRunPerWeekReadTimestamp) {
   ASSERT_TRUE(LoadHistory());
 
-  prefs()->SetTime(kWeeklyStatsReportingTimestamp,
-                   task_runner()->GetMockClock()->Now());
+  prefs()->SetTime(kWeeklyStatsReportingTimestamp, base::Time::Now());
   ScheduleReportAndRunUntilIdle();
 
   // No queries, a week did not pass yet.
@@ -218,16 +229,15 @@ TEST_F(HistoryStatisticsReporterTest, OneRunPerWeekReadTimestamp) {
 TEST_F(HistoryStatisticsReporterTest, OneRunPerWeekReadTimestampAfterWeek) {
   ASSERT_TRUE(LoadHistory());
 
-  prefs()->SetTime(
-      kWeeklyStatsReportingTimestamp,
-      task_runner()->GetMockClock()->Now() - base::TimeDelta::FromDays(8));
+  prefs()->SetTime(kWeeklyStatsReportingTimestamp,
+                   base::Time::Now() - base::TimeDelta::FromDays(8));
   ScheduleReportAndRunUntilIdle();
 
   // More than a week since last query, should have gone through.
   histograms().ExpectTotalCount("History.DatabaseMonthlyHostCountTime", 1);
   // Reporter should have left the time of request in Prefs.
-  base::Time time_now = task_runner()->GetMockClock()->Now();
-  EXPECT_EQ(time_now, prefs()->GetTime(kWeeklyStatsReportingTimestamp));
+  EXPECT_EQ(base::Time::Now(),
+            prefs()->GetTime(kWeeklyStatsReportingTimestamp));
 }
 
 }  // namespace explore_sites