Fix ArcSessionManager state machine, part 1.

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 46 matching lines @@
 
 // The Android management check is disabled by default, it's used only for
 // testing.
 bool g_enable_check_android_management_for_testing = false;
 
 // Maximum amount of time we'll wait for ARC to finish booting up. Once this
 // timeout expires, keep ARC running in case the user wants to file feedback,
 // but present the UI to try again.
 constexpr base::TimeDelta kArcSignInTimeout = base::TimeDelta::FromMinutes(5);
 
+// Updates UMA with user cancel only if error is not currently shown.
+void MaybeUpdateOptInCancelUMA(const ArcSupportHost* support_host) {
+  if (!support_host ||
+      support_host->ui_page() == ArcSupportHost::UIPage::NO_PAGE ||
+      support_host->ui_page() == ArcSupportHost::UIPage::ERROR) {
+    return;
+  }
+
+  UpdateOptInCancelUMA(OptInCancelReason::USER_CANCEL);
+}
+
 }  // namespace
 
 ArcSessionManager::ArcSessionManager(
     std::unique_ptr<ArcSessionRunner> arc_session_runner)
     : arc_session_runner_(std::move(arc_session_runner)),
       attempt_user_exit_callback_(base::Bind(chrome::AttemptUserExit)),
       weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!g_arc_session_manager);
   g_arc_session_manager = this;
@@ skipping 24 matching lines @@
   registry->RegisterBooleanPref(prefs::kArcDataRemoveRequested, false);
   registry->RegisterBooleanPref(prefs::kArcEnabled, false);
   registry->RegisterBooleanPref(prefs::kArcSignedIn, false);
   registry->RegisterBooleanPref(prefs::kArcTermsAccepted, false);
   // Note that ArcBackupRestoreEnabled and ArcLocationServiceEnabled prefs have
   // to be off by default, until an explicit gesture from the user to enable
   // them is received. This is crucial in the cases when these prefs transition
   // from a previous managed state to the unmanaged.
   registry->RegisterBooleanPref(prefs::kArcBackupRestoreEnabled, false);
   registry->RegisterBooleanPref(prefs::kArcLocationServiceEnabled, false);
   // This is used to delete the Play user ID if ARC is disabled for an

[hidehiko, 2017/03/07 14:52:20]

Note: Sorry, mistakenly mixed rebase.

   // AD-managed device.
   registry->RegisterStringPref(prefs::kArcActiveDirectoryPlayUserId,
                                std::string());
 }
 
 // static
 bool ArcSessionManager::IsOobeOptInActive() {
   // ARC OOBE OptIn is optional for now. Test if it exists and login host is
   // active.
   if (!user_manager::UserManager::Get()->IsCurrentUserNew())
@@ skipping 57 matching lines @@
   profile_->GetPrefs()->SetBoolean(prefs::kArcDataRemoveRequested, true);
 
   if (!arc_session_runner_->IsStopped()) {
     // Just set a flag. On session stopped, this will be re-called,
     // then session manager should remove the data.
     return;
   }
 
   VLOG(1) << "Starting ARC data removal";
 
   // Remove Play user ID for Active Directory managed devices.

[hidehiko, 2017/03/07 14:52:20]

Note: Sorry, mistakenly mixed rebase.

   profile_->GetPrefs()->SetString(prefs::kArcActiveDirectoryPlayUserId,
                                   std::string());
 
   SetState(State::REMOVING_DATA_DIR);
   chromeos::DBusThreadManager::Get()->GetSessionManagerClient()->RemoveArcData(
       cryptohome::Identification(
           multi_user_util::GetAccountIdFromProfile(profile_)),
       base::Bind(&ArcSessionManager::OnArcDataRemoved,
                  weak_ptr_factory_.GetWeakPtr()));
 }
@@ skipping 313 matching lines @@
 
   if (state_ == State::NOT_INITIALIZED) {
     NOTREACHED();
     return;
   }
 
   // If ARC failed to boot normally, stop ARC. Similarly, if the current page is
   // LSO, closing the window should stop ARC since the user activity chooses to
   // not sign in. In any other case, ARC is booting normally and the instance
   // should not be stopped.
-  if ((state_ != State::SHOWING_TERMS_OF_SERVICE &&
+  if ((state_ != State::NEGOTIATING_TERMS_OF_SERVICE &&
        state_ != State::CHECKING_ANDROID_MANAGEMENT) &&
       (!support_host_ ||
        (support_host_->ui_page() != ArcSupportHost::UIPage::ERROR &&
         support_host_->ui_page() != ArcSupportHost::UIPage::LSO))) {
     return;
   }
 
-  // Update UMA with user cancel only if error is not currently shown.
-  if (support_host_ &&
-      support_host_->ui_page() != ArcSupportHost::UIPage::NO_PAGE &&
-      support_host_->ui_page() != ArcSupportHost::UIPage::ERROR) {
-    UpdateOptInCancelUMA(OptInCancelReason::USER_CANCEL);
-  }
-
+  MaybeUpdateOptInCancelUMA(support_host_.get());
   StopArc();
   SetArcPlayStoreEnabledForProfile(profile_, false);
 }
 
 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(enable_requested_);
 }
@@ skipping 23 matching lines @@
 
   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;
   }
 
   PrefService* const prefs = profile_->GetPrefs();
 
-  // 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())
-    prefs->SetBoolean(prefs::kArcTermsAccepted, true);
-
-  // Skip to show UI asking users to set up ARC OptIn preferences, if all of
-  // them are managed by the admin policy. Note that the ToS agreement is anyway
-  // not shown in the case of the managed ARC.
-  if (IsArcPlayStoreEnabledPreferenceManagedForProfile(profile_) &&
-      AreArcAllOptInPreferencesManagedForProfile(profile_)) {
-    prefs->SetBoolean(prefs::kArcTermsAccepted, true);
-  }
-
   // If it is marked that sign in has been successfully done, then directly
   // start ARC.
-  // For testing, and for Kiosk mode, we also skip ToS negotiation procedure.
-  // For backward compatibility, this check needs to be prior to the
-  // kArcTermsAccepted check below.
-  if (prefs->GetBoolean(prefs::kArcSignedIn) ||
-      IsArcOptInVerificationDisabled() || IsArcKioskMode()) {
+  // For Kiosk mode, skip ToS because it is very likely that near the device
+  // there will be no one who is eligible to accept them.
+  // If opt-in verification is disabled, skip negotiation, too. This is for
+  // testing purpose.
+  if (prefs->GetBoolean(prefs::kArcSignedIn) || IsArcKioskMode() ||
+      IsArcOptInVerificationDisabled()) {
     StartArc();
     // Check Android management in parallel.
     // Note: StartBackgroundAndroidManagementCheck() may call
     // OnBackgroundAndroidManagementChecked() synchronously (or
     // asynchornously). In the callback, Google Play Store enabled preference
     // can be set to false if managed, and it triggers RequestDisable() via
     // ArcPlayStoreEnabledPreferenceHandler.
     // Thus, StartArc() should be called so that disabling should work even
     // if synchronous call case.
     StartBackgroundAndroidManagementCheck();
     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 (prefs->GetBoolean(prefs::kArcTermsAccepted)) {
-    // Don't show UI for this progress if it was not shown.
-    if (support_host_ &&
-        support_host_->ui_page() != ArcSupportHost::UIPage::NO_PAGE) {
-      support_host_->ShowArcLoading();
-    }
-    StartAndroidManagementCheck();
-    return;
-  }
-
   StartTermsOfServiceNegotiation();
 }
 
 void ArcSessionManager::RequestDisable() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(profile_);
 
   if (!enable_requested_) {
     VLOG(1) << "ARC is already disabled. Do nothing.";
     return;
   }
   enable_requested_ = 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(profile_);
   DCHECK(!terms_of_service_negotiator_);
+  // In Kiosk-mode, Terms of Service negotiation should be skipped.
+  // See also RequestEnableImpl().
+  DCHECK(!IsArcKioskMode());
+  // If opt-in verification is disabled, Terms of Service negotiation should
+  // be skipped, too. See also RequestEnableImpl().
+  DCHECK(!IsArcOptInVerificationDisabled());
 
+  // TODO(hidehiko): Remove this condition, when the state machine is fixed.
   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.
     if (support_host_) {
       support_host_->ShowError(
           ArcSupportHost::Error::SIGN_IN_SERVICE_UNAVAILABLE_ERROR, false);
     }
     return;
   }
 
-  SetState(State::SHOWING_TERMS_OF_SERVICE);
+  // TODO(hidehiko): DCHECK if |state_| is STOPPED, when the state machine
+  // is fixed.
+  SetState(State::NEGOTIATING_TERMS_OF_SERVICE);
+
+  if (!IsArcTermsOfServiceNegotiationNeeded()) {
+    // Moves to next state, Android management check, immediately, as if
+    // Terms of Service negotiation is done successfully.
+    StartAndroidManagementCheck();
+    return;
+  }
+
   if (IsOobeOptInActive()) {
     VLOG(1) << "Use OOBE negotiator.";
     terms_of_service_negotiator_ =
         base::MakeUnique<ArcTermsOfServiceOobeNegotiator>();
   } else if (support_host_) {
     VLOG(1) << "Use default negotiator.";
     terms_of_service_negotiator_ =
         base::MakeUnique<ArcTermsOfServiceDefaultNegotiator>(
             profile_->GetPrefs(), support_host_.get());
+  } else {
+    // The only case reached here is when g_disable_ui_for_testing is set
+    // so ARC support host is not created in SetProfile(), for testing purpose.
+    DCHECK(g_disable_ui_for_testing)
+        << "Negotiator is not created on production.";
+    return;
   }
 
