arc: Shuffle ArcAuthService's interface

chrome/browser/chromeos/arc/arc_auth_service.cc

 // Copyright 2015 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_auth_service.h"
 
 #include <utility>
 
 #include "ash/common/shelf/shelf_delegate.h"
 #include "ash/common/wm_shell.h"
@@ skipping 34 matching lines @@
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/app_window/app_window_registry.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "ui/base/l10n/l10n_util.h"
 
 namespace arc {
 
 namespace {
 
+constexpr size_t kMinVersionForOnAccountInfoReady = 5;
+
 // Weak pointer.  This class is owned by ArcServiceManager.
 ArcAuthService* g_arc_auth_service = nullptr;
 
 // Skip creating UI in unit tests
 bool g_disable_ui_for_testing = false;
 
 // Use specified ash::ShelfDelegate for unit tests.
 ash::ShelfDelegate* g_shelf_delegate_for_testing = nullptr;
 
 // The Android management check is disabled by default, it's used only for
@@ skipping 41 matching lines @@
     MAP_PROVISIONING_RESULT(CLOUD_PROVISION_FLOW_INTERNAL_ERROR);
   }
 #undef MAP_PROVISIONING_RESULT
 
   NOTREACHED() << "unknown reason: " << static_cast<int>(reason);
   return ProvisioningResult::UNKNOWN_ERROR;
 }
 
 }  // namespace
 
+class ArcAuthService::AccountInfoNotifier {

[khmel, 2016/11/03 20:16:45]

nit: May be move to separate class to improve readability? I am OK with this too
also OK with TODO.

[Luis Héctor Chávez, 2016/11/04 15:43:13]

Actually my goal is to eventually deprecate the old interfaces, so I'll add the
TODO to remove this instead of moving it out.

+ public:
+  explicit AccountInfoNotifier(
+      const GetAuthCodeDeprecatedCallback& auth_callback)
+      : callback_type_(CallbackType::AUTH_CODE),
+        auth_callback_(auth_callback) {}
+
+  explicit AccountInfoNotifier(
+      const GetAuthCodeAndAccountTypeDeprecatedCallback& auth_account_callback)
+      : callback_type_(CallbackType::AUTH_CODE_AND_ACCOUNT),
+        auth_account_callback_(auth_account_callback) {}
+
+  explicit AccountInfoNotifier(const AccountInfoCallback& account_info_callback)
+      : callback_type_(CallbackType::ACCOUNT_INFO),
+        account_info_callback_(account_info_callback) {}
+
+  void Notify(bool is_enforced,
+              const std::string& auth_code,
+              mojom::ChromeAccountType account_type,
+              bool is_managed) {
+    switch (callback_type_) {
+      case CallbackType::AUTH_CODE:
+        DCHECK(!auth_callback_.is_null());
+        auth_callback_.Run(auth_code, is_enforced);
+        break;
+      case CallbackType::AUTH_CODE_AND_ACCOUNT:
+        DCHECK(!auth_account_callback_.is_null());
+        auth_account_callback_.Run(auth_code, is_enforced, account_type);
+        break;
+      case CallbackType::ACCOUNT_INFO:
+        DCHECK(!account_info_callback_.is_null());
+        mojom::AccountInfoPtr account_info = mojom::AccountInfo::New();
+        if (!is_enforced) {
+          account_info->auth_code = nullptr;
+        } else {
+          account_info->auth_code = auth_code;
+        }
+        account_info->account_type = account_type;
+        account_info->is_managed = is_managed;
+        account_info_callback_.Run(std::move(account_info));
+        break;
+    }
+  }
+
+ private:
+  enum class CallbackType { AUTH_CODE, AUTH_CODE_AND_ACCOUNT, ACCOUNT_INFO };
+
+  const CallbackType callback_type_;
+  const GetAuthCodeDeprecatedCallback auth_callback_;
+  const GetAuthCodeAndAccountTypeDeprecatedCallback auth_account_callback_;
+  const AccountInfoCallback account_info_callback_;
+};
+
 ArcAuthService::ArcAuthService(ArcBridgeService* bridge_service)
     : ArcService(bridge_service), binding_(this), weak_ptr_factory_(this) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!g_arc_auth_service);
 
   g_arc_auth_service = this;
 
   arc_bridge_service()->AddObserver(this);
   arc_bridge_service()->auth()->AddObserver(this);
 }
