chromeos: Migrate DriveFS off the mojo Identity Service

The code in //chromeos/components/drivefs runs in the browser process
on the UI thread. There aren't any plans to move it out of process.
It can directly use the C++ IdentityManager instead of using the mojo
Identity Service. This will eliminate the last client of the Identity
Service, making it easier to refactor or delete it.

Convert DriveFsAuth to use PrimaryAccountAccessTokenFetcher, which
automatically handles waiting for / getting information about the
primary account.

Migrate the tests to use IdentityTestEnvironment, specifically the
helpers for access token requests. This requires rewriting tests
that used GMock on the Identity Service mojo API, but ends up being
less code overall.

Test: rewrite the chromeos_components_unittests
Test: Google Drive still works in File Manager to read / write / copy
      files.
Bug: 1054673

Change-Id: If83be823ee2ab1936289e21b2c50451004557c17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2063419
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Austin Tankiang <austinct@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744097}

chrome/browser/chromeos/drive/drive_integration_service.cc

@@ -48,6 +48,7 @@
 #include "components/metrics/metrics_pref_names.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/prefs/pref_service.h"
+#include "components/signin/public/identity_manager/identity_manager.h"
 #include "components/user_manager/user.h"
 #include "components/version_info/version_info.h"
 #include "content/public/browser/browser_context.h"
@@ -60,7 +61,6 @@
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
 #include "services/device/public/mojom/wake_lock_provider.mojom.h"
-#include "services/identity/public/mojom/identity_service.mojom.h"
 #include "services/network/public/cpp/network_connection_tracker.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "services/service_manager/public/cpp/connector.h"
@@ -456,12 +456,8 @@ class DriveIntegrationService::DriveFsHolder
     return profile_->GetURLLoaderFactory();
   }
 
