Revert "Use policy to control Chrome Cleanup reporting."

This reverts commit 8e57e010.

Reason for revert: It breaks official builds:
https://uberchromegw.corp.google.com/i/official.desktop.continuous/builders/win%20trunk/builds/84546

Original change's description:
> Use policy to control Chrome Cleanup reporting.
> 
> Bug: 834939
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: I603d1d8d43d5317f8a8ef24f4fdbb537e0a21541
> Reviewed-on: https://chromium-review.googlesource.com/1053763
> Commit-Queue: Roger Tawa <rogerta@chromium.org>
> Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
> Reviewed-by: Joe Mason <joenotcharles@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#559685}

TBR=rogerta@chromium.org,dpapad@chromium.org,joenotcharles@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 834939
Change-Id: I1b6e1a4007935abf0f6e8f9b1adf05804c431a1f
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/1069387Reviewed-by: Reid Kleckner <rnk@chromium.org>
Commit-Queue: Reid Kleckner <rnk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560748}

chrome/browser/resources/settings/chrome_cleanup_page/chrome_cleanup_page.js

@@ -138,12 +138,30 @@ Polymer({
       value: false,
     },
 
+    /** @private */
+    uploadAllowed_: {
+      type: Boolean,
+      value: true,
+    },
+
+    /** @private */
+    uploadManaged_: {
+      type: Boolean,
+      computed: 'computeUploadManaged_(cleanupManaged_, cleanupEnabled_)',
+    },
+
     /** @private */
     cleanupEnabled_: {
       type: Boolean,
       value: true,
     },
 
+    /** @private */
+    cleanupManaged_: {
+      type: Boolean,
+      value: false,
+    },
+
     /** @private */
     actionButtonLabel_: {
       type: String,
@@ -326,6 +344,16 @@ Polymer({
       this.browserProxy_.notifyShowDetails(this.itemsToRemoveSectionExpanded_);
   },
 
+  /**
+   * Uploads are managed if cleanup is controlled by policy and the policy
+   * disables the feature.  If the option is controlled by policy but enables
+   * the feature, there is no difference at all from not being managed.
+   * @return {boolean}
+   */
+  computeUploadManaged_: function() {
+    return this.cleanupManaged_ && !this.cleanupEnabled_;
+  },
+
   /**
    * @param {string} explanation
    * @return {boolean}
@@ -562,17 +590,17 @@ Polymer({
   },
 
   /**
-   * @param {boolean} managed Whether uploads are controlled by policy or not.
    * @param {boolean} enabled Whether logs upload is enabled.
    * @private
    */
-  onUploadPermissionChange_: function(managed, enabled) {
+  onUploadPermissionChange_: function(enabled) {
+    this.uploadAllowed_ = enabled;
     const pref = {
       key: '',
       type: chrome.settingsPrivate.PrefType.BOOLEAN,
-      value: enabled,
+      value: this.uploadAllowed_,
     };
-    if (managed) {
+    if (this.uploadManaged_) {
       pref.enforcement = chrome.settingsPrivate.Enforcement.ENFORCED;
       pref.controlledBy = chrome.settingsPrivate.ControlledBy.USER_POLICY;
     }
@@ -580,17 +608,20 @@ Polymer({
   },
 
   /**
+   * @param {boolean} managed Whether this is controlled by policy or not.
    * @param {boolean} enabled Whether cleanup is enabled.
    * @private
    */
-  onCleanupEnabledChange_: function(enabled) {
+  onCleanupEnabledChange_: function(managed, enabled) {
+    this.cleanupManaged_ = managed;
     this.cleanupEnabled_ = enabled;
+    this.onUploadPermissionChange_(this.uploadAllowed_);
   },
 
   /** @private */
   changeLogsPermission_: function() {
-    const enabled = this.$.chromeCleanupLogsUploadControl.checked;
-    this.browserProxy_.setLogsUploadPermission(enabled);
+    this.uploadAllowed_ = this.$.chromeCleanupLogsUploadControl.checked;
+    this.browserProxy_.setLogsUploadPermission(this.uploadAllowed_);
   },
 
   /**

chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_impl_win.cc

@@ -529,10 +529,6 @@ bool ChromeCleanerControllerImpl::IsAllowedByPolicy() {
   return safe_browsing::SwReporterIsAllowedByPolicy();
 }
 
-bool ChromeCleanerControllerImpl::IsReportingAllowedByPolicy() {
-  return safe_browsing::SwReporterReportingIsAllowedByPolicy();
-}
-
 ChromeCleanerControllerImpl::ChromeCleanerControllerImpl()
     : real_delegate_(std::make_unique<ChromeCleanerControllerDelegate>()),
       delegate_(real_delegate_.get()),
@@ -543,7 +539,9 @@ ChromeCleanerControllerImpl::ChromeCleanerControllerImpl()
 ChromeCleanerControllerImpl::~ChromeCleanerControllerImpl() = default;
 
 void ChromeCleanerControllerImpl::Init() {
-  logs_enabled_ = IsReportingAllowedByPolicy();
+  // The default value for logs is determined by whether the cleanup feature is
+  // enabled by policy.
+  logs_enabled_ = IsAllowedByPolicy();
 }
 
 void ChromeCleanerControllerImpl::NotifyObserver(Observer* observer) const {

chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_impl_win.h

@@ -68,7 +68,6 @@ class ChromeCleanerControllerImpl : public ChromeCleanerController {
                              UserResponse user_response) override;
   void Reboot() override;
   bool IsAllowedByPolicy() override;
-  bool IsReportingAllowedByPolicy() override;
 
   static void ResetInstanceForTesting();
   // Passing in a nullptr as |delegate| resets the delegate to a default

chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_win.h

@@ -207,9 +207,6 @@ class ChromeCleanerController {
   // Returns true if the cleaner is allowed to run by enterprise policy.
   virtual bool IsAllowedByPolicy() = 0;
 
-  // Returns true if cleaner reporting is allowed to run by enterprise policy.
-  virtual bool IsReportingAllowedByPolicy() = 0;
-
  protected:
   ChromeCleanerController();
   virtual ~ChromeCleanerController();

chrome/browser/safe_browsing/chrome_cleaner/mock_chrome_cleaner_controller_win.h

@@ -35,7 +35,6 @@ class MockChromeCleanerController
   MOCK_METHOD2(ReplyWithUserResponse, void(Profile*, UserResponse));
   MOCK_METHOD0(Reboot, void());
   MOCK_METHOD0(IsAllowedByPolicy, bool());
-  MOCK_METHOD0(IsReportingAllowedByPolicy, bool());
 };
 
 }  // namespace safe_browsing

chrome/browser/safe_browsing/chrome_cleaner/reporter_runner_win.cc

@@ -96,7 +96,6 @@ enum SwReporterLogsUploadsEnabled {
   REPORTER_LOGS_UPLOADS_RECENTLY_SENT_LOGS = 2,
   REPORTER_LOGS_UPLOADS_DISABLED_BY_USER = 3,
   REPORTER_LOGS_UPLOADS_ENABLED_BY_USER = 4,
-  REPORTER_LOGS_UPLOADS_DISABLED_BY_POLICY = 5,
   REPORTER_LOGS_UPLOADS_MAX,
 };
 
@@ -861,13 +860,6 @@ class ReporterRunner {
   // started during the logs upload interval.
   bool ShouldSendReporterLogs(const std::string& suffix) {
     UMAHistogramReporter uma(suffix);
-
-    // The enterprise policy overrides all other choices.
-    if (!SwReporterReportingIsAllowedByPolicy()) {
-      uma.RecordLogsUploadEnabled(REPORTER_LOGS_UPLOADS_DISABLED_BY_POLICY);
-      return false;
-    }
-
     switch (invocation_type_) {
       case SwReporterInvocationType::kUnspecified:
       case SwReporterInvocationType::kMax:
@@ -1166,21 +1158,6 @@ bool SwReporterIsAllowedByPolicy() {
   return is_allowed;
 }
 
-bool SwReporterReportingIsAllowedByPolicy() {
-  // Reporting is allowed when cleanup is enabled by policy and when the
-  // specific reporting policy is allowed.  While the former policy is not
-  // dynamic, the latter one is.
-  bool is_allowed = SwReporterIsAllowedByPolicy();
-  if (is_allowed) {
-    PrefService* local_state = g_browser_process->local_state();
-    is_allowed =
-        !local_state ||
-        !local_state->IsManagedPreference(prefs::kSwReporterReportingEnabled) ||
-        local_state->GetBoolean(prefs::kSwReporterReportingEnabled);
-  }
-  return is_allowed;
-}
-
 void SetSwReporterTestingDelegate(SwReporterTestingDelegate* delegate) {
   g_testing_delegate_ = delegate;
 }

chrome/browser/safe_browsing/chrome_cleaner/reporter_runner_win.h

@@ -219,10 +219,6 @@ void MaybeStartSwReporter(SwReporterInvocationType invocation_type,
 // Returns true if the sw_reporter is allowed to run due to enterprise policies.
 bool SwReporterIsAllowedByPolicy();
 
-// Returns true if the sw_reported is allowed to report back results due to
-// enterprise policies.
-bool SwReporterReportingIsAllowedByPolicy();
-
 // A delegate used by tests to implement test doubles (e.g., stubs, fakes, or
 // mocks).
 //

chrome/browser/ui/webui/settings/chrome_cleanup_handler.cc

@@ -96,14 +96,7 @@ std::string IdleReasonToString(
 }  // namespace
 
 ChromeCleanupHandler::ChromeCleanupHandler(Profile* profile)
-    : controller_(ChromeCleanerController::GetInstance()), profile_(profile) {
-  DCHECK(g_browser_process->local_state());
-  logs_enabled_pref_.Init(g_browser_process->local_state());
-  logs_enabled_pref_.Add(
-      prefs::kSwReporterReportingEnabled,
-      base::BindRepeating(&ChromeCleanupHandler::OnLogsEnabledPrefChanged,
-                          base::Unretained(this)));
-}
+    : controller_(ChromeCleanerController::GetInstance()), profile_(profile) {}
 
 ChromeCleanupHandler::~ChromeCleanupHandler() {
   controller_->RemoveObserver(this);
@@ -194,20 +187,8 @@ void ChromeCleanupHandler::OnRebootRequired() {
 }
 
 void ChromeCleanupHandler::OnLogsEnabledChanged(bool logs_enabled) {
-  // Logs are considered managed if the logs themselves are managed or if the
-  // entire cleanup feature is disabled by policy.
-  PrefService* local_state = g_browser_process->local_state();
-  bool is_managed = !controller_->IsAllowedByPolicy() ||
-                    (local_state && local_state->IsManagedPreference(
-                                        prefs::kSwReporterReportingEnabled));
   FireWebUIListener("chrome-cleanup-upload-permission-change",
-                    base::Value(is_managed), base::Value(logs_enabled));
-}
-
-void ChromeCleanupHandler::OnLogsEnabledPrefChanged() {
-  bool is_enabled = controller_->IsReportingAllowedByPolicy();
-  controller_->SetLogsEnabled(is_enabled);
-  OnLogsEnabledChanged(is_enabled);
+                    base::Value(logs_enabled));
 }
 
 void ChromeCleanupHandler::HandleRegisterChromeCleanerObserver(
@@ -222,7 +203,11 @@ void ChromeCleanupHandler::HandleRegisterChromeCleanerObserver(
   // Send the current logs upload state.
   OnLogsEnabledChanged(controller_->logs_enabled());
 
-  FireWebUIListener("chrome-cleanup-enabled-change",
+  // Inform the UI of the current chrome cleanup state.
+  bool is_managed = g_browser_process->local_state() &&
+                    g_browser_process->local_state()->IsManagedPreference(
+                        prefs::kSwReporterEnabled);
+  FireWebUIListener("chrome-cleanup-enabled-change", base::Value(is_managed),
                     base::Value(controller_->IsAllowedByPolicy()));
 }
 

chrome/browser/ui/webui/settings/chrome_cleanup_handler.h

@@ -12,7 +12,6 @@
 #include "chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_controller_win.h"
 #include "chrome/browser/safe_browsing/chrome_cleaner/chrome_cleaner_scanner_results.h"
 #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
-#include "components/prefs/pref_change_registrar.h"
 
 class Profile;
 
@@ -46,9 +45,6 @@ class ChromeCleanupHandler
   void OnLogsEnabledChanged(bool logs_enabled) override;
 
  private:
-  // Called when prefs::kSwReporterReportingEnabled changes.
-  void OnLogsEnabledPrefChanged();
-
   // Callback for the "registerChromeCleanerObserver" message. This registers
   // this object as an observer of the Chrome Cleanup global state and
   // and retrieves the current cleanup state.
@@ -92,7 +88,6 @@ class ChromeCleanupHandler
   safe_browsing::ChromeCleanerController* controller_;
 
   Profile* profile_;
-  PrefChangeRegistrar logs_enabled_pref_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeCleanupHandler);
 };

tools/metrics/histograms/enums.xml

@@ -42243,7 +42243,6 @@ Called by update_net_trust_anchors.py.-->
   <int value="2" label="Recently sent logs"/>
   <int value="3" label="Disabled by user"/>
   <int value="4" label="Enabled by user"/>
-  <int value="5" label="Disabled by policy"/>
 </enum>
 
 <enum name="SoftwareReporterLogsUploadResult">