Post-cleanup settings reset.

chrome/browser/safe_browsing/chrome_cleaner/settings_resetter_win.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/settings_resetter_win.h"
+
+#include <algorithm>
+#include <iterator>
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "base/barrier_closure.h"
+#include "base/bind_helpers.h"
+#include "base/callback_helpers.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/synchronization/lock.h"
+#include "base/win/registry.h"
+#include "chrome/browser/browser_process.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/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/common/pref_names.h"
+#include "components/chrome_cleaner/public/constants/constants.h"
+#include "components/pref_registry/pref_registry_syncable.h"
+#include "components/prefs/pref_service.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace safe_browsing {
+
+namespace {
+
+// Manages post-cleanup settings reset for a list of profiles.
+// An instance of this class is created by ResetPostCleanupSettingsIfTagged()
+// and will self-delete once all profiles in the list have been reset.
+class SettingsResetter : public base::RefCounted<SettingsResetter> {
+ public:
+  SettingsResetter(std::vector<Profile*> profiles_to_reset,
+                   std::unique_ptr<SettingsResetterDelegate> delegate,
+                   base::OnceClosure done_callback);
+
+  // Resets settings for all profiles in |profiles_to_reset__| and invokes

[alito, 2017/06/07 19:54:03]

nit: remove extra _

[ftirelo, 2017/06/08 21:54:11]

Done.

+  // |done_callback_| when done.
+  void Run();
+
+ protected:
+  virtual ~SettingsResetter();
+
+ private:
+  friend class base::RefCounted<SettingsResetter>;
+
+  // Resets settings for |profile| according to default values given by
+  // |master_settings|. Used as a callback for
+  // DefaultSettingsFetcher::FetchDefaultSettings().
+  void OnFetchCompleted(
+      Profile* profile,
+      std::unique_ptr<BrandcodedDefaultSettings> master_settings);
+
+  // Removes the settings reset tag for |profile|. If there are no more
+  // profiles to reset, invokes |done_callback_| and deletes this object.
+  void OnResetCompleted(Profile* profile);
+
+  // The profiles to be reset.
+  std::vector<Profile*> profiles_to_reset_;
+
+  // The number of profiles that need to be reset.
+  int num_pending_resets_;
+
+  // The callback to be invoked once settings reset completes.
+  base::OnceClosure done_callback_;
+
+  std::unique_ptr<SettingsResetterDelegate> delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(SettingsResetter);
+};
+
+SettingsResetter::SettingsResetter(
+    std::vector<Profile*> profiles_to_reset,
+    std::unique_ptr<SettingsResetterDelegate> delegate,
+    base::OnceClosure done_callback)
+    : profiles_to_reset_(std::move(profiles_to_reset)),
+      num_pending_resets_(profiles_to_reset_.size()),
+      done_callback_(std::move(done_callback)),
+      delegate_(std::move(delegate)) {
+  DCHECK_LT(0, num_pending_resets_);
+  DCHECK(done_callback_);
+  DCHECK(delegate_);
+}
+
+SettingsResetter::~SettingsResetter() {
+  DCHECK(!done_callback_);
+  DCHECK(!num_pending_resets_);
+}
+
+void SettingsResetter::Run() {
+  for (Profile* profile : profiles_to_reset_) {
+    delegate_->FetchDefaultSettings(
+        base::Bind(&SettingsResetter::OnFetchCompleted, this, profile));
+  }
+}
+
+void SettingsResetter::OnFetchCompleted(
+    Profile* profile,
+    std::unique_ptr<BrandcodedDefaultSettings> master_settings) {
+  static const ProfileResetter::ResettableFlags kSettingsToReset =
+      ProfileResetter::DEFAULT_SEARCH_ENGINE | ProfileResetter::HOMEPAGE |
+      ProfileResetter::EXTENSIONS | ProfileResetter::STARTUP_PAGES |
+      ProfileResetter::SHORTCUTS;
+
+  delegate_->GetProfileResetter(profile)->Reset(
+      kSettingsToReset, std::move(master_settings),
+      base::Bind(&SettingsResetter::OnResetCompleted, base::Unretained(this),

[alito, 2017/06/07 19:54:03]

Should this be without base::Unretained?

[ftirelo, 2017/06/08 21:54:11]

Done.

+                 profile));
+}
+
+void SettingsResetter::OnResetCompleted(Profile* profile) {
+  DCHECK_LT(0, num_pending_resets_);
+
+  RecordPostCleanupSettingsResetPending(false, profile);
+
+  --num_pending_resets_;
+  if (!num_pending_resets_)
+    std::move(done_callback_).Run();
+}
+
+// Returns true if there is information of a completed cleanup in the registry.
+bool CleanupCompletedFromRegistry() {
+  base::string16 cleaner_key_path(
+      chrome_cleaner::kSoftwareRemovalToolRegistryKey);
+  cleaner_key_path.append(L"\\").append(chrome_cleaner::kCleanerSubKey);
+
+  base::win::RegKey srt_cleaner_key(HKEY_CURRENT_USER, cleaner_key_path.c_str(),
+                                    KEY_QUERY_VALUE);
+  int64_t ts_cleanup_started = 0L, ts_cleanup_completed = 0L;
+  return srt_cleaner_key.Valid() &&
+         srt_cleaner_key.ReadInt64(
+             chrome_cleaner::kCleanupStartedTimestampValueName,
+             &ts_cleanup_started) == ERROR_SUCCESS &&
+         srt_cleaner_key.ReadInt64(
+             chrome_cleaner::kCleanupConfirmedTimestampValueName,
+             &ts_cleanup_completed) == ERROR_SUCCESS &&
+         ts_cleanup_started < ts_cleanup_completed;
+}
+
+bool CopyProfilesToReset(const std::vector<Profile*>& profiles,
+                         std::vector<Profile*>* profiles_to_reset) {
+  std::copy_if(profiles.begin(), profiles.end(),
+               std::back_inserter(*profiles_to_reset),
+               [](Profile* profile) -> bool {
+                 return PostCleanupSettingsResetPending(profile);
+               });
+  return profiles_to_reset->size() > 0;
+}
+
+}  // namespace
+
+void RegisterChromeCleanerProfilePrefs(
+    user_prefs::PrefRegistrySyncable* registry) {
+  DCHECK(registry);
+  registry->RegisterBooleanPref(prefs::kChromeCleanerResetPending, false);
+}
+
+bool PostCleanupSettingsResetPending(Profile* profile) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(profile);
+
+  if (!base::FeatureList::IsEnabled(safe_browsing::kInBrowserCleanerUIFeature))

