Update SRTFetcher browser test to use ScopedMockTimeMessageLoopTaskRunner

chrome/browser/safe_browsing/srt_fetcher_win.cc

 // Copyright 2015 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 "chrome/browser/safe_browsing/srt_fetcher_win.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <memory>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/debug/leak_annotations.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/process/launch.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task_runner_util.h"
+#include "base/task_scheduler/post_task.h"
+#include "base/task_scheduler/task_traits.h"
 #include "base/time/time.h"
+#include "base/version.h"
 #include "base/win/registry.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/browser/safe_browsing/srt_client_info_win.h"
 #include "chrome/browser/safe_browsing/srt_global_error_win.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_list_observer.h"
@@ skipping 665 matching lines @@
   prefs->SetString(prefs::kSwReporterPromptVersion,
                    reporter_version.GetString());
 
   // Download the SRT.
   RecordReporterStepHistogram(SW_REPORTER_DOWNLOAD_START);
 
   // All the work happens in the self-deleting class below.
   new SRTFetcher(profile);
 }
 
+base::Time Now() {
+  if (g_testing_delegate_)
+    return g_testing_delegate_->Now();

[Sorin Jianu, 2017/02/23 20:39:04]

could use ?:

[Joe Mason, 2017/02/24 17:35:29]

Done.

+  return base::Time::Now();
+}
+
 }  // namespace
 
 // This class tries to run a queue of reporters and react to their exit codes.
 // It schedules subsequent runs of the queue as needed, or retries as soon as a
 // browser is available when none is on first try.
 class ReporterRunner : public chrome::BrowserListObserver {
  public:
   // Registers |invocations| to run next time |TryToRun| is scheduled. (And if
   // it's not already scheduled, call it now.)
-  static void ScheduleInvocations(
-      const SwReporterQueue& invocations,
-      const base::Version& version,
-      scoped_refptr<base::TaskRunner> main_thread_task_runner,
-      scoped_refptr<base::TaskRunner> blocking_task_runner) {
+  static void ScheduleInvocations(const SwReporterQueue& invocations,
+                                  const base::Version& version) {
     if (!instance_) {
       instance_ = new ReporterRunner;
       ANNOTATE_LEAKING_OBJECT_PTR(instance_);
     }
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
     // There's nothing to do if the invocation parameters and version of the
     // reporter have not changed, we just keep running the tasks that are
     // running now.
     if (instance_->pending_invocations_ == invocations &&
         instance_->version_.IsValid() && instance_->version_ == version)
       return;
 
     instance_->pending_invocations_ = invocations;
     instance_->version_ = version;
-    instance_->main_thread_task_runner_ = std::move(main_thread_task_runner);
-    instance_->blocking_task_runner_ = std::move(blocking_task_runner);
-
     if (instance_->first_run_) {
       instance_->first_run_ = false;
       instance_->TryToRun();
     }
   }
 
  private:
   ReporterRunner() {}
   ~ReporterRunner() override {}
 
   // BrowserListObserver.
   void OnBrowserSetLastActive(Browser* browser) override {}
   void OnBrowserRemoved(Browser* browser) override {}
   void OnBrowserAdded(Browser* browser) override {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
     DCHECK(browser);
     MaybeFetchSRT(browser, version_);
     BrowserList::RemoveObserver(this);
   }
 
   // Launches the command line at the head of the queue.
   void ScheduleNextInvocation() {
     DCHECK(!current_invocations_.empty());
     auto next_invocation = current_invocations_.front();
     current_invocations_.pop();
 
-    if (g_testing_delegate_)
-      g_testing_delegate_->NotifyLaunchReady();
-
     AppendInvocationSpecificSwitches(&next_invocation);
 
-    // It's OK to simply |PostTaskAndReplyWithResult| so that
-    // |LaunchAndWaitForExit| doesn't need to access |main_thread_task_runner_|
-    // since the callback is not delayed and the test task runner won't need to
-    // force it.
+    base::TaskRunner* task_runner =
+        g_testing_delegate_ ? g_testing_delegate_->BlockingTaskRunner()
+                            : blocking_task_runner_.get();
     base::PostTaskAndReplyWithResult(
-        blocking_task_runner_.get(), FROM_HERE,
+        task_runner, FROM_HERE,
         base::Bind(&LaunchAndWaitForExit, next_invocation),
-        base::Bind(&ReporterRunner::ReporterDone, base::Unretained(this),
-                   base::Time::Now(), version_, next_invocation));
+        base::Bind(&ReporterRunner::ReporterDone, base::Unretained(this), Now(),
+                   version_, next_invocation));
   }
 
   // This method is called on the UI thread when an invocation of the reporter
   // has completed. This is run as a task posted from an interruptible worker
   // thread so should be resilient to unexpected shutdown.
   void ReporterDone(const base::Time& reporter_start_time,
                     const base::Version& version,
                     const SwReporterInvocation& finished_invocation,
                     int exit_code) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-    if (g_testing_delegate_)
-      g_testing_delegate_->NotifyReporterDone();
-
-    base::TimeDelta reporter_running_time =
-        base::Time::Now() - reporter_start_time;
+    base::Time now = Now();
+    base::TimeDelta reporter_running_time = now - reporter_start_time;
 
     // Don't continue the current queue of reporters if one failed to launch.
     if (exit_code == kReporterFailureExitCode)
       current_invocations_ = SwReporterQueue();
 
     // As soon as we're not running this queue, schedule the next overall queue
     // run after the regular delay. (If there was a failure it's not worth
     // retrying earlier, risking running too often if it always fails, since
     // not many users fail here.)
     if (current_invocations_.empty()) {
-      main_thread_task_runner_->PostDelayedTask(
+      base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
           FROM_HERE,
           base::Bind(&ReporterRunner::TryToRun, base::Unretained(this)),
           base::TimeDelta::FromDays(days_between_reporter_runs_));
     } else {
       ScheduleNextInvocation();
     }
 
     // If the reporter failed to launch, do not process the results. (The exit
     // code itself doesn't need to be logged in this case because
     // SW_REPORTER_FAILED_TO_START is logged in |LaunchAndWaitForExit|.)
     if (exit_code == kReporterFailureExitCode)
       return;
 
     UMAHistogramReporter uma(finished_invocation.suffix);
     uma.ReportVersion(version);
     uma.ReportExitCode(exit_code);
     uma.ReportEngineErrorCode();
     uma.ReportFoundUwS(finished_invocation.BehaviourIsSupported(
         SwReporterInvocation::BEHAVIOUR_LOG_TO_RAPPOR));
 
     PrefService* local_state = g_browser_process->local_state();
     if (local_state) {
       if (finished_invocation.BehaviourIsSupported(
               SwReporterInvocation::BEHAVIOUR_LOG_EXIT_CODE_TO_PREFS)) {
         local_state->SetInteger(prefs::kSwReporterLastExitCode, exit_code);
       }
       local_state->SetInt64(prefs::kSwReporterLastTimeTriggered,
-                            base::Time::Now().ToInternalValue());
+                            now.ToInternalValue());
     }
     uma.ReportRuntime(reporter_running_time);
     uma.ReportScanTimes();
     uma.ReportMemoryUsage();
     if (finished_invocation.logs_upload_enabled)
       uma.RecordLogsUploadResult();
 
     if (!finished_invocation.BehaviourIsSupported(
             SwReporterInvocation::BEHAVIOUR_TRIGGER_PROMPT)) {
       return;
@@ skipping 42 matching lines @@
 
     // Run a queue of reporters if none have been triggered in the last
     // |days_between_reporter_runs_| days, which depends if there is a pending
     // prompt to be added to Chrome's menu.
     if (local_state->GetBoolean(prefs::kSwReporterPendingPrompt)) {
       days_between_reporter_runs_ = kDaysBetweenSwReporterRunsForPendingPrompt;
       RecordReporterStepHistogram(SW_REPORTER_RAN_DAILY);
     } else {
       days_between_reporter_runs_ = kDaysBetweenSuccessfulSwReporterRuns;
     }
-    const base::Time now = base::Time::Now();
+    const base::Time now = Now();
     const base::Time last_time_triggered = base::Time::FromInternalValue(
         local_state->GetInt64(prefs::kSwReporterLastTimeTriggered));
     const base::Time next_trigger(
         last_time_triggered +
         base::TimeDelta::FromDays(days_between_reporter_runs_));
     if (!pending_invocations_.empty() &&
         (next_trigger <= now ||
          // Also make sure the kSwReporterLastTimeTriggered value is not set in
          // the future.
          last_time_triggered > now)) {
       const base::Time last_time_sent_logs = base::Time::FromInternalValue(
           local_state->GetInt64(prefs::kSwReporterLastTimeSentReport));
       const base::Time next_time_send_logs =
           last_time_sent_logs +
           base::TimeDelta::FromDays(kDaysBetweenReporterLogsSent);
       // Send the logs for this whole queue of invocations if the last send is
       // in the future or if logs have been sent at least
       // |kSwReporterLastTimeSentReport| days ago. The former is intended as a
       // measure for failure recovery, in case the time in local state is
       // incorrectly set to the future.
       in_logs_upload_period_ =
           last_time_sent_logs > now || next_time_send_logs <= now;
 
       DCHECK(current_invocations_.empty());
       current_invocations_ = pending_invocations_;
       ScheduleNextInvocation();
     } else {
-      main_thread_task_runner_->PostDelayedTask(
+      base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
           FROM_HERE,
           base::Bind(&ReporterRunner::TryToRun, base::Unretained(this)),
           next_trigger - now);
     }
   }
 
   // Returns true if the experiment to send reporter logs is enabled, the user
   // opted into Safe Browsing extended reporting, and this queue of invocations
   // started during the logs upload interval.
   bool ShouldSendReporterLogs(const std::string& suffix,
@@ skipping 26 matching lines @@
         local_state &&
         ShouldSendReporterLogs(next_invocation->suffix, *local_state)) {
       next_invocation->logs_upload_enabled = true;
       AddSwitchesForExtendedReportingUser(next_invocation);
       // Set the local state value before the first attempt to run the
       // reporter, because we only want to upload logs once in the window
       // defined by |kDaysBetweenReporterLogsSent|. If we set with other local
       // state values after the reporter runs, we could send logs again too
       // quickly (for example, if Chrome stops before the reporter finishes).
       local_state->SetInt64(prefs::kSwReporterLastTimeSentReport,
-                            base::Time::Now().ToInternalValue());
+                            Now().ToInternalValue());
     }
 
     if (ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled())
       next_invocation->command_line.AppendSwitch(kEnableCrashReporting);
   }
 
   // Adds switches to be sent to the Software Reporter when the user opted into
   // extended Safe Browsing reporting and is not incognito.
   void AddSwitchesForExtendedReportingUser(SwReporterInvocation* invocation) {
     invocation->command_line.AppendSwitch(kExtendedSafeBrowsingEnabledSwitch);
     invocation->command_line.AppendSwitchASCII(
         kChromeVersionSwitch, version_info::GetVersionNumber());
     invocation->command_line.AppendSwitchNative(
         kChromeChannelSwitch, base::IntToString16(ChannelAsInt()));
   }
 
   bool first_run_ = true;
 
   // The queue of invocations that are currently running.
   SwReporterQueue current_invocations_;
 
   // The invocations to run next time the SwReporter is run.
   SwReporterQueue pending_invocations_;
 
   base::Version version_;
