Added Arc.Session.Finished as metric to ARC

BUG=b:139752841
TEST=Ran on betty VM and eve-arcvm device, stopped ARC by opting out of Play store, and checked histograms on chrome.

Change-Id: Ia400c6c38614e3b9bd19b1382ed26ba2bd76030e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1792788
Commit-Queue: Shao-Chuan Lee <shaochuan@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Shao-Chuan Lee <shaochuan@chromium.org>
Reviewed-by: Hidehiko Abe <hidehiko@chromium.org>
Reviewed-by: Yury Khmel <khmel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#715572}

chrome/browser/chromeos/arc/metrics/arc_metrics_service_proxy.cc

@@ -5,6 +5,9 @@
 #include "chrome/browser/chromeos/arc/metrics/arc_metrics_service_proxy.h"
 
 #include "base/memory/singleton.h"
+#include "base/metrics/histogram_functions.h"
+#include "chrome/browser/chromeos/arc/arc_util.h"
+#include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs_factory.h"
 #include "components/arc/arc_browser_context_keyed_service_factory_base.h"
 #include "components/arc/metrics/arc_metrics_service.h"
@@ -49,9 +52,11 @@ ArcMetricsServiceProxy::ArcMetricsServiceProxy(
     : arc_app_list_prefs_(ArcAppListPrefs::Get(context)),
       arc_metrics_service_(ArcMetricsService::GetForBrowserContext(context)) {
   arc_app_list_prefs_->AddObserver(this);
+  arc::ArcSessionManager::Get()->AddObserver(this);
 }
 
 void ArcMetricsServiceProxy::Shutdown() {
+  arc::ArcSessionManager::Get()->RemoveObserver(this);
   arc_app_list_prefs_->RemoveObserver(this);
 }
 
@@ -66,4 +71,13 @@ void ArcMetricsServiceProxy::OnTaskDestroyed(int32_t task_id) {
   arc_metrics_service_->OnTaskDestroyed(task_id);
 }
 
+void ArcMetricsServiceProxy::OnArcSessionStopped(ArcStopReason stop_reason) {
+  const auto* profile = ProfileManager::GetPrimaryUserProfile();
+  if (arc::IsArcAllowedForProfile(profile)) {
+    base::UmaHistogramEnumeration(
+        GetHistogramNameByUserType("Arc.Session.StopReason", profile),
+        stop_reason);
+  }
+}
+
 }  // namespace arc

chrome/browser/chromeos/arc/metrics/arc_metrics_service_proxy.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_CHROMEOS_ARC_METRICS_ARC_METRICS_SERVICE_PROXY_H_
 
 #include "base/macros.h"
+#include "chrome/browser/chromeos/arc/session/arc_session_manager.h"
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
 #include "components/keyed_service/core/keyed_service.h"
 
@@ -22,7 +23,8 @@ class ArcMetricsService;
 // chrome/browser. Should be merged into ArcMetricsService once dependency
 // issues are cleared. TODO(crbug.com/903048): Remove the proxy.
 class ArcMetricsServiceProxy : public KeyedService,
-                               public ArcAppListPrefs::Observer {
+                               public ArcAppListPrefs::Observer,
+                               public ArcSessionManager::Observer {
  public:
   static ArcMetricsServiceProxy* GetForBrowserContext(
       content::BrowserContext* context);
@@ -41,6 +43,9 @@ class ArcMetricsServiceProxy : public KeyedService,
                      const std::string& intent) override;
   void OnTaskDestroyed(int32_t task_id) override;
 
+  // ArcSessionManager::Observer overrides.
+  void OnArcSessionStopped(ArcStopReason stop_reason) override;
+
  private:
   ArcAppListPrefs* const arc_app_list_prefs_;
   ArcMetricsService* const arc_metrics_service_;

components/arc/session/arc_stop_reason.h

@@ -10,21 +10,26 @@
 namespace arc {
 
 // Describes the reason the ARC instance is stopped.
+// These values are persisted to logs. Entries should not be renumbered and
+// numeric values should never be reused.
+// Should be synced with ArcStopReason in tools/metrics/histograms/enums.xml.
 enum class ArcStopReason {
   // ARC instance has been gracefully shut down.
-  SHUTDOWN,
+  SHUTDOWN = 0,
 
   // Errors occurred during the ARC instance boot. This includes any failures
   // before the instance is actually attempted to be started (e.g.
   // session_manager failed to fork/exec the instance), and also a "clean"
   // (non crashy) but unexpected container shutdown.
-  GENERIC_BOOT_FAILURE,
+  GENERIC_BOOT_FAILURE = 1,
 
   // The device is critically low on disk space.
-  LOW_DISK_SPACE,
+  LOW_DISK_SPACE = 2,
 
   // ARC instance has crashed.
-  CRASH,
+  CRASH = 3,
+
+  kMaxValue = CRASH,
 };
 
 // Defines "<<" operator for LOGging purpose.

tools/metrics/histograms/enums.xml

@@ -2860,6 +2860,14 @@ Unknown properties are collapsed to zero. -->
   <int value="6" label="BAD_RESPONSE_FROM_DOCUMENTS_UI"/>
 </enum>
 
+<enum name="ArcStopReason">
+  <summary>Describes the reason the ARC instance is stopped</summary>
+  <int value="0" label="SHUTDOWN"/>
+  <int value="1" label="GENERIC_BOOT_FAILURE"/>
+  <int value="2" label="LOW_DISK_SPACE"/>
+  <int value="3" label="CRASH"/>
+</enum>
+
 <enum name="ArcSupervisionTransitionResult">
   <summary>
     Defines Arc supervision transition success and failure reasons

tools/metrics/histograms/histograms.xml

@@ -7239,6 +7239,20 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="Arc.Session.StopReason" enum="ArcStopReason"
+    expires_after="2020-12-05">
+<!-- Name completed by histogram_suffixes name="ArcUserTypes" -->
+
+  <owner>shaochuan@google.com</owner>
+  <owner>khmel@google.com</owner>
+  <owner>mmatena@google.com</owner>
+  <summary>
+    This is updated in case ARC++ stops for the reasons detailed in
+    ArcStopReason. For example, this is recorded in case of a graceful ARC++
+    shutdown or if ARC++ crashes.
+  </summary>
+</histogram>
+
 <histogram name="Arc.ShareFilesOnExit" enum="ArcShareFilesOnExit"
     expires_after="M75">
   <owner>weifangsun@google.com</owner>
@@ -169703,6 +169717,7 @@ regressions. -->
   <affected-histogram name="Arc.Provisioning.TimeDelta.Failure"/>
   <affected-histogram name="Arc.Provisioning.TimeDelta.Success"/>
   <affected-histogram name="Arc.Reauthorization.Result"/>
+  <affected-histogram name="Arc.Session.StopReason"/>
   <affected-histogram name="Arc.StateByUserType"/>
   <affected-histogram name="Arc.UiAvailable.AlreadyProvisioned.TimeDelta"/>
   <affected-histogram name="Arc.UiAvailable.InSessionProvisioning.TimeDelta"/>