crostini: Bucket RestarterResult metrics after an unclean start separately

After e.g. a Chrome crash we're in a known unsupported state, we tell
the user this but allow them to continue. Restarter errors in this case
should go in their own bucket, since we're in a different starting state
with its own set of failure modes.

Bug: 1142321
Test: Launch Crostini before/after a crash, check chrome://histograms
Change-Id: I7927b18f6d98572b6a083426eb3d8a7a05f13f52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532135
Commit-Queue: David Munro <davidmunro@google.com>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#830067}

chrome/browser/chromeos/crostini/crostini_manager.cc

@@ -277,6 +277,13 @@ class CrostiniManager::CrostiniRestarter
     // separate.
     if (!is_initial_install_) {
       base::UmaHistogramEnumeration("Crostini.RestarterResult", result_);
+      if (crostini_manager_->IsUncleanStartup()) {
+        base::UmaHistogramEnumeration("Crostini.UncleanSession.RestarterResult",
+                                      result_);
+      } else {
+        base::UmaHistogramEnumeration("Crostini.CleanSession.RestarterResult",
+                                      result_);
+      }
     }
     crostini_manager_->RemoveVmShutdownObserver(this);
     if (completed_callback_) {

chrome/browser/chromeos/crostini/crostini_manager_unittest.cc

@@ -739,6 +739,10 @@ class CrostiniManagerRestartTest : public CrostiniManagerTest,
   void ExpectRestarterUmaCount(int count) {
     histogram_tester_.ExpectTotalCount("Crostini.Restarter.Started", count);
     histogram_tester_.ExpectTotalCount("Crostini.RestarterResult", count);
+    histogram_tester_.ExpectTotalCount("Crostini.CleanSession.RestarterResult",
+                                       count);
+    histogram_tester_.ExpectTotalCount(
+        "Crostini.UncleanSession.RestarterResult", 0);
     histogram_tester_.ExpectTotalCount("Crostini.Installer.Started", 0);
   }
 
@@ -790,6 +794,33 @@ TEST_F(CrostiniManagerRestartTest, RestartSuccess) {
   ExpectRestarterUmaCount(1);
 }
 
+TEST_F(CrostiniManagerRestartTest, UncleanRestartReportsMetricToUncleanBucket) {
+  crostini_manager()->SetUncleanStartupForTesting(true);
+  restart_id_ = crostini_manager()->RestartCrostini(
+      container_id(),
+      base::BindOnce(&CrostiniManagerRestartTest::RestartCrostiniCallback,
+                     base::Unretained(this), run_loop()->QuitClosure()),
+      this);
+  run_loop()->Run();
+  EXPECT_TRUE(fake_concierge_client_->create_disk_image_called());
+  EXPECT_TRUE(fake_concierge_client_->start_termina_vm_called());
+  // Mount only performed for termina/penguin.
+  EXPECT_FALSE(fake_concierge_client_->get_container_ssh_keys_called());
+  EXPECT_EQ(1, restart_crostini_callback_count_);
+
+  base::Optional<ContainerInfo> container_info =
+      crostini_manager()->GetContainerInfo(container_id());
+  EXPECT_EQ(container_info.value().username,
+            DefaultContainerUserNameForProfile(profile()));
+  histogram_tester_.ExpectTotalCount("Crostini.Restarter.Started", 1);
+  histogram_tester_.ExpectTotalCount("Crostini.RestarterResult", 1);
+  histogram_tester_.ExpectTotalCount("Crostini.CleanSession.RestarterResult",
+                                     0);
+  histogram_tester_.ExpectTotalCount("Crostini.UncleanSession.RestarterResult",
+                                     1);
+  histogram_tester_.ExpectTotalCount("Crostini.Installer.Started", 0);
+}
+
 TEST_F(CrostiniManagerRestartTest, RestartDelayAndSuccessWhenVmStopping) {
   crostini_manager()->AddStoppingVmForTesting(kVmName);
   on_stage_started_ =

tools/metrics/histograms/histograms_xml/crostini/histograms.xml

@@ -140,6 +140,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   <summary>Time taken for successful backup.</summary>
 </histogram>
 
+<histogram name="Crostini.CleanSession.RestarterResult" enum="CrostiniResult"
+    expires_after="2021-04-04">
+  <owner>clumptini@google.com</owner>
+  <owner>tbuckley@chromium.org</owner>
+  <summary>
+    The result of a single run of CrostiniRestarter. This is the same as
+    Crostini.RestarterResult except only emitted in sessions that happen after a
+    clean start (e.g. after fresh login).
+  </summary>
+</histogram>
+
 <histogram name="Crostini.ContainerOsVersion" enum="CrostiniContainerOsVersion"
     expires_after="2021-04-04">
   <owner>clumptini@google.com</owner>
@@ -253,7 +264,7 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   <owner>tbuckley@chromium.org</owner>
   <summary>
     The result of a single run of CrostiniRestarter. This is recorded any time
-    the crostini restart flow is triggered, except during the initial install.
+    the crostini restart flow is triggered except during the initial install.
   </summary>
 </histogram>
 
@@ -382,6 +393,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="Crostini.UncleanSession.RestarterResult" enum="CrostiniResult"
+    expires_after="2021-04-04">
+  <owner>clumptini@google.com</owner>
+  <owner>tbuckley@chromium.org</owner>
+  <summary>
+    The result of a single run of CrostiniRestarter. This is the same as
+    Crostini.RestarterResult except only emitted in sessions that happen after
+    an unclean restart (e.g. after a Chrome crash).
+  </summary>
+</histogram>
+
 <histogram name="Crostini.UninstallResult" enum="CrostiniUninstallResult"
     expires_after="2021-04-04">
   <owner>clumptini@google.com</owner>