Adds error handling support for the SwReporter launcher.

chrome/browser/safe_browsing/srt_fetcher_win.h

 // 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.
 
 #ifndef CHROME_BROWSER_SAFE_BROWSING_SRT_FETCHER_WIN_H_
 #define CHROME_BROWSER_SAFE_BROWSING_SRT_FETCHER_WIN_H_
 
 #include <limits.h>
 #include <stdint.h>
 
 #include <queue>
 #include <string>
 
 #include "base/command_line.h"
 #include "base/feature_list.h"
 #include "base/process/launch.h"
 #include "base/process/process.h"
 #include "base/time/time.h"
+#include "chrome/browser/safe_browsing/srt_chrome_prompt_impl.h"

[grt (UTC plus 2), 2017/04/25 07:06:40]

can you replace this with a forward decl for ChromePromptImpl and #include
"components/chrome_cleaner/public/interfaces/chrome_prompt.mojom.h"?

[ftirelo, 2017/04/25 22:24:05]

Done.

 
 namespace base {
 class FilePath;
 class TaskRunner;
 class Version;
 }
 
 class Browser;
 
 namespace safe_browsing {
 
 // A special exit code identifying a failure to run the reporter.
-const int kReporterFailureExitCode = INT_MAX;
+const int kReporterNotLaunchedExitCode = INT_MAX;
 
 // The number of days to wait before triggering another reporter run.
 const int kDaysBetweenSuccessfulSwReporterRuns = 7;
 const int kDaysBetweenSwReporterRunsForPendingPrompt = 1;
 // The number of days to wait before sending out reporter logs.
 const int kDaysBetweenReporterLogsSent = 7;
 
 // When enabled, moves all user interaction with the Software Reporter and the
 // Chrome Cleanup tool to Chrome.
 extern const base::Feature kInBrowserCleanerUIFeature;
@@ skipping 54 matching lines @@
 // recent Reporter run terminated with an exit code indicating the presence of
 // UwS.
 bool ReporterFoundUws();
 
 // Returns true iff a valid registry key for the SRT Cleaner exists, and that
 // key is nonempty.
 // TODO(tmartino): Consider changing to check whether the user has recently
 // run the cleaner, rather than checking if they've run it at all.
 bool UserHasRunCleaner();
 
 // Mocks and callbacks for the unit tests.

[grt (UTC plus 2), 2017/04/25 07:06:40]

nittynit:

// A delegate used by tests to implement test doubles (e.g., stubs, fakes, or
mocks).

[ftirelo, 2017/04/25 22:24:05]

Done.

 class SwReporterTestingDelegate {
  public:
   virtual ~SwReporterTestingDelegate() {}
 
   // Test mock for launching the reporter.

[grt (UTC plus 2), 2017/04/25 07:06:40]

the use of "mock" here deviates from normal naming conventions. since this is
just an interface, neither it nor its methods are mocks in and of themselves
(go/mock-objects#mocks-stubs-fakes). i think it's best to document when each of
these will be called and what the expectation is. for example:

// Invoked during tests in place of base::LaunchProcess; see
SwReporterProcess::LaunchReporterProcess.

[ftirelo, 2017/04/25 22:24:05]

Done.

   virtual base::Process LaunchReporter(
       const SwReporterInvocation& invocation,
       const base::LaunchOptions& launch_options) = 0;
 
   // Test mock for showing the prompt.
   virtual void TriggerPrompt(Browser* browser,
                              const std::string& reporter_version) = 0;
 
   // Returns the test's idea of the current time.
   virtual base::Time Now() const = 0;
 
   // A task runner used to spawn the reporter process (which blocks).
   virtual base::TaskRunner* BlockingTaskRunner() const = 0;
+
+  // Returns a ChromePromptImpl object that keeps track of specific actions
+  // during execution.
+  virtual std::unique_ptr<ChromePromptImpl> CreateChromePromptImpl(
+      chrome_cleaner::mojom::ChromePromptRequest request) = 0;
+
+  // Handles the case when the remote end has been closed, by performing the
+  // necessary cleanups if the prompt dialog is being shown to the user.
+  virtual void OnConnectionClosed() = 0;
+
+  // Handles the case when a mojo::ReportBadMessage has been explicitly
+  // reported.
+  virtual void OnConnectionError(const std::string& message) = 0;
 };
 
 // Set a delegate for testing. The implementation will not take ownership of
 // |delegate| - it must remain valid until this function is called again to
 // reset the delegate. If |delegate| is nullptr, any previous delegate is
 // cleared.
 void SetSwReporterTestingDelegate(SwReporterTestingDelegate* delegate);
 
 void DisplaySRTPromptForTesting(const base::FilePath& download_path);
 
 }  // namespace safe_browsing
 
 #endif  // CHROME_BROWSER_SAFE_BROWSING_SRT_FETCHER_WIN_H_