-  scoped_refptr<base::TaskRunner> main_thread_task_runner_;
-  scoped_refptr<base::TaskRunner> blocking_task_runner_;
+
+  scoped_refptr<base::TaskRunner> blocking_task_runner_ =
+      base::CreateTaskRunnerWithTraits(

[Jialiu Lin, 2017/02/23 18:28:00]

nit: maybe put this initialization logic inside the constructor?

[gab, 2017/02/23 19:01:51]

I this and think we should use C++11 member initialization as much as possible
(one less place to look at to know the initial value of things -- and also much
easier to remember to init new members).

[gab, 2017/02/23 19:02:25]

I *like* this (i.e. the code as-is in this patch set)

[Joe Mason, 2017/02/24 17:35:29]

I agree.

+          // LaunchAndWaitForExit() creates (MayBlock()) and joins
+          // (WithBaseSyncPrimitives()) a process.
+          base::TaskTraits()
+              .WithShutdownBehavior(
+                  base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN)
+              .WithPriority(base::TaskPriority::BACKGROUND)
+              .MayBlock()
+              .WithBaseSyncPrimitives());
 
   // This value is used to identify how long to wait before starting a new run
   // of the reporter queue. It's initialized with the default value and may be
   // changed to a different value when a prompt is pending and the reporter
   // should be run before adding the global error to the Chrome menu.
   int days_between_reporter_runs_ = kDaysBetweenSuccessfulSwReporterRuns;
 
   // This will be true if the current queue of invocations started at a time
   // when logs should be uploaded.
   bool in_logs_upload_period_ = false;
