Keep state even if AndroidManagement check returns MANAGED or ERROR.

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 570 matching lines @@
     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()) {
     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, SetArcPlayStoreEnabledForProfile() 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()));
+    // Note: The callback OnBackgroundAndroidManagementChecked() can be called

[Yusuke Sato, 2017/03/06 22:11:49]

nit: 'StartBackgroundAndroidManagementCheck() may call
OnBackgroundAndroidManagementChecked() synchronously' (active voice) may be
simpler and clearer.

[hidehiko, 2017/03/07 05:09:59]

Done.

+    // synchronously or asynchronously. 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();
     }
-    StartArcAndroidManagementCheck();
+    StartAndroidManagementCheck();
     return;
   }
 
   StartTermsOfServiceNegotiation();
 }
 
 void ArcSessionManager::RequestDisable() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(profile_);
 
@@ skipping 55 matching lines @@
     return;
   }
 
   // Terms were accepted.
   profile_->GetPrefs()->SetBoolean(prefs::kArcTermsAccepted, true);
 
   // 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();
-  StartArcAndroidManagementCheck();
+  StartAndroidManagementCheck();
 }
 
 bool ArcSessionManager::AreArcAllOptInPreferencesManaged() const {
   return IsArcPlayStoreEnabledPreferenceManagedForProfile(profile_) &&
          profile_->GetPrefs()->IsManagedPreference(
              prefs::kArcBackupRestoreEnabled) &&
          profile_->GetPrefs()->IsManagedPreference(
              prefs::kArcLocationServiceEnabled);
 }
 
-void ArcSessionManager::StartArcAndroidManagementCheck() {
+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_);
   SetState(State::CHECKING_ANDROID_MANAGEMENT);
 
-  android_management_checker_.reset(new ArcAndroidManagementChecker(
+  android_management_checker_ = base::MakeUnique<ArcAndroidManagementChecker>(
       profile_, context_->token_service(), context_->account_id(),
-      false /* retry_on_error */));
+      false /* retry_on_error */);
   android_management_checker_->StartCheck(
       base::Bind(&ArcSessionManager::OnAndroidManagementChecked,
                  weak_ptr_factory_.GetWeakPtr()));
 }
 
 void ArcSessionManager::OnAndroidManagementChecked(
     policy::AndroidManagementClient::Result result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK_EQ(state_, State::CHECKING_ANDROID_MANAGEMENT);
+  DCHECK(android_management_checker_);
+  android_management_checker_.reset();
 
   switch (result) {
     case policy::AndroidManagementClient::Result::UNMANAGED:
       VLOG(1) << "Starting ARC for first sign in.";
       sign_in_start_time_ = base::Time::Now();
       arc_sign_in_timer_.Start(
           FROM_HERE, kArcSignInTimeout,
           base::Bind(&ArcSessionManager::OnArcSignInTimeout,
                      weak_ptr_factory_.GetWeakPtr()));
       StartArc();
       break;
     case policy::AndroidManagementClient::Result::MANAGED:
-      ShutdownSession();
       if (support_host_) {
         support_host_->ShowError(
             ArcSupportHost::Error::ANDROID_MANAGEMENT_REQUIRED_ERROR, false);
       }
       UpdateOptInCancelUMA(OptInCancelReason::ANDROID_MANAGEMENT_REQUIRED);
       break;
     case policy::AndroidManagementClient::Result::ERROR:
-      ShutdownSession();
       if (support_host_) {
         support_host_->ShowError(
             ArcSupportHost::Error::SERVER_COMMUNICATION_ERROR, false);
       }
       UpdateOptInCancelUMA(OptInCancelReason::NETWORK_ERROR);
       break;
   }
 }
 
+void ArcSessionManager::StartBackgroundAndroidManagementCheck() {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK_EQ(state_, State::ACTIVE);
+  DCHECK(!android_management_checker_);
+
+  // 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;
+  }
+
+  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()));
+}
+
 void ArcSessionManager::OnBackgroundAndroidManagementChecked(
     policy::AndroidManagementClient::Result result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(android_management_checker_);
+  android_management_checker_.reset();
+
   switch (result) {
     case policy::AndroidManagementClient::Result::UNMANAGED:
       // Do nothing. ARC should be started already.
       break;
     case policy::AndroidManagementClient::Result::MANAGED:
       SetArcPlayStoreEnabledForProfile(profile_, false);
       break;
     case policy::AndroidManagementClient::Result::ERROR:
       // This code should not be reached. For background check,
       // retry_on_error should be set.
@@ skipping 64 matching lines @@
     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();
-    StartArcAndroidManagementCheck();
+    StartAndroidManagementCheck();
   }
 }
 
 void ArcSessionManager::OnSendFeedbackClicked() {
   DCHECK(support_host_);
   chrome::OpenFeedbackDialog(nullptr);
 }
 
 void ArcSessionManager::SetArcSessionRunnerForTesting(
     std::unique_ptr<ArcSessionRunner> arc_session_runner) {
@@ skipping 28 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