Post-cleanup settings reset.

chrome/browser/safe_browsing/chrome_cleaner/settings_resetter_browsertest_win.cc

Index: chrome/browser/safe_browsing/chrome_cleaner/settings_resetter_browsertest_win.cc
diff --git a/chrome/browser/safe_browsing/chrome_cleaner/settings_resetter_browsertest_win.cc b/chrome/browser/safe_browsing/chrome_cleaner/settings_resetter_browsertest_win.cc
new file mode 100644
index 0000000000000000000000000000000000000000..d12cfde3d1ffdfdd2ed1d8340df3b466e50aafbc
--- /dev/null
+++ b/chrome/browser/safe_browsing/chrome_cleaner/settings_resetter_browsertest_win.cc
@@ -0,0 +1,239 @@
+// 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/settings_resetter_win.h"
+
+#include <memory>
+
+#include "base/run_loop.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/test/test_reg_util_win.h"
+#include "base/win/registry.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profile_resetter/brandcoded_default_settings.h"
+#include "chrome/browser/profile_resetter/profile_resetter.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/safe_browsing/chrome_cleaner/srt_field_trial_win.h"
+#include "chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_test_utils.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "components/chrome_cleaner/public/constants/constants.h"
+#include "components/prefs/pref_service.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace safe_browsing {
+namespace {
+
+using ::testing::_;
+using ::testing::StrictMock;
+
+// Callback for CreateProfile() that assigns |profile| to |*out_profile|
+// if the profile creation is successful.
+void CreateProfileCallback(Profile** out_profile,
+                           const base::Closure& closure,
+                           Profile* profile,
+                           Profile::CreateStatus status) {
+  DCHECK(out_profile);
+  if (status == Profile::CREATE_STATUS_INITIALIZED)
+    *out_profile = profile;
+  closure.Run();
+}
+
+// Creates a new profile from the UI thread.
+Profile* CreateProfile() {
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  Profile* profile = nullptr;
+  base::RunLoop run_loop;
+  profile_manager->CreateProfileAsync(
+      profile_manager->GenerateNextProfileDirectoryPath(),
+      base::Bind(&CreateProfileCallback, &profile, run_loop.QuitClosure()),
+      base::string16(), std::string(), std::string());
+  run_loop.Run();
+  return profile;
+}
+
+// Returns true if |profile| is tagged for settings reset.
+bool ProfileIsTagged(Profile* profile) {
+  return profile->GetPrefs()->GetBoolean(prefs::kChromeCleanerResetPending);
+}
+
+// If |value| is positive, saves it in the registry at the Chrome Cleaner
+// registry key under |value_name|.
+void SetValueIfPositive(const wchar_t* value_name, int64_t value) {
+  if (value <= 0)
+    return;
+
+  base::string16 cleaner_key_path(
+      chrome_cleaner::kSoftwareRemovalToolRegistryKey);
+  cleaner_key_path.append(L"\\").append(chrome_cleaner::kCleanerSubKey);
+
+  LONG result = base::win::RegKey(HKEY_CURRENT_USER, cleaner_key_path.c_str(),
+                                  KEY_SET_VALUE)
+                    .WriteValue(value_name, &value, sizeof(value), REG_QWORD);
+  ASSERT_EQ(ERROR_SUCCESS, result);
+}
+
+// Params:
+//  - bool in_browser_cleaner_ui: if true, consider that kInBrowserCleanerUI

[alito, 2017/06/09 01:52:07]

If you create a single test for the Create() function to ensure that is returns
non-null iff the in-browser feature is  enabled, then you can get rid of this
parameter in all subsequent tests (just always enable the feature and assert
that Create returns non-null).

[ftirelo, 2017/06/09 21:24:55]

Excellent idea. Done.

+//                                feature is enabled.
+class TagProfileForResettingTest : public InProcessBrowserTest,
+                                   public ::testing::WithParamInterface<bool> {
+ public:
+  void SetUpInProcessBrowserTestFixture() override {
+    InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
+
+    in_browser_cleaner_ui_ = GetParam();
+    if (in_browser_cleaner_ui_)
+      scoped_feature_list_.InitAndEnableFeature(kInBrowserCleanerUIFeature);
+    else
+      scoped_feature_list_.InitAndDisableFeature(kInBrowserCleanerUIFeature);
+  }
+
+ protected:
+  base::test::ScopedFeatureList scoped_feature_list_;
+  bool in_browser_cleaner_ui_;
+};
+
+IN_PROC_BROWSER_TEST_P(TagProfileForResettingTest, Run) {
+  Browser* browser = chrome::FindLastActive();
+  ASSERT_TRUE(browser);
+  Profile* profile = browser->profile();
+  ASSERT_TRUE(profile);
+
+  std::unique_ptr<PostCleanupSettingsResetter> resetter(
+      PostCleanupSettingsResetter::Create());
+  if (!in_browser_cleaner_ui_) {
+    ASSERT_FALSE(resetter);
+    return;
+  }
+
+  ASSERT_TRUE(resetter);
+  resetter->TagForResetting(profile);
+  EXPECT_EQ(in_browser_cleaner_ui_, ProfileIsTagged(profile));
+}
+
+INSTANTIATE_TEST_CASE_P(Default, TagProfileForResettingTest, testing::Bool());
+
+class SettingsResetterTestDelegate
+    : public PostCleanupSettingsResetter::Delegate {
+ public:
+  explicit SettingsResetterTestDelegate(int* num_resets)
+      : num_resets_(num_resets) {}
+  ~SettingsResetterTestDelegate() override = default;
+
+  void FetchDefaultSettings(
+      DefaultSettingsFetcher::SettingsCallback callback) override {
+    callback.Run(base::MakeUnique<BrandcodedDefaultSettings>());
+  }
+
+  // Returns a MockProfileResetter that requires Reset() be called.
+  std::unique_ptr<ProfileResetter> GetProfileResetter(
+      Profile* profile) override {
+    ++(*num_resets_);
+    auto mock_profile_resetter =
+        base::MakeUnique<StrictMock<MockProfileResetter>>(profile);
+    EXPECT_CALL(*mock_profile_resetter, MockReset(_, _, _));
+    return std::move(mock_profile_resetter);
+  }
+
+ private:
+  int* num_resets_;
+
+  DISALLOW_COPY_AND_ASSIGN(SettingsResetterTestDelegate);
+};
+
+const struct SettingsResetTestParams {
+  int64_t ts_cleanup_started;
+  int64_t ts_cleanup_completed;
+  bool settings_reset;
+} kSettingsResetTestParams[] = {{0, 0, false},
+                                {10000, 0, false},
+                                {0, 20000, false},
+                                {20000, 10000, false},
+                                {10000, 20000, true}};
+
+// Params:
+//  - SettingsResetTestParams params: values to write to the registry and
+//                                    whether settings should be reset.
+//  - bool in_browser_cleaner_ui: if true, consider that kInBrowserCleanerUI
+//                                feature is enabled.
+class ResetPostCleanupSettingsIfTaggedTest
+    : public InProcessBrowserTest,
+      public ::testing::WithParamInterface<
+          std::tuple<bool, SettingsResetTestParams>> {
+ public:
+  void SetUpInProcessBrowserTestFixture() override {
+    InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
+
+    std::tie(in_browser_cleaner_ui_, params_) = GetParam();
+    if (in_browser_cleaner_ui_)
+      scoped_feature_list_.InitAndEnableFeature(kInBrowserCleanerUIFeature);
+    else
+      scoped_feature_list_.InitAndDisableFeature(kInBrowserCleanerUIFeature);
+  }
+
+ protected:
+  // Test params.
+  SettingsResetTestParams params_;
+  bool in_browser_cleaner_ui_;
+
+  base::test::ScopedFeatureList scoped_feature_list_;
+  registry_util::RegistryOverrideManager registry_override_manager_;
+};
+
+IN_PROC_BROWSER_TEST_P(ResetPostCleanupSettingsIfTaggedTest, Run) {
+  ASSERT_NO_FATAL_FAILURE(
+      registry_override_manager_.OverrideRegistry(HKEY_CURRENT_USER));
+  SetValueIfPositive(chrome_cleaner::kCleanupStartedTimestampValueName,
+                     params_.ts_cleanup_started);
+  SetValueIfPositive(chrome_cleaner::kCleanupConfirmedTimestampValueName,
+                     params_.ts_cleanup_completed);
+
+  // Profile objects are owned by ProfileManager.
+  Profile* profile1 = CreateProfile();
+  ASSERT_TRUE(profile1);
+  Profile* profile2 = CreateProfile();
+  ASSERT_TRUE(profile2);
+  Profile* profile3 = CreateProfile();
+  ASSERT_TRUE(profile3);
+
+  profile1->GetPrefs()->SetBoolean(prefs::kChromeCleanerResetPending, true);
+  profile3->GetPrefs()->SetBoolean(prefs::kChromeCleanerResetPending, true);
+
+  int num_resets = 0;
+  auto delegate = base::MakeUnique<SettingsResetterTestDelegate>(&num_resets);
+
+  std::unique_ptr<PostCleanupSettingsResetter> resetter(
+      PostCleanupSettingsResetter::Create());
+  if (!in_browser_cleaner_ui_) {
+    ASSERT_FALSE(resetter);
+    return;
+  }
+
+  ASSERT_TRUE(resetter);
+  base::RunLoop run_loop_for_reset;
+  resetter->ResetTaggedProfiles({profile1, profile2, profile3},
+                                run_loop_for_reset.QuitClosure(),
+                                std::move(delegate));
+  run_loop_for_reset.Run();
+
+  // If settings reset is on, profiles 1 and 3 should be reset.

[alito, 2017/06/09 01:52:07]

Now that the Create() function returns a null ptr when the feature is not
enabled, we cannot reach this code if the reset settings is not on. So at this
point profiles 1 and 3 should always be reset.

[ftirelo, 2017/06/09 21:24:55]

In fact, it depends on the registry entries above too. Added a comment.

[alito, 2017/06/09 23:24:08]

Acknowledged.

+  EXPECT_EQ(params_.settings_reset ? 2 : 0, num_resets);
+  EXPECT_EQ(!params_.settings_reset, ProfileIsTagged(profile1));
+  EXPECT_EQ(false, ProfileIsTagged(profile2));
+  EXPECT_EQ(!params_.settings_reset, ProfileIsTagged(profile3));
+}
+
+INSTANTIATE_TEST_CASE_P(
+    Default,
+    ResetPostCleanupSettingsIfTaggedTest,
+    testing::Combine(testing::Bool(),
+                     testing::ValuesIn(kSettingsResetTestParams)));
+
+}  // namespace
+}  // namespace safe_browsing