Enable IdentityTestEnvironment to take in IdentityManager's dependencies

Currently, IdentityTestEnvironment internally constructs all the
dependencies of IdentityManager as well as the IdentityManager object
itself. This behavior is nice for hiding the fact that IdentityManager
has these dependencies from consumers, but doesn't allow for
usage of IdentityTestEnviroment in incremental conversion of tests that
have broad direct usage of these dependencies. Concretely,
profile_sync_test_util.h provides one such problematic case: it
constructs and directly exposes these dependencies, which are then
broadly used by a set of tests that consume this utility class.
Converting all these tests in one go is not practical.

This CL adds an alternate IdentityTestEnvironment constructor that
takes in the dependencies and constructs IdentityManager via those
passed-in dependencies. To enable this constructor, the CL also
encapsulates the fact that IdentityTestEnvironment otherwise obtains
these dependencies from an internal object within the current
IdentityTestEnvironment constructor.  It also moves ownership of
IdentityManager from that internal object to IdentityTestEnvironment
itself so that it can be created/owned independent of that internal
object.

A followup CL will use this new constructor.

Bug: 886599
Change-Id: I4c6a503dcc11c327193f80bc7d7c48e467c6919b
Reviewed-on: https://chromium-review.googlesource.com/c/1273298
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599178}

services/identity/public/cpp/identity_test_environment.h

@@ -6,21 +6,52 @@
 #define SERVICES_IDENTITY_PUBLIC_CPP_IDENTITY_TEST_ENVIRONMENT_H_
 
 #include "base/optional.h"
+#include "components/signin/core/browser/account_tracker_service.h"
+#include "components/signin/core/browser/fake_gaia_cookie_manager_service.h"
+#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
+#include "components/signin/core/browser/fake_signin_manager.h"
 #include "services/identity/public/cpp/identity_manager.h"
 #include "services/identity/public/cpp/identity_test_utils.h"
 
 namespace identity {
 
-class IdentityTestEnvironmentInternal;
+// Internal class that creates and owns dependencies of IdentityManager
+// when those dependencies are not passed in externally.
+class IdentityManagerDependenciesOwner;
 
 // Class that creates an IdentityManager for use in testing contexts and
 // provides facilities for driving that IdentityManager. The IdentityManager
 // instance is brought up in an environment where the primary account is
 // not available; call MakePrimaryAccountAvailable() as needed.
 class IdentityTestEnvironment : public IdentityManager::DiagnosticsObserver {
+#if defined(OS_CHROMEOS)
+  using SigninManagerForTest = FakeSigninManagerBase;
+#else
+  using SigninManagerForTest = FakeSigninManager;
+#endif  // OS_CHROMEOS
+
  public:
+  // Preferred constructor: constructs an IdentityManager object and its
+  // dependencies internally. Cannot be used if the client of this class
+  // is still interacting directly with those dependencies (e.g., if
+  // IdentityTestEnvironment is being introduced to incrementally convert
+  // a test). In that case, use the below constructor and switch to this
+  // constructor once the conversion is complete.
   IdentityTestEnvironment(
       bool use_fake_url_loader_for_gaia_cookie_manager = false);
+
+  // Constructor that takes in instances of the dependencies of
+  // IdentityManager and constructs an IdentityManager instance from those
+  // dependencies. For use in contexts where those dependencies are still
+  // being used directly by the creator of this object (i.e., while a test is
+  // being incrementally converted). Prefer the above constructor, and switch to
+  // that constructor once possible (e.g., when an incremental conversion is
+  // completed). NOTE: The passed-in objects must all outlive this object.
+  IdentityTestEnvironment(
+      AccountTrackerService* account_tracker_service,
+      FakeProfileOAuth2TokenService* token_service,
+      SigninManagerForTest* signin_manager,
+      FakeGaiaCookieManagerService* gaia_cookie_manager_service);
   ~IdentityTestEnvironment() override;
 
   // The IdentityManager instance created and owned by this instance.
@@ -180,6 +211,17 @@ class IdentityTestEnvironment : public IdentityManager::DiagnosticsObserver {
     base::OnceClosure on_available;
   };
 
+  // Constructs this object and its IdentityManager instance from the supplied
+  // dependencies of IdentityManager, which must be either:
+  // (1) non-null instances of the backing classes, OR
+  // (2) a non-null instance of |dependencies_owner|.
+  IdentityTestEnvironment(
+      AccountTrackerService* account_tracker_service,
+      FakeProfileOAuth2TokenService* token_service,
+      SigninManagerForTest* signin_manager,
+      FakeGaiaCookieManagerService* gaia_cookie_manager_service,
+      std::unique_ptr<IdentityManagerDependenciesOwner> dependencies_owner);
+
   // IdentityManager::DiagnosticsObserver:
   void OnAccessTokenRequested(
       const std::string& account_id,
@@ -199,7 +241,16 @@ class IdentityTestEnvironment : public IdentityManager::DiagnosticsObserver {
   void WaitForAccessTokenRequestIfNecessary(
       base::Optional<std::string> account_id);
 
-  std::unique_ptr<IdentityTestEnvironmentInternal> internals_;
+  // NOTE: This object must be first in the list, as it owns the objects
+  // pointed to below in the case where those objects are not passed in via
+  // the IdentityTestEnvironment constructor.
+  std::unique_ptr<IdentityManagerDependenciesOwner> dependencies_owner_;
+  AccountTrackerService* account_tracker_service_ = nullptr;
+  FakeProfileOAuth2TokenService* token_service_ = nullptr;
+  SigninManagerForTest* signin_manager_ = nullptr;
+  FakeGaiaCookieManagerService* gaia_cookie_manager_service_ = nullptr;
+  std::unique_ptr<IdentityManager> identity_manager_;
+
   base::OnceClosure on_access_token_requested_callback_;
   std::vector<AccessTokenRequestState> requesters_;