-  void BindIdentityAccessor(
-      mojo::PendingReceiver<identity::mojom::IdentityAccessor> receiver)
-      override {
-    auto* service = profile_->GetIdentityService();
-    if (service)
-      service->BindIdentityAccessor(std::move(receiver));
+  signin::IdentityManager* GetIdentityManager() override {
+    return IdentityManagerFactory::GetForProfile(profile_);
   }
 
   const AccountId& GetAccountId() override {

chromeos/components/drivefs/BUILD.gn

@@ -30,12 +30,12 @@ component("drivefs") {
     "//chromeos/disks",
     "//components/account_id",
     "//components/drive",
+    "//components/signin/public/identity_manager",
     "//dbus",
     "//google_apis",
     "//mojo/public/cpp/bindings",
     "//mojo/public/cpp/platform",
     "//net",
-    "//services/identity/public/mojom",
     "//services/network/public/cpp:cpp",
   ]
   defines = [ "IS_DRIVEFS_IMPL" ]
@@ -77,10 +77,11 @@ source_set("unit_tests") {
     "//components/account_id",
     "//components/drive",
     "//components/invalidation/impl:test_support",
+    "//components/signin/public/identity_manager",
+    "//components/signin/public/identity_manager:test_support",
     "//mojo/public/cpp/bindings",
     "//net",
     "//net:test_support",
-    "//services/identity/public/mojom",
     "//services/network:test_support",
     "//services/network/public/cpp:cpp",
     "//testing/gmock",

chromeos/components/drivefs/DEPS

 include_rules = [
   "+components/drive",
   "+components/invalidation/impl/fake_invalidation_service.h",
+  "+components/signin",
   "+mojo/public",
-  "+services/identity/public",
 ]

chromeos/components/drivefs/drivefs_auth.cc

@@ -6,6 +6,11 @@
 
 #include "base/bind.h"
 #include "components/account_id/account_id.h"
+#include "components/signin/public/identity_manager/access_token_info.h"
+#include "components/signin/public/identity_manager/consent_level.h"
+#include "components/signin/public/identity_manager/identity_manager.h"
+#include "components/signin/public/identity_manager/primary_account_access_token_fetcher.h"
+#include "google_apis/gaia/google_service_auth_error.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 
 namespace drivefs {
@@ -23,7 +28,7 @@ DriveFsAuth::DriveFsAuth(const base::Clock* clock,
       timer_(std::move(timer)),
       delegate_(delegate) {}
 
-DriveFsAuth::~DriveFsAuth() {}
+DriveFsAuth::~DriveFsAuth() = default;
 
 base::Optional<std::string> DriveFsAuth::GetCachedAccessToken() {
   const auto& token = GetOrResetCachedToken(true);
@@ -48,35 +53,32 @@ void DriveFsAuth::GetAccessToken(
     return;
   }
 
+  signin::IdentityManager* identity_manager = delegate_->GetIdentityManager();
+  if (!identity_manager) {
+    std::move(callback).Run(mojom::AccessTokenStatus::kAuthError, "");
+    return;
+  }
   get_access_token_callback_ = std::move(callback);
-  timer_->Start(FROM_HERE, base::TimeDelta::FromSeconds(30),
-                base::BindOnce(&DriveFsAuth::AuthTimeout,
-                               weak_ptr_factory_.GetWeakPtr()));
-  GetIdentityAccessor()->GetUnconsentedPrimaryAccountWhenAvailable(
-      base::BindOnce(&DriveFsAuth::AccountReady,
-                     weak_ptr_factory_.GetWeakPtr()));
-}
-
-void DriveFsAuth::AccountReady(const CoreAccountId& account_id,
-                               const std::string& gaia,
-                               const std::string& email,
-                               const identity::AccountState& state) {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  weak_ptr_factory_.InvalidateWeakPtrs();
-  timer_->Stop();
-  GetIdentityAccessor()->GetAccessToken(
-      account_id, {"https://www.googleapis.com/auth/drive"},
-      kIdentityConsumerId,
+  // Timer is cancelled when it is destroyed, so use base::Unretained().
+  timer_->Start(
+      FROM_HERE, base::TimeDelta::FromSeconds(30),
+      base::BindOnce(&DriveFsAuth::AuthTimeout, base::Unretained(this)));
+  std::set<std::string> scopes({"https://www.googleapis.com/auth/drive"});
+  access_token_fetcher_ =
+      std::make_unique<signin::PrimaryAccountAccessTokenFetcher>(
+          kIdentityConsumerId, identity_manager, scopes,
           base::BindOnce(&DriveFsAuth::GotChromeAccessToken,
-                     base::Unretained(this)));
+                         base::Unretained(this)),
+          signin::PrimaryAccountAccessTokenFetcher::Mode::kWaitUntilAvailable,
+          signin::ConsentLevel::kNotRequired);
 }
 
 void DriveFsAuth::GotChromeAccessToken(
-    const base::Optional<std::string>& access_token,
-    base::Time expiration_time,
-    const GoogleServiceAuthError& error) {
+    GoogleServiceAuthError error,
+    signin::AccessTokenInfo access_token_info) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (!access_token) {
+  timer_->Stop();
+  if (error.state() != GoogleServiceAuthError::NONE) {
     std::move(get_access_token_callback_)
         .Run(error.IsPersistentError()
                  ? mojom::AccessTokenStatus::kAuthError
@@ -84,9 +86,9 @@ void DriveFsAuth::GotChromeAccessToken(
              "");
     return;
   }
-  UpdateCachedToken(*access_token, expiration_time);
+  UpdateCachedToken(access_token_info.token, access_token_info.expiration_time);
   std::move(get_access_token_callback_)
-      .Run(mojom::AccessTokenStatus::kSuccess, *access_token);
+      .Run(mojom::AccessTokenStatus::kSuccess, access_token_info.token);
 }
 
 const std::string& DriveFsAuth::GetOrResetCachedToken(bool use_cached) {
@@ -104,18 +106,8 @@ void DriveFsAuth::UpdateCachedToken(const std::string& token,
 
 void DriveFsAuth::AuthTimeout() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  weak_ptr_factory_.InvalidateWeakPtrs();
   std::move(get_access_token_callback_)
       .Run(mojom::AccessTokenStatus::kAuthError, "");
 }
 
-identity::mojom::IdentityAccessor* DriveFsAuth::GetIdentityAccessor() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  if (!identity_accessor_) {
-    delegate_->BindIdentityAccessor(
-        identity_accessor_.BindNewPipeAndPassReceiver());
-  }
-  return identity_accessor_.get();
-}
-
 }  // namespace drivefs

chromeos/components/drivefs/drivefs_auth.h

@@ -11,20 +11,23 @@
 
 #include "base/component_export.h"
 #include "base/macros.h"
-#include "base/memory/weak_ptr.h"
 #include "base/time/clock.h"
 #include "base/timer/timer.h"
 #include "chromeos/components/drivefs/mojom/drivefs.mojom.h"
-#include "mojo/public/cpp/bindings/pending_receiver.h"
-#include "mojo/public/cpp/bindings/remote.h"
-#include "services/identity/public/mojom/identity_accessor.mojom.h"
 
 class AccountId;
+class GoogleServiceAuthError;
 
 namespace network {
 class SharedURLLoaderFactory;
 }  // namespace network
 
+namespace signin {
+struct AccessTokenInfo;
+class IdentityManager;
+class PrimaryAccountAccessTokenFetcher;
+}  // namespace signin
+
 namespace drivefs {
 
 class COMPONENT_EXPORT(DRIVEFS) DriveFsAuth {
@@ -36,8 +39,7 @@ class COMPONENT_EXPORT(DRIVEFS) DriveFsAuth {
 
     virtual scoped_refptr<network::SharedURLLoaderFactory>
     GetURLLoaderFactory() = 0;
-    virtual void BindIdentityAccessor(
-        mojo::PendingReceiver<identity::mojom::IdentityAccessor> receiver) = 0;
+    virtual signin::IdentityManager* GetIdentityManager() = 0;
     virtual const AccountId& GetAccountId() = 0;
     virtual std::string GetObfuscatedAccountId() = 0;
     virtual bool IsMetricsCollectionEnabled() = 0;
@@ -71,14 +73,8 @@ class COMPONENT_EXPORT(DRIVEFS) DriveFsAuth {
       mojom::DriveFsDelegate::GetAccessTokenCallback callback);
 
  private:
-  void AccountReady(const CoreAccountId& account_id,
-                    const std::string& gaia,
-                    const std::string& email,
-                    const identity::AccountState& state);
-
-  void GotChromeAccessToken(const base::Optional<std::string>& access_token,
-                            base::Time expiration_time,
-                            const GoogleServiceAuthError& error);
+  void GotChromeAccessToken(GoogleServiceAuthError error,
+                            signin::AccessTokenInfo access_token_info);
 
   const std::string& GetOrResetCachedToken(bool use_cached);
 
@@ -86,16 +82,14 @@ class COMPONENT_EXPORT(DRIVEFS) DriveFsAuth {
 
   void AuthTimeout();
 
-  identity::mojom::IdentityAccessor* GetIdentityAccessor();
-
   SEQUENCE_CHECKER(sequence_checker_);
   const base::Clock* const clock_;
   const base::FilePath profile_path_;
   const std::unique_ptr<base::OneShotTimer> timer_;
   Delegate* const delegate_;
 
-  // The connection to the identity service. Access via |GetIdentityAccessor()|.
-  mojo::Remote<identity::mojom::IdentityAccessor> identity_accessor_;
+  std::unique_ptr<signin::PrimaryAccountAccessTokenFetcher>
+      access_token_fetcher_;
 
   // Pending callback for an in-flight GetAccessToken request.
   mojom::DriveFsDelegate::GetAccessTokenCallback get_access_token_callback_;
@@ -103,7 +97,6 @@ class COMPONENT_EXPORT(DRIVEFS) DriveFsAuth {
   std::string last_token_;
   base::Time last_token_expiry_;
 
-  base::WeakPtrFactory<DriveFsAuth> weak_ptr_factory_{this};
   DISALLOW_COPY_AND_ASSIGN(DriveFsAuth);
 };
 

chromeos/components/drivefs/fake_drivefs.cc

@@ -27,6 +27,7 @@
 #include "mojo/public/cpp/bindings/remote.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
 #include "net/base/mime_util.h"
+#include "url/gurl.h"
 
 namespace drivefs {
 namespace {