Split IsArcPlayStoreEnabled() and ArcSessionManager's enabled concepts.

chrome/browser/chromeos/arc/arc_session_manager.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/arc/arc_session_manager.h"
 
 #include <utility>
 
 #include "ash/common/shelf/shelf_delegate.h"
 #include "ash/common/wm_shell.h"
@@ skipping 213 matching lines @@
 
   MaybeReenableArc();
 }
 
 void ArcSessionManager::MaybeReenableArc() {
   // Here check if |reenable_arc_| is marked or not.
   // The only case this happens should be in the special case for enterprise
   // "on managed lost" case. In that case, OnSessionStopped() should trigger
   // the RemoveArcData(), then this.
   // TODO(hidehiko): It looks necessary to reset |reenable_arc_| regardless of
-  // IsArcPlayStoreEnabled(). Fix it.
-  if (!reenable_arc_ || !IsArcPlayStoreEnabled())
+  // |enabled_|. Fix it.
+  if (!reenable_arc_ || !enabled_)
     return;
 
   // Restart ARC anyway. Let the enterprise reporting instance decide whether
   // the ARC user data wipe is still required or not.
   reenable_arc_ = false;
   VLOG(1) << "Reenable ARC";
-  OnOptInPreferenceChanged();
+  EnableImpl();
 }
 
 void ArcSessionManager::OnProvisioningFinished(ProvisioningResult result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // If the Mojo message to notify finishing the provisioning is already sent
   // from the container, it will be processed even after requesting to stop the
   // container. Ignore all |result|s arriving while ARC is disabled, in order to
   // avoid popping up an error message triggered below. This code intentionally
   // does not support the case of reenabling.
-  if (!IsArcPlayStoreEnabled()) {
+  if (!enabled_) {
     LOG(WARNING) << "Provisioning result received after ARC was disabled. "
                  << "Ignoring result " << static_cast<int>(result) << ".";
     return;
   }
 
   // Due asynchronous nature of stopping the ARC instance,
   // OnProvisioningFinished may arrive after setting the |State::STOPPED| state
   // and |State::Active| is not guaranteed to be set here.
   // prefs::kArcDataRemoveRequested also can be active for now.
 
@@ skipping 147 matching lines @@
   // TODO(hidehiko): Revisit to think about lazy initialization.
   //
   // Don't show UI for ARC Kiosk because the only one UI in kiosk mode must
   // be the kiosk app. In case of error the UI will be useless as well, because
   // in typical use case there will be no one nearby the kiosk device, who can
   // do some action to solve the problem be means of UI.
   if (!g_disable_ui_for_testing && !IsArcOptInVerificationDisabled() &&
       !IsArcKioskMode()) {
     DCHECK(!support_host_);
     support_host_ = base::MakeUnique<ArcSupportHost>(profile_);
+    support_host_->SetArcManaged(IsArcManaged());
     support_host_->AddObserver(this);
   }
 
   DCHECK_EQ(State::NOT_INITIALIZED, state_);
   SetState(State::STOPPED);
 
   context_.reset(new ArcAuthContext(profile_));
 
   if (!g_disable_ui_for_testing ||
       g_enable_check_android_management_for_testing) {
     ArcAndroidManagementChecker::StartClient();
   }
+
   pref_change_registrar_.Init(profile_->GetPrefs());
   pref_change_registrar_.Add(
       prefs::kArcEnabled,
       base::Bind(&ArcSessionManager::OnOptInPreferenceChanged,
                  weak_ptr_factory_.GetWeakPtr()));
-  if (profile_->GetPrefs()->GetBoolean(prefs::kArcEnabled)) {
-    // Don't start ARC if there is a pending request to remove the data. Restart
-    // ARC once data removal finishes.
-    if (profile_->GetPrefs()->GetBoolean(prefs::kArcDataRemoveRequested)) {
-      reenable_arc_ = true;
-      VLOG(1) << "ARC previously requested to remove data.";
-      RemoveArcData();
-    } else {
-      OnOptInPreferenceChanged();
-    }
+
+  // Chrome may be shut down before completing ARC data removal.
+  // In such a case, start removing the data now.
+  if (profile_->GetPrefs()->GetBoolean(prefs::kArcDataRemoveRequested)) {
+    VLOG(1) << "ARC data removal requested in previous session.";
+    RemoveArcData();
+  }
+
+  if (IsArcPlayStoreEnabled()) {
+    VLOG(1) << "ARC is already enabled.";
+    DCHECK(!enabled_);
+    Enable();
   } else {
-    VLOG(1) << "ARC disabled on profile. Removing data.";
+    VLOG(1) << "ARC is initially disabled. Removing data.";
     RemoveArcData();
     PrefServiceSyncableFromProfile(profile_)->AddObserver(this);
     OnIsSyncingChanged();
   }
 }
 
 void ArcSessionManager::OnIsSyncingChanged() {
   sync_preferences::PrefServiceSyncable* const pref_service_syncable =
       PrefServiceSyncableFromProfile(profile_);
   if (!pref_service_syncable->IsSyncing())
     return;
 
   pref_service_syncable->RemoveObserver(this);
   if (!g_disable_ui_for_testing &&
       !base::CommandLine::ForCurrentProcess()->HasSwitch(
           chromeos::switches::kEnableArcOOBEOptIn) &&
       profile_->IsNewProfile() &&
       !profile_->GetPrefs()->HasPrefPath(prefs::kArcEnabled)) {
     ArcAuthNotification::Show(profile_);
   }
 }
 
 void ArcSessionManager::Shutdown() {
   if (!g_disable_ui_for_testing)
     ArcAuthNotification::Hide();
 
+  enabled_ = false;

[Luis Héctor Chávez, 2017/02/15 21:54:43]

Ah so this is what makes |enabled_| different from opt-in. A better name for
this would definitely help make the concept clearer.

[hidehiko, 2017/02/16 02:44:28]

Acknowledged.

   ShutdownSession();
   if (support_host_) {
     support_host_->Close();
     support_host_->RemoveObserver(this);
     support_host_.reset();
   }
   if (profile_) {
     sync_preferences::PrefServiceSyncable* pref_service_syncable =
         PrefServiceSyncableFromProfile(profile_);
     pref_service_syncable->RemoveObserver(this);
@@ skipping 27 matching lines @@
     if (!arc_enabled) {
       // Remove the pinned Play Store icon launcher in Shelf.
       // This is only for non-Managed cases. In managed cases, it is expected
       // to be "disabled" rather than "removed", so keep it here.
       ash::ShelfDelegate* shelf_delegate = GetShelfDelegate();
       if (shelf_delegate)
         shelf_delegate->UnpinAppWithID(ArcSupportHost::kHostAppId);
     }
   }
 
-  for (auto& observer : observer_list_)
-    observer.OnArcPlayStoreEnabledChanged(arc_enabled);
+  if (support_host_)
+    support_host_->SetArcManaged(IsArcManaged());
 
   // Hide auth notification if it was opened before and arc.enabled pref was
   // explicitly set to true or false.
   if (!g_disable_ui_for_testing &&
       profile_->GetPrefs()->HasPrefPath(prefs::kArcEnabled)) {
     ArcAuthNotification::Hide();
   }
 
-  if (!arc_enabled) {
-    // Reset any pending request to re-enable ARC.
-    VLOG(1) << "ARC opt-out. Removing user data.";
-    reenable_arc_ = false;
-    StopArc();
-    RemoveArcData();
-    return;
-  }
+  if (arc_enabled)
+    Enable();
+  else
+    Disable();
 
-  if (state_ == State::ACTIVE)
-    return;
-
-  if (state_ == State::REMOVING_DATA_DIR) {
-    // Data removal request is in progress. Set flag to re-enable Arc once it is
-    // finished.
-    reenable_arc_ = true;
-    return;
-  }
-
-  if (support_host_)
-    support_host_->SetArcManaged(IsArcManaged());
-
-  // For ARC Kiosk we skip ToS because it is very likely that near the device
-  // there will be no one who is eligible to accept them.
-  // TODO(poromov): Move to more Kiosk dedicated set-up phase.
-  if (IsArcKioskMode())
-    profile_->GetPrefs()->SetBoolean(prefs::kArcTermsAccepted, true);
-
-  // If it is marked that sign in has been successfully done, then directly
-  // start ARC.
-  // For testing, and for Kisok mode, we also skip ToS negotiation procedure.
-  // For backward compatibility, this check needs to be prior to the
-  // kArcTermsAccepted check below.
-  if (profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn) ||
-      IsArcOptInVerificationDisabled() || IsArcKioskMode()) {
-    StartArc();
-
-    // Skip Android management check for testing.
-    // We also skip if Android management check for Kiosk mode,
-    // because there are no managed human users for Kiosk exist.
-    if (IsArcOptInVerificationDisabled() || IsArcKioskMode() ||
-        (g_disable_ui_for_testing &&
-         !g_enable_check_android_management_for_testing)) {
-      return;
-    }
-
-    // Check Android management in parallel.
-    // Note: Because the callback may be called in synchronous way (i.e. called
-    // on the same stack), StartCheck() needs to be called *after* StartArc().
-    // Otherwise, SetArcPlayStoreEnabled() which may be called in
-    // OnBackgroundAndroidManagementChecked() could be ignored.
-    android_management_checker_ = base::MakeUnique<ArcAndroidManagementChecker>(
-        profile_, context_->token_service(), context_->account_id(),
-        true /* retry_on_error */);
-    android_management_checker_->StartCheck(
-        base::Bind(&ArcSessionManager::OnBackgroundAndroidManagementChecked,
-                   weak_ptr_factory_.GetWeakPtr()));
-    return;
-  }
-
-  // If it is marked that the Terms of service is accepted already,
-  // just skip the negotiation with user, and start Android management
-  // check directly.
-  // This happens, e.g., when;
-  // 1) User accepted the Terms of service on OOBE flow.
-  // 2) User accepted the Terms of service on Opt-in flow, but logged out
-  //   before ARC sign in procedure was done. Then, logs in again.
-  if (profile_->GetPrefs()->GetBoolean(prefs::kArcTermsAccepted)) {
-    // Don't show UI for this progress if it was not shown.
-    if (support_host_->ui_page() != ArcSupportHost::UIPage::NO_PAGE)
-      support_host_->ShowArcLoading();
-    StartArcAndroidManagementCheck();
-    return;
-  }
-
-  // Need user's explicit Terms Of Service agreement. Prevent race condition
-  // when ARC can be enabled before profile is synced. In last case
-  // OnOptInPreferenceChanged is called twice.
-  // TODO(crbug.com/687185): Remove the condition.
-  if (state_ != State::SHOWING_TERMS_OF_SERVICE)
-    StartTermsOfServiceNegotiation();
+  for (auto& observer : observer_list_)
+    observer.OnArcPlayStoreEnabledChanged(arc_enabled);
 }
 
 void ArcSessionManager::ShutdownSession() {
   arc_sign_in_timer_.Stop();
   playstore_launcher_.reset();
   terms_of_service_negotiator_.reset();
   android_management_checker_.reset();
   arc_session_runner_->RequestStop();
   // TODO(hidehiko): The ARC instance's stopping is asynchronous, so it might
   // still be running when we return from this function. Do not set the
@@ skipping 117 matching lines @@
     return;
   }
 
   profile_->GetPrefs()->SetBoolean(prefs::kArcEnabled, enabled);
 }
 
 void ArcSessionManager::RecordArcState() {
   // Only record 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 (IsAllowed())
-    UpdateEnabledStateUMA(IsArcPlayStoreEnabled());
+    UpdateEnabledStateUMA(enabled_);
+}
+
+void ArcSessionManager::Enable() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(profile_);
+
+  if (enabled_) {
+    VLOG(1) << "ARC is already enabled. Do nothing.";
+    return;
+  }
+  enabled_ = true;
+
+  VLOG(1) << "ARC opt-in. Starting ARC session.";
+  EnableImpl();
+}
+
+void ArcSessionManager::EnableImpl() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(profile_);
+  DCHECK_NE(state_, State::ACTIVE);
+
+  if (state_ == State::REMOVING_DATA_DIR) {
+    // Data removal request is in progress. Set flag to re-enable ARC once it
+    // is finished.
+    reenable_arc_ = true;
+    return;
+  }
+
+  // For ARC Kiosk we skip ToS because it is very likely that near the device
+  // there will be no one who is eligible to accept them.
+  // TODO(poromov): Move to more Kiosk dedicated set-up phase.
+  if (IsArcKioskMode())
+    profile_->GetPrefs()->SetBoolean(prefs::kArcTermsAccepted, true);
+
+  // If it is marked that sign in has been successfully done, then directly
+  // start ARC.
+  // For testing, and for Kisok mode, we also skip ToS negotiation procedure.
+  // For backward compatibility, this check needs to be prior to the
+  // kArcTermsAccepted check below.
+  if (profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn) ||
+      IsArcOptInVerificationDisabled() || IsArcKioskMode()) {
+    StartArc();
+
+    // Skip Android management check for testing.
+    // We also skip if Android management check for Kiosk mode,
+    // because there are no managed human users for Kiosk exist.
+    if (IsArcOptInVerificationDisabled() || IsArcKioskMode() ||
+        (g_disable_ui_for_testing &&
+         !g_enable_check_android_management_for_testing)) {
+      return;
+    }
+
+    // Check Android management in parallel.
+    // Note: Because the callback may be called in synchronous way (i.e. called
+    // on the same stack), StartCheck() needs to be called *after* StartArc().
+    // Otherwise, SetArcPlayStoreEnabled() which may be called in
+    // OnBackgroundAndroidManagementChecked() could be ignored.
+    android_management_checker_ = base::MakeUnique<ArcAndroidManagementChecker>(
+        profile_, context_->token_service(), context_->account_id(),
+        true /* retry_on_error */);
+    android_management_checker_->StartCheck(
+        base::Bind(&ArcSessionManager::OnBackgroundAndroidManagementChecked,
+                   weak_ptr_factory_.GetWeakPtr()));
+    return;
+  }
+
+  // If it is marked that the Terms of service is accepted already,
+  // just skip the negotiation with user, and start Android management
+  // check directly.
+  // This happens, e.g., when;
+  // 1) User accepted the Terms of service on OOBE flow.
+  // 2) User accepted the Terms of service on Opt-in flow, but logged out
+  //   before ARC sign in procedure was done. Then, logs in again.
+  if (profile_->GetPrefs()->GetBoolean(prefs::kArcTermsAccepted)) {
+    // Don't show UI for this progress if it was not shown.
+    if (support_host_->ui_page() != ArcSupportHost::UIPage::NO_PAGE)
+      support_host_->ShowArcLoading();
+    StartArcAndroidManagementCheck();
+    return;
+  }
+
+  StartTermsOfServiceNegotiation();
+}
+
+void ArcSessionManager::Disable() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(profile_);
+
+  if (!enabled_) {
+    VLOG(1) << "ARC is already disabled. Do nothing.";
+    return;
+  }
+  enabled_ = false;
+
+  // Reset any pending request to re-enable ARC.
+  VLOG(1) << "ARC opt-out. Removing user data.";
+  reenable_arc_ = false;
+  StopArc();
+  RemoveArcData();
 }
 
 void ArcSessionManager::StartTermsOfServiceNegotiation() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!terms_of_service_negotiator_);
 
   if (!arc_session_runner_->IsStopped()) {
     // If the user attempts to re-enable ARC while the ARC instance is still
     // running the user should not be able to continue until the ARC instance
     // has stopped.
@@ skipping 200 matching lines @@
 
 #undef MAP_STATE
 
   // Some compilers report an error even if all values of an enum-class are
   // covered exhaustively in a switch statement.
   NOTREACHED() << "Invalid value " << static_cast<int>(state);
   return os;
 }
 
 }  // namespace arc