Only report Demo Mode resources removal if resources exist

When removing preinstalled Demo Mode Resources, only track UMA stats for
resource removal reasons and results for devices that actually have the
offline resources.

Bug: 905794
Change-Id: I76aefd5ea71d6d2bc2be8eeb92cf154e05eecac3
Reviewed-on: https://chromium-review.googlesource.com/c/1430539Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625761}

chrome/browser/chromeos/login/demo_mode/demo_mode_resources_remover.cc

@@ -228,16 +228,12 @@ void DemoModeResourcesRemover::AttemptRemoval(RemovalReason reason,
     return;
   removal_in_progress_ = true;
 
-  // Report this metric only once per resources directory removal task.
-  // Concurrent removal requests should not be reported multiple times.
-  UMA_HISTOGRAM_ENUMERATION("DemoMode.ResourcesRemoval.Reason", reason);
-
   base::PostTaskWithTraitsAndReplyWithResult(
       FROM_HERE,
       {base::MayBlock(), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
       base::BindOnce(&RemoveDirectory, DemoResources::GetPreInstalledPath()),
       base::BindOnce(&DemoModeResourcesRemover::OnRemovalDone,
-                     weak_ptr_factory_.GetWeakPtr()));
+                     weak_ptr_factory_.GetWeakPtr(), reason));
 }
 
 void DemoModeResourcesRemover::OverrideTimeForTesting(
@@ -292,7 +288,8 @@ bool DemoModeResourcesRemover::AttemptRemovalIfUsageOverThreshold() {
   return true;
 }
 
-void DemoModeResourcesRemover::OnRemovalDone(RemovalResult result) {
+void DemoModeResourcesRemover::OnRemovalDone(RemovalReason reason,
+                                             RemovalResult result) {
   DCHECK(removal_in_progress_);
   removal_in_progress_ = false;
 
@@ -308,7 +305,14 @@ void DemoModeResourcesRemover::OnRemovalDone(RemovalResult result) {
     usage_end_ = base::nullopt;
   }
 
-  UMA_HISTOGRAM_ENUMERATION("DemoMode.ResourcesRemoval.Result", result);
+  // Only report metrics when the resources were found; otherwise this is
+  // reported on almost every sign-in.
+  // Only report metrics once per resources directory removal task.
+  // Concurrent removal requests should not be reported multiple times.
+  if (result == RemovalResult::kSuccess || result == RemovalResult::kFailed) {
+    UMA_HISTOGRAM_ENUMERATION("DemoMode.ResourcesRemoval.Reason", reason);
+    UMA_HISTOGRAM_ENUMERATION("DemoMode.ResourcesRemoval.Result", result);
+  }
 
   std::vector<RemovalCallback> callbacks;
   callbacks.swap(removal_callbacks_);

chrome/browser/chromeos/login/demo_mode/demo_mode_resources_remover.h

@@ -173,7 +173,7 @@ class DemoModeResourcesRemover
   bool AttemptRemovalIfUsageOverThreshold();
 
   // Passes as the callback to directory removal file operations.
-  void OnRemovalDone(RemovalResult result);
+  void OnRemovalDone(RemovalReason reason, RemovalResult result);
 
   PrefService* const local_state_;
 

tools/metrics/histograms/enums.xml

@@ -10967,7 +10967,7 @@ Called by update_net_error_codes.py.-->
 
 <enum name="DemoModeResourcesRemovalResult">
   <int value="0" label="Successfully removed"/>
-  <int value="1" label="Not found on disk"/>
+  <int value="1" label="Not found on disk (deprecated)"/>
   <int value="2" label="Removal not allowed (should not be reported)"/>
   <int value="3" label="Resources already removed (should not be reported)"/>
   <int value="4" label="Removal failed"/>