Post-cleanup settings reset.

chrome/browser/safe_browsing/chrome_cleaner/settings_resetter_win.h

+// 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.
+
+#ifndef CHROME_BROWSER_SAFE_BROWSING_CHROME_CLEANER_SETTINGS_RESETTER_WIN_H_
+#define CHROME_BROWSER_SAFE_BROWSING_CHROME_CLEANER_SETTINGS_RESETTER_WIN_H_
+
+#include <memory>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "chrome/browser/safe_browsing/settings_reset_prompt/default_settings_fetcher.h"
+
+class Profile;
+class ProfileResetter;
+
+namespace user_prefs {
+class PrefRegistrySyncable;
+}  // namespace user_prefs
+
+namespace safe_browsing {
+
+// Handles settings reset for user's profile to complete a Chrome Cleaner run.
+// Allows tagging a profile for resetting once a cleanup starts and resetting
+// settings once a cleanup is completed. Completed cleanup is identified by
+// annotations in the registry written by the cleaner.
+class PostCleanupSettingsResetter {
+ public:
+  class Delegate {
+   public:
+    Delegate();
+    virtual ~Delegate();
+
+    virtual void FetchDefaultSettings(
+        DefaultSettingsFetcher::SettingsCallback callback);
+
+    virtual std::unique_ptr<ProfileResetter> GetProfileResetter(
+        Profile* profile);
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Delegate);
+  };
+
+  // Factory method for this class. Returns null if post-cleanup settings reset
+  // is disabled.

[robertshield, 2017/06/12 15:00:04]

Why does this class need a factory method? It looks like Create() checks the
field trial state here, and then returns null or not. 

This still requires callers to check for null. How about making IsEnabled()
static, and making the constructor public?

This will make it clearer at the call sites in ChromeCleanerControllerDelegate
whether something is happening or not and why.

[ftirelo, 2017/06/13 18:52:49]

Done.

+  static std::unique_ptr<PostCleanupSettingsResetter> Create();
+
+  // Tags |profile| to have its settings reset once the current cleanup
+  // finishes. It's safe to delete the current object once this function
+  // returns.

[robertshield, 2017/06/12 15:00:04]

Furthermore, if callers create these things on the stack, you won't need
comments like "it's safe to delete the current object once this function
returns" since that will be implied.

[ftirelo, 2017/06/13 18:52:49]

Done.

+  void TagForResetting(Profile* profile);
+
+  // Resets settings for the profiles in |profiles| there are tagged for
+  // resetting if cleanup has completed. Invokes |continuation| once all
+  // profiles is |profiles| have been reset. It's safe to delete this

[robertshield, 2017/06/12 15:00:04]

s/is/in/

[ftirelo, 2017/06/13 18:52:49]

Done.

+  // object once this function returns.
+  void ResetTaggedProfiles(
+      std::vector<Profile*> profiles,
+      base::OnceClosure continuation,
+      std::unique_ptr<PostCleanupSettingsResetter::Delegate> delegate =
+          nullptr);
+
+  // Registers the settings reset pending tracked preference.
+  static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
+
+ protected:
+  PostCleanupSettingsResetter();
+
+ private:
+  // This object doesn't hold any state, so it's safe to delete it even after
+  // an async function is called. For example, it's fine to let the object get
+  // out of scope after invoking ResetTaggedProfiles() and there is not need
+  // to wait for the callback to be run to release it. If you are intending to
+  // change that assumption, please make sure you don't break the contract
+  // where this class is used.
+
+  DISALLOW_COPY_AND_ASSIGN(PostCleanupSettingsResetter);
+};
+
+}  // namespace safe_browsing
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_CHROME_CLEANER_SETTINGS_RESETTER_WIN_H_