@@ skipping 145 matching lines @@
   EnableArc();
 }
 
 std::string ArcAuthService::GetAndResetAuthCode() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   std::string auth_code;
   auth_code_.swap(auth_code);
   return auth_code;
 }
 
-void ArcAuthService::GetAuthCodeDeprecated(
-    const GetAuthCodeDeprecatedCallback& callback) {
+void ArcAuthService::GetAuthCodeDeprecated0(
+    const GetAuthCodeDeprecated0Callback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!IsOptInVerificationDisabled());
   callback.Run(GetAndResetAuthCode());
 }
 
-void ArcAuthService::GetAuthCode(const GetAuthCodeCallback& callback) {
+void ArcAuthService::GetAuthCodeDeprecated(
+    const GetAuthCodeDeprecatedCallback& callback) {
+  RequestAccountInfoInternal(
+      base::MakeUnique<ArcAuthService::AccountInfoNotifier>(callback));
+}
+
+void ArcAuthService::GetAuthCodeAndAccountTypeDeprecated(
+    const GetAuthCodeAndAccountTypeDeprecatedCallback& callback) {
+  RequestAccountInfoInternal(
+      base::MakeUnique<ArcAuthService::AccountInfoNotifier>(callback));
+}
+
+void ArcAuthService::RequestAccountInfo() {
+  RequestAccountInfoInternal(
+      base::MakeUnique<ArcAuthService::AccountInfoNotifier>(
+          base::Bind(&ArcAuthService::OnAccountInfoReady,
+                     weak_ptr_factory_.GetWeakPtr())));
+}
+
+void ArcAuthService::OnAccountInfoReady(mojom::AccountInfoPtr account_info) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  // GetAuthCodeAndAccountType operation must not be in progress.
-  DCHECK(auth_account_callback_.is_null());
+  auto* instance = arc_bridge_service()->auth()->GetInstanceForMethod(
+      "OnAccountInfoReady", kMinVersionForOnAccountInfoReady);
+  DCHECK(instance);
+  instance->OnAccountInfoReady(std::move(account_info));
+}
+
+void ArcAuthService::RequestAccountInfoInternal(
+    std::unique_ptr<ArcAuthService::AccountInfoNotifier>
+        account_info_notifier) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  // No other auth code-related operation may be in progress.
+  DCHECK(!account_info_notifier_);
 
   const std::string auth_code = GetAndResetAuthCode();