[alito, 2017/06/07 19:54:03]

As discussed, it would make sense to make these checks more explicit in the
public interface, for example by having a class and a factory function that
retuns null if the feature is not enabled.

[ftirelo, 2017/06/08 21:54:11]

Done.

+    return false;
+
+  PrefService* prefs = profile->GetPrefs();
+  DCHECK(prefs);
+  return prefs->GetBoolean(prefs::kChromeCleanerResetPending);
+}
+
+void RecordPostCleanupSettingsResetPending(bool value, Profile* profile) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(profile);
+
+  if (!base::FeatureList::IsEnabled(safe_browsing::kInBrowserCleanerUIFeature))
+    return;
+
+  PrefService* prefs = profile->GetPrefs();
+  DCHECK(prefs);
+  prefs->SetBoolean(prefs::kChromeCleanerResetPending, value);
+}
+
+void TagProfileForResetting(Profile* profile) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(profile);
+
+  if (base::FeatureList::IsEnabled(safe_browsing::kInBrowserCleanerUIFeature))
+    RecordPostCleanupSettingsResetPending(true, profile);
+}
+
+void ResetPostCleanupSettingsIfTagged(

[alito, 2017/06/07 19:54:03]

Please move this function to the end of the file so the order of definitions
matches the declaration order in the header.

[ftirelo, 2017/06/08 21:54:11]

Done.

+    std::vector<Profile*> profiles,
+    base::OnceClosure continuation,
+    std::unique_ptr<SettingsResetterDelegate> delegate) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(continuation);
+
+  std::vector<Profile*> profiles_to_reset;
+  if (!base::FeatureList::IsEnabled(
+          safe_browsing::kInBrowserCleanerUIFeature) ||
+      !CopyProfilesToReset(profiles, &profiles_to_reset) ||
+      !CleanupCompletedFromRegistry()) {
+    std::move(continuation).Run();
+    return;
+  }
+
+  // This object will self-delete once |continuation| is invoked.
+  make_scoped_refptr(
+      new SettingsResetter(std::move(profiles_to_reset),
+                           delegate
+                               ? std::move(delegate)
+                               : base::MakeUnique<SettingsResetterDelegate>(),
+                           std::move(continuation)))
+      ->Run();
+}
+
+SettingsResetterDelegate::SettingsResetterDelegate() {}
+
+SettingsResetterDelegate::~SettingsResetterDelegate() {}
+
+void SettingsResetterDelegate::FetchDefaultSettings(
+    DefaultSettingsFetcher::SettingsCallback callback) {
+  DefaultSettingsFetcher::FetchDefaultSettings(std::move(callback));
+}
+
+std::unique_ptr<ProfileResetter> SettingsResetterDelegate::GetProfileResetter(
+    Profile* profile) {
+  return base::MakeUnique<ProfileResetter>(profile);
+}
+
+}  // namespace safe_browsing