-  if (terms_of_service_negotiator_) {
-    terms_of_service_negotiator_->StartNegotiation(
-        base::Bind(&ArcSessionManager::OnTermsOfServiceNegotiated,
-                   weak_ptr_factory_.GetWeakPtr()));
-  }
+  terms_of_service_negotiator_->StartNegotiation(
+      base::Bind(&ArcSessionManager::OnTermsOfServiceNegotiated,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void ArcSessionManager::OnTermsOfServiceNegotiated(bool accepted) {
+  DCHECK_EQ(state_, State::NEGOTIATING_TERMS_OF_SERVICE);
+  DCHECK(profile_);
   DCHECK(terms_of_service_negotiator_);
   terms_of_service_negotiator_.reset();
 
   if (!accepted) {
-    // To cancel, user needs to close the window. Note that clicking "Cancel"
-    // button effectively just closes the window.
-    CancelAuthCode();
+    // User does not accept the Terms of Service. Disable Google Play Store.
+    MaybeUpdateOptInCancelUMA(support_host_.get());
+    SetArcPlayStoreEnabledForProfile(profile_, false);
     return;
   }
 
   // Terms were accepted.
   profile_->GetPrefs()->SetBoolean(prefs::kArcTermsAccepted, true);
+  StartAndroidManagementCheck();
+}
 
-  // Don't show UI for this progress if it was not shown.
-  if (support_host_ &&
-      support_host_->ui_page() != ArcSupportHost::UIPage::NO_PAGE)
-    support_host_->ShowArcLoading();
-  StartAndroidManagementCheck();
+bool ArcSessionManager::IsArcTermsOfServiceNegotiationNeeded() const {
+  DCHECK(profile_);
+
+  // Skip to show UI asking users to set up ARC OptIn preferences, if all of
+  // them are managed by the admin policy. Note that the ToS agreement is anyway
+  // not shown in the case of the managed ARC.
+  if (AreArcAllOptInPreferencesManagedForProfile(profile_)) {
+    VLOG(1) << "All opt-in preferences are under managed. "
+            << "Skip ARC Terms of Service negotiation.";
+    return false;
+  }
+
+  // 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 a 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)) {
+    VLOG(1) << "The user already accepts ARC Terms of Service.";
+    return false;
+  }
+
+  return true;
 }
 
 void ArcSessionManager::StartAndroidManagementCheck() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(arc_session_runner_->IsStopped());
-  DCHECK(state_ == State::SHOWING_TERMS_OF_SERVICE ||
-         state_ == State::CHECKING_ANDROID_MANAGEMENT ||
-         (state_ == State::STOPPED &&
-          profile_->GetPrefs()->GetBoolean(prefs::kArcTermsAccepted)));
-  DCHECK(!android_management_checker_);
+  DCHECK(state_ == State::NEGOTIATING_TERMS_OF_SERVICE ||
+         state_ == State::CHECKING_ANDROID_MANAGEMENT);
   SetState(State::CHECKING_ANDROID_MANAGEMENT);
 
+  // Show loading UI only if ARC support app's window is already shown.
+  // User may not see any ARC support UI if everything needed is done in
+  // background. In such a case, showing loading UI here (then closed sometime
+  // soon later) would look just noisy.
+  if (support_host_ &&
+      support_host_->ui_page() != ArcSupportHost::UIPage::NO_PAGE) {
+    support_host_->ShowArcLoading();
+  }
+
   android_management_checker_ = base::MakeUnique<ArcAndroidManagementChecker>(
       profile_, context_->token_service(), context_->account_id(),
       false /* retry_on_error */);
   android_management_checker_->StartCheck(
       base::Bind(&ArcSessionManager::OnAndroidManagementChecked,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void ArcSessionManager::OnAndroidManagementChecked(
     policy::AndroidManagementClient::Result result) {
@@ skipping 133 matching lines @@
     support_host_->ShowArcLoading();
     ShutdownSession();
     reenable_arc_ = true;
   } else if (state_ == State::ACTIVE) {
     // This case is handled in ArcAuthService.
     // Do nothing.
   } else {
     // Otherwise, we restart ARC. Note: this is the first boot case.
     // For second or later boot, either ERROR_WITH_FEEDBACK case or ACTIVE
     // case must hit.
-    support_host_->ShowArcLoading();
     StartAndroidManagementCheck();
   }
 }
 
 void ArcSessionManager::OnSendFeedbackClicked() {
   DCHECK(support_host_);
   chrome::OpenFeedbackDialog(nullptr);
 }
 
 void ArcSessionManager::SetArcSessionRunnerForTesting(
@@ skipping 14 matching lines @@
 
 std::ostream& operator<<(std::ostream& os,
                          const ArcSessionManager::State& state) {
 #define MAP_STATE(name)                \
   case ArcSessionManager::State::name: \
     return os << #name
 
   switch (state) {
     MAP_STATE(NOT_INITIALIZED);
     MAP_STATE(STOPPED);
-    MAP_STATE(SHOWING_TERMS_OF_SERVICE);
+    MAP_STATE(NEGOTIATING_TERMS_OF_SERVICE);
     MAP_STATE(CHECKING_ANDROID_MANAGEMENT);
     MAP_STATE(REMOVING_DATA_DIR);
     MAP_STATE(ACTIVE);
   }
 
 #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