-  const bool verification_disabled = IsOptInVerificationDisabled();
-  if (!auth_code.empty() || verification_disabled) {
-    callback.Run(auth_code, !verification_disabled);
+  const bool is_enforced = !IsOptInVerificationDisabled();
+  if (!auth_code.empty() || !is_enforced) {
+    account_info_notifier->Notify(is_enforced, auth_code,
+                                  mojom::ChromeAccountType::USER_ACCOUNT,
+                                  policy_util::IsAccountManaged(profile_));
     return;
   }
 
-  auth_callback_ = callback;
-  PrepareContextForAuthCodeRequest();
-}
-
-void ArcAuthService::GetAuthCodeAndAccountType(
-    const GetAuthCodeAndAccountTypeCallback& callback) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  // GetAuthCode operation must not be in progress.
-  DCHECK(auth_callback_.is_null());
-
-  const std::string auth_code = GetAndResetAuthCode();
-  const bool verification_disabled = IsOptInVerificationDisabled();
-  if (!auth_code.empty() || verification_disabled) {
-    callback.Run(auth_code, !verification_disabled,
-                 mojom::ChromeAccountType::USER_ACCOUNT);
-    return;
-  }
-
-  auth_account_callback_ = callback;
+  account_info_notifier_ = std::move(account_info_notifier);
   PrepareContextForAuthCodeRequest();
 }
 
 bool ArcAuthService::IsAuthCodeRequest() const {
-  return !auth_callback_.is_null() || !auth_account_callback_.is_null();
+  return account_info_notifier_ != nullptr;
 }
 
 void ArcAuthService::PrepareContextForAuthCodeRequest() {
   // Requesting auth code on demand happens in following cases:
   // 1. To handle account password revoke.
   // 2. In case Arc is activated in OOBE flow.
   // 3. For any other state on Android side that leads device appears in
   // non-signed state.
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(IsAuthCodeRequest());
   DCHECK_EQ(state_, State::ACTIVE);
   initial_opt_in_ = false;
   context_->PrepareContext();
 }
 
 void ArcAuthService::OnSignInComplete() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK_EQ(state_, State::ACTIVE);
-  DCHECK(!sign_in_time_.is_null());
 
-  arc_sign_in_timer_.Stop();
+  CloseUI();
 
-  if (!IsOptInVerificationDisabled() &&
-      !profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn)) {
+  if (profile_->GetPrefs()->GetBoolean(prefs::kArcSignedIn))

[khmel, 2016/11/03 20:16:45]

Is it guaranty that timers are not set?

nit: maybe DCHECK(sign_in_time_.is_null());
           DCHECK(arc_sign_in_timer_.is_null()); ?

[Luis Héctor Chávez, 2016/11/04 15:43:13]

It should be guaranteed, but after more careful consideration, I think it's
better if this function mirrors OnSignInFailedInternal, so I'll use the same

if (!sign_in_time_.is_null()) {
  arc_sign_in_timer_.Stop();
  // update UMA
}

+    return;
+
+  profile_->GetPrefs()->SetBoolean(prefs::kArcSignedIn, true);
+  if (!IsOptInVerificationDisabled()) {
     playstore_launcher_.reset(
         new ArcAppLauncher(profile_, kPlayStoreAppId, true));
   }
 
-  profile_->GetPrefs()->SetBoolean(prefs::kArcSignedIn, true);
-  CloseUI();
+  DCHECK(!sign_in_time_.is_null());
+  arc_sign_in_timer_.Stop();
   UpdateProvisioningTiming(base::Time::Now() - sign_in_time_, true,
                            policy_util::IsAccountManaged(profile_));
   UpdateProvisioningResultUMA(ProvisioningResult::SUCCESS,
                               policy_util::IsAccountManaged(profile_));
 
   for (auto& observer : observer_list_)
     observer.OnInitialStart();
 }
 
 void ArcAuthService::OnSignInFailed(mojom::ArcSignInFailureReason reason) {
   OnSignInFailedInternal(
       ConvertArcSignInFailureReasonToProvisioningResult(reason));
 }
 
 void ArcAuthService::OnSignInFailedInternal(ProvisioningResult result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK_EQ(state_, State::ACTIVE);
-  DCHECK(!sign_in_time_.is_null());
 
-  arc_sign_in_timer_.Stop();
+  if (!sign_in_time_.is_null()) {
+    arc_sign_in_timer_.Stop();
 
-  UpdateProvisioningTiming(base::Time::Now() - sign_in_time_, false,
-                           policy_util::IsAccountManaged(profile_));
-  UpdateOptInCancelUMA(OptInCancelReason::CLOUD_PROVISION_FLOW_FAIL);
-  UpdateProvisioningResultUMA(result, policy_util::IsAccountManaged(profile_));
+    UpdateProvisioningTiming(base::Time::Now() - sign_in_time_, false,
+                             policy_util::IsAccountManaged(profile_));
+    UpdateOptInCancelUMA(OptInCancelReason::CLOUD_PROVISION_FLOW_FAIL);
+    UpdateProvisioningResultUMA(result,
+                                policy_util::IsAccountManaged(profile_));
+  }
 
   int error_message_id;
   switch (result) {
     case ProvisioningResult::GMS_NETWORK_ERROR:
       error_message_id = IDS_ARC_SIGN_IN_NETWORK_ERROR;
       break;
     case ProvisioningResult::GMS_SERVICE_UNAVAILABLE:
     case ProvisioningResult::GMS_SIGN_IN_FAILED:
     case ProvisioningResult::GMS_SIGN_IN_TIMEOUT:
     case ProvisioningResult::GMS_SIGN_IN_INTERNAL_ERROR:
@@ skipping 35 matching lines @@
       result == ProvisioningResult::UNKNOWN_ERROR) {
     RemoveArcData();
   }
 
   // We'll delay shutting down the bridge in this case to allow people to send
   // feedback.
   ShowUI(UIPage::ERROR_WITH_FEEDBACK,
          l10n_util::GetStringUTF16(error_message_id));
 }
 
