arc: Fix ARC enabled state recording on browser shutdown

ArcSessionManager state is reset before final UMA recording happens on
browser shutdown, which result in missing Arc.State and incorrectly
recorded Arc.StateByUserType.

This CL adds fields to ArcSessionManager to persist state across
Shutdown() for UMA recording purposes.

Bug: 929583
Change-Id: I5fdf58ec60f9720599fa15c997cd2a23ed524fd9
Reviewed-on: https://chromium-review.googlesource.com/c/1481180
Commit-Queue: Shao-Chuan Lee <shaochuan@chromium.org>
Commit-Queue: Hidehiko Abe <hidehiko@chromium.org>
Auto-Submit: 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@{#634549}

chrome/browser/chromeos/arc/arc_session_manager.cc

@@ -438,6 +438,7 @@ void ArcSessionManager::SetProfile(Profile* profile) {
   DCHECK(!profile || !profile_);
   DCHECK(!profile || IsArcAllowedForProfile(profile));
   profile_ = profile;
+  UpdatePersistentUMAState();
 }
 
 void ArcSessionManager::Initialize() {
@@ -609,8 +610,10 @@ void ArcSessionManager::CancelAuthCode() {
 void ArcSessionManager::RecordArcState() {
   // Only record legacy enabled state if ARC is allowed in the first place, so
   // we do not split the ARC population by devices that cannot run ARC.
+  if (should_record_legacy_enabled_state_)
+    UpdateEnabledStateUMA(enabled_state_uma_);
+
   if (IsAllowed()) {
-    UpdateEnabledStateUMA(enable_requested_);
     UpdateEnabledStateByUserTypeUMA(enable_requested_, profile_);
     ArcMetricsService* service =
         ArcMetricsService::GetForBrowserContext(profile_);
@@ -631,7 +634,7 @@ void ArcSessionManager::RecordArcState() {
     return;
   }
 
-  UpdateEnabledStateByUserTypeUMA(enable_requested_, profile);
+  UpdateEnabledStateByUserTypeUMA(enabled_state_uma_, profile);
 }
 
 void ArcSessionManager::RequestEnable() {
@@ -643,6 +646,7 @@ void ArcSessionManager::RequestEnable() {
     return;
   }
   enable_requested_ = true;
+  UpdatePersistentUMAState();
 
   VLOG(1) << "ARC opt-in. Starting ARC session.";
 
@@ -751,6 +755,7 @@ void ArcSessionManager::RequestDisable() {
 
   directly_started_ = false;
   enable_requested_ = false;
+  UpdatePersistentUMAState();
   scoped_opt_in_tracker_.reset();
   pai_starter_.reset();
   fast_app_reinstall_starter_.reset();
@@ -1178,6 +1183,11 @@ void ArcSessionManager::EmitLoginPromptVisibleCalled() {
   arc_session_runner_->RequestStartMiniInstance();
 }
 
+void ArcSessionManager::UpdatePersistentUMAState() {
+  should_record_legacy_enabled_state_ = IsAllowed();
+  enabled_state_uma_ = enable_requested_;
+}
+
 std::ostream& operator<<(std::ostream& os,
                          const ArcSessionManager::State& state) {
 #define MAP_STATE(name)                \

chrome/browser/chromeos/arc/arc_session_manager.h

@@ -349,6 +349,11 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
   // chromeos::SessionManagerClient::Observer:
   void EmitLoginPromptVisibleCalled() override;
 
+  // Updates |should_record_legacy_enabled_state_| and |enabled_state_uma_|
+  // whenever |profile_| or |enable_requested_| is changed (except Shutdown()).
+  // TODO(crbug.com/929583): Remove this temporary fix.
+  void UpdatePersistentUMAState();
+
   std::unique_ptr<ArcSessionRunner> arc_session_runner_;
 
   // Unowned pointer. Keeps current profile.
@@ -385,6 +390,13 @@ class ArcSessionManager : public ArcSessionRunner::Observer,
   base::Time arc_start_time_;
   base::Closure attempt_user_exit_callback_;
 
+  // ARC state depends on |profile_| and |enable_requested_| which is reset in
+  // Shutdown(). Since UMA recording happens after Shutdown() on browser
+  // shutdown, here we persist relevant state for UMA recording purposes.
+  // TODO(crbug.com/929583): Remove this temporary fix.
+  bool should_record_legacy_enabled_state_ = false;
+  bool enabled_state_uma_ = false;
+
   // Must be the last member.
   base::WeakPtrFactory<ArcSessionManager> weak_ptr_factory_;