Add CheckAndroidManagement to ARC sign-in flow.

chrome/browser/chromeos/arc/arc_auth_service.cc

Index: chrome/browser/chromeos/arc/arc_auth_service.cc
diff --git a/chrome/browser/chromeos/arc/arc_auth_service.cc b/chrome/browser/chromeos/arc/arc_auth_service.cc
index 41bf9da80aa1d4e06f7776497138f953a7f75231..c526eae71df429fbda00b7e01a987ed4cde8428a 100644
--- a/chrome/browser/chromeos/arc/arc_auth_service.cc
+++ b/chrome/browser/chromeos/arc/arc_auth_service.cc
@@ -10,10 +10,14 @@
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
+#include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_checker.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/browser_process_platform_part.h"
 #include "chrome/browser/chromeos/arc/arc_auth_notification.h"
 #include "chrome/browser/chromeos/arc/arc_optin_uma.h"
+#include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/policy/profile_policy_connector.h"
@@ -28,6 +32,8 @@
 #include "chrome/grit/generated_resources.h"
 #include "chromeos/chromeos_switches.h"
 #include "components/arc/arc_bridge_service.h"
+#include "components/policy/core/browser/browser_policy_connector.h"
+#include "components/policy/core/common/cloud/device_management_service.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
@@ -58,6 +64,9 @@ const char kArcSupportStorageId[] = "arc_support";
 // Skip creating UI in unit tests
 bool disable_ui_for_testing = false;
 
+// Do check Android management requirement in browser tests.
+bool enable_check_android_management_for_testing = false;
+
 const char kStateStopped[] = "STOPPED";
 const char kStateFetchingCode[] = "FETCHING_CODE";
 const char kStateActive[] = "ACTIVE";
@@ -109,6 +118,11 @@ bool ArcAuthService::IsOptInVerificationDisabled() {
       chromeos::switches::kDisableArcOptInVerification);
 }
 
