Chrome Cleaner UI: Add logs upload permission checkbox to the dialog

chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_win_unittest.cc

 // Copyright 2017 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/chrome_cleaner/chrome_cleaner_controller_win.h"
 
 #include <string>
 #include <tuple>
 #include <utility>
 
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/run_loop.h"
 #include "base/task_scheduler/post_task.h"
 #include "base/test/multiprocess_test.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/safe_browsing/chrome_cleaner/mock_chrome_cleaner_process_win.h"
 #include "chrome/browser/safe_browsing/chrome_cleaner/reporter_runner_win.h"
 #include "chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chrome/test/base/testing_profile_manager.h"
 #include "components/chrome_cleaner/public/constants/constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/test/test_browser_thread_bundle.h"
+#include "content/public/test/test_utils.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/multiprocess_func_list.h"
 
 namespace safe_browsing {
 namespace {
 
+using ::chrome_cleaner::mojom::PromptAcceptance;
 using ::testing::Combine;
 using ::testing::DoAll;
 using ::testing::InvokeWithoutArgs;
 using ::testing::SaveArg;
 using ::testing::StrictMock;
 using ::testing::UnorderedElementsAreArray;
 using ::testing::Values;
 using ::testing::_;
 using CrashPoint = MockChromeCleanerProcess::CrashPoint;
 using IdleReason = ChromeCleanerController::IdleReason;
 using State = ChromeCleanerController::State;
 using UserResponse = ChromeCleanerController::UserResponse;
 
+// Returns the PromptAcceptance value that ChromeCleanerController is supposed
+// to send to the Chrome Cleaner process when ReplyWithUserResponse() is
+// called with |user_response|.
+PromptAcceptance UserResponseToPromptAcceptance(UserResponse user_response) {
+  switch (user_response) {
+    case UserResponse::kAcceptedWithLogs:
+      return PromptAcceptance::ACCEPTED_WITH_LOGS;
+    case UserResponse::kAcceptedWithoutLogs:
+      return PromptAcceptance::ACCEPTED_WITHOUT_LOGS;
+    case UserResponse::kDenied:  // Fallthrough
+    case UserResponse::kDismissed:
+      return PromptAcceptance::DENIED;
+  }
+
+  NOTREACHED();
+  return PromptAcceptance::UNSPECIFIED;
+}
+
 class MockChromeCleanerControllerObserver
     : public ChromeCleanerController::Observer {
  public:
   MOCK_METHOD1(OnIdle, void(ChromeCleanerController::IdleReason));
   MOCK_METHOD0(OnScanning, void());
   MOCK_METHOD1(OnInfected, void(const std::set<base::FilePath>&));
   MOCK_METHOD1(OnCleaning, void(const std::set<base::FilePath>&));
   MOCK_METHOD0(OnRebootRequired, void());
+  MOCK_METHOD0(OnRebootFailed, void());
+  MOCK_METHOD1(OnLogsEnabledChanged, void(bool));
 };
 
 enum class MetricsStatus {
   kEnabled,
   kDisabled,
 };
 
 enum class ScoutStatus {
   kEnabled,
   kDisabled,
@@ skipping 65 matching lines @@
   // ChromeCleanerRunnerTestDelegate overrides.
 
   base::Process LaunchTestProcess(
       const base::CommandLine& command_line,
       const base::LaunchOptions& launch_options) override {
     command_line_ = command_line;
     // Return an invalid process.
     return base::Process();
   }
 
+  void OnCleanerProcessDone(
+      const ChromeCleanerRunner::ProcessStatus& process_status) override {}
+
  protected:
   // We need this because we need UI and IO threads during tests. The thread
   // bundle should be the first member of the class so that it will be destroyed
   // last.
   content::TestBrowserThreadBundle thread_bundle_;
   base::test::ScopedFeatureList scoped_feature_list_;
 
   bool metrics_enabled_;
   bool scout_enabled_;
   base::CommandLine command_line_;
@@ skipping 73 matching lines @@
 
   void SetUp() override {
     std::tie(process_status_, crash_point_, uws_found_status_, user_response_) =
         GetParam();
 
     cleaner_process_options_.SetDoFindUws(uws_found_status_ !=
                                           UwsFoundStatus::kNoUwsFound);
     cleaner_process_options_.set_reboot_required(
         uws_found_status_ == UwsFoundStatus::kUwsFoundRebootRequired);
     cleaner_process_options_.set_crash_point(crash_point_);
+    cleaner_process_options_.set_expected_user_response(
+        uws_found_status_ == UwsFoundStatus::kNoUwsFound
+            ? PromptAcceptance::DENIED
+            : UserResponseToPromptAcceptance(user_response_));
 
     controller_ = ChromeCleanerController::GetInstance();
     ASSERT_TRUE(controller_);
+    // Since tests may change the logs collection permissions, ensure that the
+    // controller always starts off with logs collection enabled.
+    controller_->SetLogsEnabled(true);

[proberge, 2017/06/29 13:55:53]

Shouldn't the entire controller be reset to a new singleton instance instead?

[alito, 2017/06/29 17:14:27]

Done.

 
     scoped_feature_list_.InitAndEnableFeature(kInBrowserCleanerUIFeature);
     SetChromeCleanerRunnerTestDelegateForTesting(this);
     controller_->SetDelegateForTesting(this);
   }
 
   void TearDown() override {
     if (controller_->state() == State::kRebootRequired)
       controller_->DismissRebootForTesting();

[proberge, 2017/06/29 17:58:36]

Are all instances of DismissRebootForTesting removed? If so, maybe we can clean
it from the controller.

[alito, 2017/06/29 18:13:51]

Done.

 
     controller_->SetDelegateForTesting(nullptr);
     SetChromeCleanerRunnerTestDelegateForTesting(nullptr);
   }
 
   // ChromeCleanerControllerDelegate overrides.
 
   void FetchAndVerifyChromeCleaner(FetchedCallback fetched_callback) override {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
@@ skipping 46 matching lines @@
         &test_process_command_line);
 
     base::SpawnChildResult result = base::SpawnMultiProcessTestChild(
         "MockChromeCleanerProcessMain", test_process_command_line,
         launch_options);
 
     EXPECT_TRUE(result.process.IsValid());
     return std::move(result.process);
   }
 
+  void OnCleanerProcessDone(
+      const ChromeCleanerRunner::ProcessStatus& process_status) override {
+    cleaner_process_status_ = process_status;
+  }
+
   ChromeCleanerController::State ExpectedFinalState() {
     if (process_status_ == CleanerProcessStatus::kFetchSuccessValidProcess &&
         crash_point_ == CrashPoint::kNone &&
         uws_found_status_ == UwsFoundStatus::kUwsFoundRebootRequired &&
-        user_response_ == UserResponse::kAccepted) {
+        (user_response_ == UserResponse::kAcceptedWithLogs ||
+         user_response_ == UserResponse::kAcceptedWithoutLogs)) {
       return State::kRebootRequired;
     }
     return State::kIdle;
   }
 
   bool ExpectedOnIdleCalled() { return ExpectedFinalState() == State::kIdle; }
 
   bool ExpectedOnInfectedCalled() {
     return process_status_ == CleanerProcessStatus::kFetchSuccessValidProcess &&
            crash_point_ != CrashPoint::kOnStartup &&
            crash_point_ != CrashPoint::kAfterConnection &&
            uws_found_status_ != UwsFoundStatus::kNoUwsFound;
   }
 
   bool ExpectedOnCleaningCalled() {
     return ExpectedOnInfectedCalled() &&
            crash_point_ != CrashPoint::kAfterRequestSent &&
-           user_response_ == UserResponse::kAccepted;
+           (user_response_ == UserResponse::kAcceptedWithLogs ||
+            user_response_ == UserResponse::kAcceptedWithoutLogs);
   }
 
   bool ExpectedOnRebootRequiredCalled() {
     return ExpectedFinalState() == State::kRebootRequired;
   }
 
   bool ExpectedUwsFound() { return ExpectedOnInfectedCalled(); }
 
   bool ExpectedToTagProfile() {
     return process_status_ == CleanerProcessStatus::kFetchSuccessValidProcess &&
            (crash_point_ == CrashPoint::kNone ||
             crash_point_ == CrashPoint::kAfterResponseReceived) &&
            (uws_found_status_ == UwsFoundStatus::kUwsFoundNoRebootRequired ||
             uws_found_status_ == UwsFoundStatus::kUwsFoundRebootRequired) &&
-           user_response_ == UserResponse::kAccepted;
+           (user_response_ == UserResponse::kAcceptedWithLogs ||
+            user_response_ == UserResponse::kAcceptedWithoutLogs);
   }
 
   bool ExpectedToResetSettings() {
     return process_status_ == CleanerProcessStatus::kFetchSuccessValidProcess &&
            crash_point_ == CrashPoint::kNone &&
            uws_found_status_ == UwsFoundStatus::kUwsFoundNoRebootRequired &&
-           user_response_ == UserResponse::kAccepted;
+           (user_response_ == UserResponse::kAcceptedWithLogs ||
+            user_response_ == UserResponse::kAcceptedWithoutLogs);
   }
 
   ChromeCleanerController::IdleReason ExpectedIdleReason() {
     EXPECT_EQ(ExpectedFinalState(), State::kIdle);
 
     if (process_status_ != CleanerProcessStatus::kFetchSuccessValidProcess ||
         crash_point_ == CrashPoint::kOnStartup ||
         crash_point_ == CrashPoint::kAfterConnection) {
       return IdleReason::kScanningFailed;
     }
 
     if (uws_found_status_ == UwsFoundStatus::kNoUwsFound)
       return IdleReason::kScanningFoundNothing;
 
     if (ExpectedOnInfectedCalled() &&
         (user_response_ == UserResponse::kDenied ||
          user_response_ == UserResponse::kDismissed)) {
       return IdleReason::kUserDeclinedCleanup;
     }
 
     if (ExpectedOnInfectedCalled() &&
-        user_response_ == UserResponse::kAccepted &&
+        (user_response_ == UserResponse::kAcceptedWithLogs ||
+         user_response_ == UserResponse::kAcceptedWithoutLogs) &&
         crash_point_ == CrashPoint::kAfterResponseReceived) {
       return IdleReason::kCleaningFailed;
     }
 
     return IdleReason::kCleaningSucceeded;
   }
 
  protected:
   // We need this because we need UI and IO threads during tests. The thread
   // bundle should be the first member of the class so that it will be destroyed
   // last.
   content::TestBrowserThreadBundle thread_bundle_;
   base::test::ScopedFeatureList scoped_feature_list_;
 
   CleanerProcessStatus process_status_;
   MockChromeCleanerProcess::CrashPoint crash_point_;
   UwsFoundStatus uws_found_status_;
   ChromeCleanerController::UserResponse user_response_;
 
   MockChromeCleanerProcess::Options cleaner_process_options_;
 
   StrictMock<MockChromeCleanerControllerObserver> mock_observer_;
   ChromeCleanerController* controller_;
+  ChromeCleanerRunner::ProcessStatus cleaner_process_status_;
 
   std::vector<Profile*> profiles_tagged_;
   std::vector<Profile*> profiles_to_reset_if_tagged_;
 };
 
 MULTIPROCESS_TEST_MAIN(MockChromeCleanerProcessMain) {
   base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
   MockChromeCleanerProcess::Options options;
   EXPECT_TRUE(MockChromeCleanerProcess::Options::FromCommandLine(*command_line,
                                                                  &options));
@@ skipping 48 matching lines @@
             InvokeWithoutArgs([&run_loop]() { run_loop.QuitWhenIdle(); }));
   }
 
   if (ExpectedOnInfectedCalled()) {
     EXPECT_CALL(mock_observer_, OnInfected(_))
         .WillOnce(DoAll(SaveArg<0>(&files_to_delete_on_infected),
                         InvokeWithoutArgs([this, profile1]() {
                           controller_->ReplyWithUserResponse(profile1,
                                                              user_response_);
                         })));
+    // Since logs upload is enabled by default, OnLogsEnabledChanged() will be
+    // called only if the user response is kAcceptedWithoutLogs.
+    if (user_response_ == UserResponse::kAcceptedWithoutLogs)
+      EXPECT_CALL(mock_observer_, OnLogsEnabledChanged(false));
   }
 
   if (ExpectedOnCleaningCalled()) {
     EXPECT_CALL(mock_observer_, OnCleaning(_))
         .WillOnce(SaveArg<0>(&files_to_delete_on_cleaning));
   }
 
   if (ExpectedOnRebootRequiredCalled()) {
     EXPECT_CALL(mock_observer_, OnRebootRequired())
         .WillOnce(
             InvokeWithoutArgs([&run_loop]() { run_loop.QuitWhenIdle(); }));
   }
 
   // Assert here that we expect at least one of OnIdle or OnRebootRequired to be
   // called, since otherwise, the test is set up incorrectly and is expected to
   // never stop.
   ASSERT_TRUE(ExpectedOnIdleCalled() || ExpectedOnRebootRequiredCalled());
   run_loop.Run();
+  // Also ensure that we wait until the mock cleaner process has finished and
+  // that all tasks that posted by ChromeCleanerRunner have run.
+  content::RunAllBlockingPoolTasksUntilIdle();
 
+  EXPECT_NE(cleaner_process_status_.exit_code,
+            MockChromeCleanerProcess::kInternalTestFailureExitCode);
   EXPECT_EQ(controller_->state(), ExpectedFinalState());
   EXPECT_EQ(!files_to_delete_on_infected.empty(), ExpectedUwsFound());
   EXPECT_EQ(!files_to_delete_on_cleaning.empty(),
             ExpectedUwsFound() && ExpectedOnCleaningCalled());
   if (!files_to_delete_on_infected.empty() &&
       !files_to_delete_on_cleaning.empty()) {
     EXPECT_EQ(files_to_delete_on_infected, files_to_delete_on_cleaning);
   }
 
   std::vector<Profile*> expected_tagged;
@@ skipping 21 matching lines @@
             Values(CrashPoint::kNone,
                    CrashPoint::kOnStartup,
                    CrashPoint::kAfterConnection,
                    // CrashPoint::kAfterRequestSent is not used because we
                    // cannot ensure the order between the Mojo request being
                    // received by Chrome and the connection being lost.
                    CrashPoint::kAfterResponseReceived),
             Values(UwsFoundStatus::kNoUwsFound,
                    UwsFoundStatus::kUwsFoundRebootRequired,
                    UwsFoundStatus::kUwsFoundNoRebootRequired),
-            Values(UserResponse::kAccepted,
+            Values(UserResponse::kAcceptedWithLogs,
+                   UserResponse::kAcceptedWithoutLogs,
                    UserResponse::kDenied,
                    UserResponse::kDismissed)));
 
 }  // namespace
 }  // namespace safe_browsing