-void ArcAuthService::GetIsAccountManaged(
-    const GetIsAccountManagedCallback& callback) {
+void ArcAuthService::GetIsAccountManagedDeprecated(
+    const GetIsAccountManagedDeprecatedCallback& callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   callback.Run(policy_util::IsAccountManaged(profile_));
 }
 
 void ArcAuthService::SetState(State state) {
   if (state_ == state)
     return;
 
   state_ = state;
@@ skipping 199 matching lines @@
                      weak_ptr_factory_.GetWeakPtr()));
     }
   }
 
   UpdateEnabledStateUMA(true);
 }
 
 void ArcAuthService::ShutdownBridge() {
   arc_sign_in_timer_.Stop();
   playstore_launcher_.reset();
-  auth_callback_.Reset();
-  auth_account_callback_.Reset();
+  account_info_notifier_.reset();
   android_management_checker_.reset();
   auth_code_fetcher_.reset();
   arc_bridge_service()->RequestStop();
   if (state_ != State::NOT_INITIALIZED)
     SetState(State::STOPPED);
   for (auto& observer : observer_list_)
     observer.OnShutdownBridge();
 }
 
 void ArcAuthService::ShutdownBridgeAndCloseUI() {
@@ skipping 48 matching lines @@
   SetState(State::ACTIVE);
 }
 
 void ArcAuthService::SetAuthCodeAndStartArc(const std::string& auth_code) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(!auth_code.empty());
 
   if (IsAuthCodeRequest()) {
     DCHECK_EQ(state_, State::FETCHING_CODE);
     SetState(State::ACTIVE);
-    if (!auth_callback_.is_null()) {
-      auth_callback_.Run(auth_code, !IsOptInVerificationDisabled());
-      auth_callback_.Reset();
-      return;
-    } else {
-      auth_account_callback_.Run(auth_code, !IsOptInVerificationDisabled(),
-                                 mojom::ChromeAccountType::USER_ACCOUNT);
-      auth_account_callback_.Reset();
-      return;
-    }
+    account_info_notifier_->Notify(!IsOptInVerificationDisabled(), auth_code,
+                                   mojom::ChromeAccountType::USER_ACCOUNT,
+                                   policy_util::IsAccountManaged(profile_));
+    account_info_notifier_.reset();
+    return;
   }
 
   if (state_ != State::FETCHING_CODE) {
     ShutdownBridgeAndCloseUI();
     return;
   }
 
   sign_in_time_ = base::Time::Now();
   VLOG(1) << "Starting ARC for first sign in.";
 
@@ skipping 219 matching lines @@
       return os << "FETCHING_CODE";
     case ArcAuthService::State::ACTIVE:
       return os << "ACTIVE";
     default:
       NOTREACHED();
       return os;
   }
 }
 
 }  // namespace arc