+// static
+void ArcAuthService::EnableCheckAndroidManagementForTesting() {
+  enable_check_android_management_for_testing = true;
+}
+
 void ArcAuthService::OnAuthInstanceReady() {
   arc_bridge_service()->auth_instance()->Init(
       binding_.CreateInterfacePtrAndBind());
@@ -201,7 +215,7 @@ void ArcAuthService::GetIsAccountManaged(
 }
 
 void ArcAuthService::SetState(State state) {
-  if (state_ == state)
+  if (state_ == state && !enable_check_android_management_for_testing)
     return;
 
   state_ = state;
@@ -233,6 +247,9 @@ void ArcAuthService::OnPrimaryUserProfilePrepared(Profile* profile) {
 
   // In case UI is disabled we assume that ARC is opted-in.
   if (!IsOptInVerificationDisabled()) {
+    if (!disable_ui_for_testing || enable_check_android_management_for_testing)
+      StartAndroidManagementClient();
+
     pref_change_registrar_.Init(profile_->GetPrefs());
     pref_change_registrar_.Add(
         prefs::kArcEnabled,
@@ -278,6 +295,7 @@ void ArcAuthService::Shutdown() {
   }
   pref_change_registrar_.RemoveAll();
   profile_ = nullptr;
+  android_management_client_.reset();

[khmel1, 2016/04/20 19:13:31]

Probably ArcAuthService::ShutdownBridge is better place

[Polina Bondarenko, 2016/04/25 19:59:49]

If android_management_client_ is destroyed not at the same time with profile_,
then the next scenario is possible:

1) Error is occurred during LSO.
2) That causes ShutdownBridge call. -> android_management_client_ is destroyed.
3) User presses "Retry" on error screen.
4) Start the flow again, but android_management_client_ has been already
destroyed and OnPrimaryUserProfilePrepared hasn't been called since we re-do the
flow for the current profile.
5) It causes failure:(

[khmel, 2016/04/27 16:07:42]

Thanks for explanation. Short comment may be welcome here.

[Polina Bondarenko, 2016/04/27 16:58:50]

Added comment.

 }
 
 void ArcAuthService::ShowUI(UIPage page, const base::string16& status) {
@@ -305,8 +323,8 @@ void ArcAuthService::OnMergeSessionSuccess(const std::string& data) {
   DCHECK(thread_checker.Get().CalledOnValidThread());
 
   DCHECK(!initial_opt_in_);
-  context_prepared_ = true;
   ShowUI(UIPage::LSO_PROGRESS, base::string16());
+  CheckAndroidManagement();
 }
 
 void ArcAuthService::OnMergeSessionFailure(
@@ -321,6 +339,7 @@ void ArcAuthService::OnUbertokenSuccess(const std::string& token) {
   merger_fetcher_.reset(
       new GaiaAuthFetcher(this, GaiaConstants::kChromeOSSource,
                           storage_partition_->GetURLRequestContext()));
+  auth_token_ = token;

[xiyuan, 2016/04/20 18:21:41]

|token| is uber token, not sure if it is what you can use with DM request. You
might need a login scoped access token instead.

[Polina Bondarenko, 2016/04/25 19:59:49]

Thanks, fixed to access token.

   merger_fetcher_->StartMergeSession(token, std::string());
 }
 
@@ -356,8 +375,12 @@ void ArcAuthService::OnOptInPreferenceChanged() {
         initial_opt_in_ = true;
         StartUI();
       } else {
-        // Ready to start Arc.
-        StartArc();
+        // Ready to start Arc, but check android management first.
+        if (!disable_ui_for_testing ||
+            enable_check_android_management_for_testing)

[khmel1, 2016/04/20 19:13:31]

nit: 2+ lines, please use {}

[Polina Bondarenko, 2016/04/25 19:59:49]

Done.

+          CheckAndroidManagement();
+        else
+          StartArc();
       }
 
       UpdateEnabledStateUMA(true);
@@ -455,7 +478,7 @@ void ArcAuthService::StartLso() {
 void ArcAuthService::CancelAuthCode() {
   DCHECK(thread_checker.Get().CalledOnValidThread());
 
-  if (state_ != State::FETCHING_CODE)
+  if (state_ != State::FETCHING_CODE && ui_page_ != UIPage::ERROR)
     return;
 
   // Update UMA with user cancel only if error is not currently shown.
@@ -515,6 +538,63 @@ void ArcAuthService::OnPrepareContextFailed() {
   UpdateOptInCancelUMA(OptInCancelReason::NETWORK_ERROR);
 }
 
+void ArcAuthService::StartAndroidManagementClient() {
+  policy::BrowserPolicyConnectorChromeOS* const connector =
+      g_browser_process->platform_part()->browser_policy_connector_chromeos();
+  policy::DeviceManagementService* const service =
+      connector->device_management_service();
+  service->ScheduleInitialization(0);
+  android_management_client_.reset(new policy::AndroidManagementClient(
+      service, g_browser_process->system_request_context()));

[xiyuan, 2016/04/20 18:21:41]

Should we be using storage_partition_->GetURLRequestContext()? Or request
context does not really matter since we don't send and set cookies?

[Polina Bondarenko, 2016/04/25 19:59:49]

Done.

+}
+
+void ArcAuthService::CheckAndroidManagement() {
+  // Do not send requests for Chrome OS managed users.
+  if (policy::ProfilePolicyConnectorFactory::GetForBrowserContext(profile_)
+          ->IsManaged()) {

[xiyuan, 2016/04/20 18:21:41]

Would ProfilePolicyConnectorFactory::GetForBrowserContext() return nullptr for
an unmanaged user?

[Polina Bondarenko, 2016/04/25 19:59:49]

No, it seems like it always returns a value.

+    OnAndroidManagementChecked(
+        policy::AndroidManagementClient::Result::RESULT_UNMANAGED);
+    return;
+  }
+
+  // Do not send requests for well-known consumer domains.
+  if (policy::BrowserPolicyConnector::IsNonEnterpriseUser(
+          profile_->GetProfileUserName())) {
+    OnAndroidManagementChecked(
+        policy::AndroidManagementClient::Result::RESULT_UNMANAGED);
+    return;
+  }
+
+  android_management_client_->CheckAndroidManagement(
+      auth_token_, base::Bind(&ArcAuthService::OnAndroidManagementChecked,
+                              base::Unretained(this)));

[xiyuan, 2016/04/20 18:21:41]

WeakPtr instead of base::Unretained. Or add a comment to explain why it is safe
to use base::Unretained here.

[Polina Bondarenko, 2016/04/25 19:59:49]

Done.

+}
+
+void ArcAuthService::OnAndroidManagementChecked(
+    policy::AndroidManagementClient::Result result) {
+  switch (result) {
+    case policy::AndroidManagementClient::Result::RESULT_UNMANAGED:
+      context_prepared_ = true;
+      if (!profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn))
+        ShowUI(UIPage::LSO, base::string16());

[khmel1, 2016/04/20 19:13:31]

Please use UIPage::LSO_PROGRESS... instead UIPage::LSO

[Polina Bondarenko, 2016/04/25 19:59:49]

Done.

+      else
+        StartArc();
+      break;
+    case policy::AndroidManagementClient::Result::RESULT_MANAGED:
+      ShutdownBridgeAndShowUI(
+          UIPage::ERROR,
+          l10n_util::GetStringUTF16(IDS_ARC_ANDROID_MANAGEMENT_REQUIRED_ERROR));
+      UpdateOptInCancelUMA(OptInCancelReason::ANDROID_MANAGEMENT_REQUIRED);
+      break;
+    case policy::AndroidManagementClient::Result::RESULT_ERROR:
+      ShutdownBridgeAndShowUI(
+          UIPage::ERROR,
+          l10n_util::GetStringUTF16(IDS_ARC_SERVER_COMMUNICATION_ERROR));
+      UpdateOptInCancelUMA(OptInCancelReason::NETWORK_ERROR);
+      break;
+  }

[khmel1, 2016/04/20 19:13:31]

nit:
default:
 NOTREACHED();
?

[Polina Bondarenko, 2016/04/25 19:59:49]

Done.

+}
+
 std::ostream& operator<<(std::ostream& os, const ArcAuthService::State& state) {
   switch (state) {
     case ArcAuthService::State::STOPPED: