Tests can disable the execution of OPM maintenance tasks

To avoid postponed maintenance tasks form effecting Chrome unit tests
this change allows the scheduling and running of them to be disabled.

Bug: 822006
Change-Id: I042bdef8b450531d0e4699779691a3b8cc3e65f7
Reviewed-on: https://chromium-review.googlesource.com/965257
Commit-Queue: Yafei Duan <romax@chromium.org>
Reviewed-by: Yafei Duan <romax@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543621}

chrome/browser/offline_pages/offline_page_request_job_unittest.cc

@@ -496,7 +496,7 @@ class OfflinePageRequestJobTest : public testing::Test {
   TestingProfileManager profile_manager_;
   TestingProfile* profile_;
   std::unique_ptr<content::WebContents> web_contents_;
-  base::HistogramTester histogram_tester_;
+  std::unique_ptr<base::HistogramTester> histogram_tester_;
   OfflinePageTabHelper* offline_page_tab_helper_;  // Not owned.
   int64_t last_offline_id_;
   std::string data_received_;
@@ -578,6 +578,9 @@ void OfflinePageRequestJobTest::SetUp() {
   // metadata already written to the store.
   model->SetSkipClearingOriginalUrlForTesting();
 
+  // Avoid running the model's maintenance tasks.
+  model->DoNotRunMaintenanceTasksForTesting();
+
   // Move test data files into their respective temporary test directories. The
   // model's maintenance tasks must not be executed in the meantime otherwise
   // these files will be wiped by consistency checks.
@@ -591,6 +594,8 @@ void OfflinePageRequestJobTest::SetUp() {
       test_data_dir_path.AppendASCII(kPublicOfflineFileDir);
   ASSERT_TRUE(base::CopyDirectory(test_data_public_archives_dir,
                                   public_archives_dir_.DirName(), true));
+
+  histogram_tester_ = std::make_unique<base::HistogramTester>();
 }
 
 void OfflinePageRequestJobTest::TearDown() {
@@ -598,8 +603,8 @@ void OfflinePageRequestJobTest::TearDown() {
   EXPECT_TRUE(public_archives_temp_base_dir_.Delete());
   // This check confirms that the model's maintenance tasks were not executed
   // during the test run.
-  histogram_tester_.ExpectTotalCount("OfflinePages.ClearTemporaryPages.Result",
-                                     0);
+  histogram_tester_->ExpectTotalCount("OfflinePages.ClearTemporaryPages.Result",
+                                      0);
 }
 
 void OfflinePageRequestJobTest::SimulateHasNetworkConnectivity(bool online) {
@@ -709,54 +714,56 @@ std::unique_ptr<net::URLRequest> OfflinePageRequestJobTest::CreateRequest(
 
 void OfflinePageRequestJobTest::ExpectOneUniqueSampleForAggregatedRequestResult(
     OfflinePageRequestJob::AggregatedRequestResult result) {
-  histogram_tester_.ExpectUniqueSample(kAggregatedRequestResultHistogram,
-                                       static_cast<int>(result), 1);
+  histogram_tester_->ExpectUniqueSample(kAggregatedRequestResultHistogram,
+                                        static_cast<int>(result), 1);
 }
 
 void OfflinePageRequestJobTest::
     ExpectMultiUniqueSampleForAggregatedRequestResult(
         OfflinePageRequestJob::AggregatedRequestResult result,
         int count) {
-  histogram_tester_.ExpectUniqueSample(kAggregatedRequestResultHistogram,
-                                       static_cast<int>(result), count);
+  histogram_tester_->ExpectUniqueSample(kAggregatedRequestResultHistogram,
+                                        static_cast<int>(result), count);
 }
 
 void OfflinePageRequestJobTest::
     ExpectOneNonuniqueSampleForAggregatedRequestResult(
         OfflinePageRequestJob::AggregatedRequestResult result) {
-  histogram_tester_.ExpectBucketCount(kAggregatedRequestResultHistogram,
-                                      static_cast<int>(result), 1);
+  histogram_tester_->ExpectBucketCount(kAggregatedRequestResultHistogram,
+                                       static_cast<int>(result), 1);
 }
 
 void OfflinePageRequestJobTest::ExpectNoSamplesInAggregatedRequestResult() {
-  histogram_tester_.ExpectTotalCount(kAggregatedRequestResultHistogram, 0);
+  histogram_tester_->ExpectTotalCount(kAggregatedRequestResultHistogram, 0);
 }
 
 void OfflinePageRequestJobTest::ExpectOpenFileErrorCode(int result) {
-  histogram_tester_.ExpectUniqueSample(kOpenFileErrorCodeHistogram, -result, 1);
+  histogram_tester_->ExpectUniqueSample(kOpenFileErrorCodeHistogram, -result,
+                                        1);
 }
 
 void OfflinePageRequestJobTest::ExpectSeekFileErrorCode(int result) {
-  histogram_tester_.ExpectUniqueSample(kSeekFileErrorCodeHistogram, -result, 1);
+  histogram_tester_->ExpectUniqueSample(kSeekFileErrorCodeHistogram, -result,
+                                        1);
 }
 
 void OfflinePageRequestJobTest::ExpectAccessEntryPoint(
     OfflinePageRequestJob::AccessEntryPoint entry_point) {
-  histogram_tester_.ExpectUniqueSample(
+  histogram_tester_->ExpectUniqueSample(
       std::string(kAccessEntryPointHistogram) + kDownloadNamespace,
       static_cast<int>(entry_point), 1);
 }
 
 void OfflinePageRequestJobTest::ExpectNoAccessEntryPoint() {
   EXPECT_TRUE(
-      histogram_tester_.GetTotalCountsForPrefix(kAccessEntryPointHistogram)
+      histogram_tester_->GetTotalCountsForPrefix(kAccessEntryPointHistogram)
           .empty());
 }
 
 void OfflinePageRequestJobTest::ExpectOfflinePageSizeUniqueSample(
     int bucket,
     int count) {
-  histogram_tester_.ExpectUniqueSample(
+  histogram_tester_->ExpectUniqueSample(
       std::string(kPageSizeAccessOfflineHistogramBase) + kDownloadNamespace,
       bucket, count);
 }
@@ -765,7 +772,7 @@ void OfflinePageRequestJobTest::ExpectOfflinePageSizeTotalSuffixCount(
     int count) {
   int total_offline_count = 0;
   base::HistogramTester::CountsMap all_offline_counts =
-      histogram_tester_.GetTotalCountsForPrefix(
+      histogram_tester_->GetTotalCountsForPrefix(
           kPageSizeAccessOfflineHistogramBase);
   for (const std::pair<std::string, base::HistogramBase::Count>&
            namespace_and_count : all_offline_counts) {
@@ -779,7 +786,7 @@ void OfflinePageRequestJobTest::ExpectOfflinePageSizeTotalSuffixCount(
 void OfflinePageRequestJobTest::ExpectOnlinePageSizeUniqueSample(
     int bucket,
     int count) {
-  histogram_tester_.ExpectUniqueSample(
+  histogram_tester_->ExpectUniqueSample(
       std::string(kPageSizeAccessOnlineHistogramBase) + kDownloadNamespace,
       bucket, count);
 }
@@ -788,7 +795,7 @@ void OfflinePageRequestJobTest::ExpectOnlinePageSizeTotalSuffixCount(
     int count) {
   int online_count = 0;
   base::HistogramTester::CountsMap all_online_counts =
-      histogram_tester_.GetTotalCountsForPrefix(
+      histogram_tester_->GetTotalCountsForPrefix(
           kPageSizeAccessOnlineHistogramBase);
   for (const std::pair<std::string, base::HistogramBase::Count>&
            namespace_and_count : all_online_counts) {

components/offline_pages/core/model/offline_page_model_taskified.cc

@@ -141,6 +141,7 @@ OfflinePageModelTaskified::OfflinePageModelTaskified(
       clock_(std::move(clock)),
       task_queue_(this),
       skip_clearing_original_url_for_testing_(false),
+      skip_maintenance_tasks_for_testing_(false),
       task_runner_(task_runner),
       weak_ptr_factory_(this) {
   DCHECK_LT(kMaintenanceTasksDelay, OfflinePageMetadataStoreSQL::kClosingDelay);
@@ -553,6 +554,8 @@ void OfflinePageModelTaskified::RemoveFromDownloadManager(
 }
 
 void OfflinePageModelTaskified::ScheduleMaintenanceTasks() {
+  if (skip_maintenance_tasks_for_testing_)
+    return;
   // If not enough time has passed, don't queue maintenance tasks.
   base::Time now = GetCurrentTime();
   if (now - last_maintenance_tasks_schedule_time_ < kClearStorageInterval)
@@ -570,6 +573,7 @@ void OfflinePageModelTaskified::ScheduleMaintenanceTasks() {
 
 void OfflinePageModelTaskified::RunMaintenanceTasks(const base::Time now,
                                                     bool first_run) {
+  DCHECK(!skip_maintenance_tasks_for_testing_);
   // If this is the first run of this session, enqueue the run-once tasks.
   if (first_run) {
     // TODO(romax): When we have external directory, adding the support of

components/offline_pages/core/model/offline_page_model_taskified.h

@@ -145,6 +145,9 @@ class OfflinePageModelTaskified : public OfflinePageModel,
   void SetSkipClearingOriginalUrlForTesting() {
     skip_clearing_original_url_for_testing_ = true;
   }
+  void DoNotRunMaintenanceTasksForTesting() {
+    skip_maintenance_tasks_for_testing_ = true;
+  }
 
  private:
   // TODO(romax): https://crbug.com/791115, remove the friend class usage.
@@ -255,6 +258,11 @@ class OfflinePageModelTaskified : public OfflinePageModel,
   // model to skip saving original_urls.
   bool skip_clearing_original_url_for_testing_;
 
+  // For testing only.
+  // This flag controls the execution of maintenance tasks; when false they will
+  // not be executed.
+  bool skip_maintenance_tasks_for_testing_;
+
   const scoped_refptr<base::SequencedTaskRunner> task_runner_;
 
   base::WeakPtrFactory<OfflinePageModelTaskified> weak_ptr_factory_;

components/offline_pages/core/model/offline_page_model_taskified_unittest.cc

@@ -1447,4 +1447,33 @@ TEST_F(OfflinePageModelTaskifiedTest, ClearStorage) {
       "OfflinePages.ClearTemporaryPages.BatchSize", 0);
 }
 
+TEST_F(OfflinePageModelTaskifiedTest, MaintenanceTasksAreDisabled) {
+  // The maintenance tasks should not be executed when disabled by tests.
+  model()->DoNotRunMaintenanceTasksForTesting();
+
+  // With that setting GetAllPages and saving a page should not schedule
+  // maintenance tasks.
+  base::MockCallback<MultipleOfflinePageItemCallback> callback;
+  model()->GetAllPages(callback.Get());
+  auto archiver = BuildArchiver(kTestUrl, ArchiverResult::SUCCESSFULLY_CREATED);
+  SavePageWithExpectedResult(kTestUrl, kTestClientId1, kTestUrl2,
+                             kEmptyRequestOrigin, std::move(archiver),
+                             SavePageResult::SUCCESS);
+  PumpLoop();
+  EXPECT_EQ(base::Time(), last_maintenance_tasks_schedule_time());
+
+  // Advance the clock considerably and confirm no runs happened.
+  task_runner()->FastForwardBy(base::TimeDelta::FromDays(1));
+  PumpLoop();
+  EXPECT_EQ(base::Time(), last_maintenance_tasks_schedule_time());
+  histogram_tester()->ExpectTotalCount(
+      "OfflinePages.ClearTemporaryPages.Result", 0);
+  histogram_tester()->ExpectTotalCount(
+      "OfflinePages.ClearTemporaryPages.BatchSize", 0);
+  histogram_tester()->ExpectTotalCount(
+      "OfflinePages.ConsistencyCheck.Temporary.Result", 0);
+  histogram_tester()->ExpectTotalCount(
+      "OfflinePages.ConsistencyCheck.Persistent.Result", 0);
+}
+
 }  // namespace offline_pages