Make IdentityAccessorImpl scoped to IdentityService

IdentityAccessorImpl uses and observes identity manager, which is scoped
to profile, so it should not outlive the profile it's assigned to
(before this IdentityAccessorImpl instances were leaked, never removed
themselves from IdentityManager observer list, and were causing DCHECK
failures during IdentityManager's observer_list_)

BUG=None

Change-Id: I6d7966d0d2a2db859c08e9a938a3cf73302ee37a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1573169Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#652543}

services/identity/identity_accessor_impl.cc

@@ -30,25 +30,13 @@ void IdentityAccessorImpl::OnTokenRequestCompleted(
   access_token_fetchers_.erase(callback_id);
 }
 
-// static
-void IdentityAccessorImpl::Create(mojom::IdentityAccessorRequest request,
-                                  IdentityManager* identity_manager) {
-  new IdentityAccessorImpl(std::move(request), identity_manager);
-}
-
-IdentityAccessorImpl::IdentityAccessorImpl(
-    mojom::IdentityAccessorRequest request,
-    IdentityManager* identity_manager)
-    : binding_(this, std::move(request)), identity_manager_(identity_manager) {
-  binding_.set_connection_error_handler(base::BindRepeating(
-      &IdentityAccessorImpl::OnConnectionError, base::Unretained(this)));
-
+IdentityAccessorImpl::IdentityAccessorImpl(IdentityManager* identity_manager)
+    : identity_manager_(identity_manager) {
   identity_manager_->AddObserver(this);
 }
 
 IdentityAccessorImpl::~IdentityAccessorImpl() {
   identity_manager_->RemoveObserver(this);
-  binding_.Close();
 }
 
 void IdentityAccessorImpl::GetPrimaryAccountInfo(
@@ -148,8 +136,4 @@ AccountState IdentityAccessorImpl::GetStateOfAccount(
   return account_state;
 }
 
-void IdentityAccessorImpl::OnConnectionError() {
-  delete this;
-}
-
 }  // namespace identity

services/identity/identity_accessor_impl.h

@@ -11,7 +11,6 @@
 #include "base/callback_list.h"
 #include "components/signin/core/browser/account_info.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
-#include "mojo/public/cpp/bindings/binding.h"
 #include "services/identity/public/cpp/account_state.h"
 #include "services/identity/public/cpp/identity_manager.h"
 #include "services/identity/public/cpp/scope_set.h"
@@ -22,11 +21,7 @@ namespace identity {
 class IdentityAccessorImpl : public mojom::IdentityAccessor,
                              public IdentityManager::Observer {
  public:
-  static void Create(mojom::IdentityAccessorRequest request,
-                     IdentityManager* identity_manager);
-
-  IdentityAccessorImpl(mojom::IdentityAccessorRequest request,
-                       IdentityManager* identity_manager);
+  explicit IdentityAccessorImpl(IdentityManager* identity_manager);
   ~IdentityAccessorImpl() override;
 
  private:
@@ -64,11 +59,6 @@ class IdentityAccessorImpl : public mojom::IdentityAccessor,
   // Gets the current state of the account represented by |account_info|.
   AccountState GetStateOfAccount(const CoreAccountInfo& account_info);
 
-  // Called when |binding_| hits a connection error. Destroys this instance,
-  // since it's no longer needed.
-  void OnConnectionError();
-
-  mojo::Binding<mojom::IdentityAccessor> binding_;
   IdentityManager* identity_manager_;
 
   // The set of pending requests for access tokens.

services/identity/identity_service.cc

@@ -36,6 +36,7 @@ void IdentityService::ShutDown() {
     return;
 
   identity_manager_ = nullptr;
+  identity_accessor_bindings_.CloseAllBindings();
 }
 
 bool IdentityService::IsShutDown() {
@@ -47,7 +48,9 @@ void IdentityService::Create(mojom::IdentityAccessorRequest request) {
   if (IsShutDown())
     return;
 
-  IdentityAccessorImpl::Create(std::move(request), identity_manager_);
+  identity_accessor_bindings_.AddBinding(
+      std::make_unique<IdentityAccessorImpl>(identity_manager_),
+      std::move(request));
 }
 
 }  // namespace identity

services/identity/identity_service.h

@@ -6,6 +6,7 @@
 #define SERVICES_IDENTITY_IDENTITY_SERVICE_H_
 
 #include "components/signin/core/browser/signin_manager_base.h"
+#include "mojo/public/cpp/bindings/strong_binding_set.h"
 #include "services/identity/public/cpp/identity_manager.h"
 #include "services/identity/public/mojom/identity_accessor.mojom.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
@@ -13,6 +14,10 @@
 #include "services/service_manager/public/cpp/service_binding.h"
 #include "services/service_manager/public/mojom/service.mojom.h"
 
+namespace mojom {
+class IdentityAccessor;
+}
+
 namespace identity {
 
 class IdentityService : public service_manager::Service {
@@ -39,6 +44,8 @@ class IdentityService : public service_manager::Service {
 
   IdentityManager* identity_manager_;
 
+  mojo::StrongBindingSet<mojom::IdentityAccessor> identity_accessor_bindings_;
+
   service_manager::BinderRegistry registry_;
 
   DISALLOW_COPY_AND_ASSIGN(IdentityService);