@@ skipping 29 matching lines @@
          supported_behaviours == other.supported_behaviours &&
          logs_upload_enabled == other.logs_upload_enabled;
 }
 
 bool SwReporterInvocation::BehaviourIsSupported(
     SwReporterInvocation::Behaviours intended_behaviour) const {
   return (supported_behaviours & intended_behaviour) != 0;
 }
 
 void RunSwReporters(const SwReporterQueue& invocations,
-                    const base::Version& version,
-                    scoped_refptr<base::TaskRunner> main_thread_task_runner,
-                    scoped_refptr<base::TaskRunner> blocking_task_runner) {
+                    const base::Version& version) {
   DCHECK(!invocations.empty());
   DCHECK(version.IsValid());
-  ReporterRunner::ScheduleInvocations(invocations, version,
-                                      std::move(main_thread_task_runner),
-                                      std::move(blocking_task_runner));
+  ReporterRunner::ScheduleInvocations(invocations, version);
 }
 
 bool ReporterFoundUws() {
   PrefService* local_state = g_browser_process->local_state();
   if (!local_state)
     return false;
   int exit_code = local_state->GetInteger(prefs::kSwReporterLastExitCode);
   return exit_code == kSwReporterCleanupNeeded;
 }
 
 bool UserHasRunCleaner() {
   base::string16 cleaner_key_path(kSoftwareRemovalToolRegistryKey);
   cleaner_key_path.append(L"\\").append(kCleanerSubKey);
 
   base::win::RegKey srt_cleaner_key;
   return srt_cleaner_key.Open(HKEY_CURRENT_USER, cleaner_key_path.c_str(),
                               KEY_QUERY_VALUE) == ERROR_SUCCESS &&
          srt_cleaner_key.GetValueCount() > 0;
 }
 
 void SetSwReporterTestingDelegate(SwReporterTestingDelegate* delegate) {
   g_testing_delegate_ = delegate;
 }
 
 }  // namespace safe_browsing