IdentityTestEnv: Add API to wait for token requests for given account

The current IdentityTestEnvironment APIs to wait for access token
requests and issue responses operate in a shotgun manner: when the next
access token request is received, the test API implementation issues a
response for *all* pending access token requests. However, unittests
that is targeted in an upcoming conversion requires more flexibility:
as the production code that it is testing interacts with multiple
accounts, the test issues access token requests for multiple accounts,
and then sequentially issues responses and checks that a specific flow
for each account in question was initiated in response to the specific
response. Moreover, one of these unittests issues responses in an
order different to the order in which the requests were made.

To accommodate porting of these tests, this CL adds the ability to wait
for an access token request for a given account and then issue a
response only for that account. If while waiting for a request from a
specific account a request for a *different* account is seen, the
waiting code forwards on the handling of the request for that other
account to the next iteration of the runloop; this allows for the case
where a test wants to handle requests for multiple accounts in an
order different to the order in which those requests were made.

We leave in a variant that does not take in the account ID and operates
as before, as this is convenient for the common case of tests that don't
care about this level of detail (usually because they are testing a
production flow that operates on the primary account).

Bug: 798699, 809923
Change-Id: I6b1d93efd47b1eba128ac0b62c833736700e5196
Reviewed-on: https://chromium-review.googlesource.com/1149876
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578647}

services/identity/public/cpp/identity_test_environment.cc

@@ -186,18 +186,37 @@ void IdentityTestEnvironment::
     WaitForAccessTokenRequestIfNecessaryAndRespondWithToken(
         const std::string& token,
         const base::Time& expiration) {
-  WaitForAccessTokenRequestIfNecessary();
+  WaitForAccessTokenRequestIfNecessary(base::nullopt);
   internals_->token_service()->IssueTokenForAllPendingRequests(token,
                                                                expiration);
 }
 
+void IdentityTestEnvironment::
+    WaitForAccessTokenRequestIfNecessaryAndRespondWithToken(
+        const std::string& account_id,
+        const std::string& token,
+        const base::Time& expiration) {
+  WaitForAccessTokenRequestIfNecessary(account_id);
+  internals_->token_service()->IssueAllTokensForAccount(account_id, token,
+                                                        expiration);
+}
+
 void IdentityTestEnvironment::
     WaitForAccessTokenRequestIfNecessaryAndRespondWithError(
         const GoogleServiceAuthError& error) {
-  WaitForAccessTokenRequestIfNecessary();
+  WaitForAccessTokenRequestIfNecessary(base::nullopt);
   internals_->token_service()->IssueErrorForAllPendingRequests(error);
 }
 
+void IdentityTestEnvironment::
+    WaitForAccessTokenRequestIfNecessaryAndRespondWithError(
+        const std::string& account_id,
+        const GoogleServiceAuthError& error) {
+  WaitForAccessTokenRequestIfNecessary(account_id);
+  internals_->token_service()->IssueErrorForAllPendingRequestsForAccount(
+      account_id, error);
+}
+
 void IdentityTestEnvironment::SetCallbackForNextAccessTokenRequest(
     base::OnceClosure callback) {
   on_access_token_requested_callback_ = std::move(callback);
@@ -215,18 +234,41 @@ void IdentityTestEnvironment::OnAccessTokenRequested(
   base::SequencedTaskRunnerHandle::Get()->PostTask(
       FROM_HERE,
       base::BindOnce(&IdentityTestEnvironment::HandleOnAccessTokenRequested,
-                     base::Unretained(this)));
+                     base::Unretained(this), account_id));
 }
 
-void IdentityTestEnvironment::HandleOnAccessTokenRequested() {
+void IdentityTestEnvironment::HandleOnAccessTokenRequested(
+    std::string account_id) {
+  if (pending_access_token_requester_ &&
+      *pending_access_token_requester_ != account_id) {
+    // An access token request came in for a different account than the one for
+    // which we are waiting. Some unittests make access token requests for
+    // multiple accounts and interleave their responses in an order different
+    // from the requests. To accommodate this case, defer the handling of this
+    // access token request until the next iteration of the run loop, where it
+    // may then be being waited for.
+    base::SequencedTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE,
+        base::BindOnce(&IdentityTestEnvironment::HandleOnAccessTokenRequested,
+                       base::Unretained(this), account_id));
+    return;
+  }
+
+  pending_access_token_requester_.reset();
+
   if (on_access_token_requested_callback_)
     std::move(on_access_token_requested_callback_).Run();
 }
 
-void IdentityTestEnvironment::WaitForAccessTokenRequestIfNecessary() {
+void IdentityTestEnvironment::WaitForAccessTokenRequestIfNecessary(
+    base::Optional<std::string> pending_access_token_requester) {
+  DCHECK(!pending_access_token_requester_);
+  pending_access_token_requester_ = std::move(pending_access_token_requester);
+
   DCHECK(!on_access_token_requested_callback_);
   base::RunLoop run_loop;
   on_access_token_requested_callback_ = run_loop.QuitClosure();
+
   run_loop.Run();
 }
 

services/identity/public/cpp/identity_test_environment.h

@@ -5,6 +5,7 @@
 #ifndef SERVICES_IDENTITY_PUBLIC_CPP_IDENTITY_TEST_ENVIRONMENT_H_
 #define SERVICES_IDENTITY_PUBLIC_CPP_IDENTITY_TEST_ENVIRONMENT_H_
 
+#include "base/optional.h"
 #include "services/identity/public/cpp/identity_manager.h"
 #include "services/identity/public/cpp/identity_test_utils.h"
 
@@ -83,33 +84,72 @@ class IdentityTestEnvironment : public IdentityManager::DiagnosticsObserver {
   // an access token value of "access_token".
   void SetAutomaticIssueOfAccessTokens(bool grant);
 
-  // Issues |token| in response to an access token request that either has (a)
+  // Issues |token| in response to any access token request that either has (a)
   // just occurred in the current iteration of the run loop, or (b) will occur
   // in the future via a task that was posted in the current iteration of the
   // run loop. In the latter case, waits until the access token request occurs.
   // NOTE: This method behaves this way to allow IdentityTestEnvironment to be
   // agnostic with respect to whether access token requests are handled
   // synchronously or asynchronously in the production code.
-  // NOTE: The implementation currently issues tokens in response to *all*
-  // pending access token requests. If you need finer granularity, contact
-  // blundell@chromium.org
+  // NOTE: This version is suitable for use in the common context where access
+  // token requests are only being made for one account. If you need to
+  // disambiguate requests coming for different accounts, see the version below.
   void WaitForAccessTokenRequestIfNecessaryAndRespondWithToken(
       const std::string& token,
       const base::Time& expiration);
 
-  // Issues |error| in response to an access token request that either has (a)
+  // Issues |token| in response to an access token request for |account_id| that
+  // either has (a) just occurred in the current iteration of the run loop, or
+  // (b) will occur in the future via a task that was posted in the current
+  // iteration of the run loop. In the latter case, waits until the access token
+  // request occurs.
+  // NOTE: Any access token request for a *different* account
+  // that is seen before an access token request for the given account will
+  // be deferred to be handled on the next iteration of the runloop; this
+  // behavior accommodates tests that interleave handling of production access
+  // token requests for multiple accounts in an order different from the order
+  // in which the requests are made.
+  // NOTE: This method behaves this way to allow
+  // IdentityTestEnvironment to be agnostic with respect to whether access token
+  // requests are handled synchronously or asynchronously in the production
+  // code.
+  void WaitForAccessTokenRequestIfNecessaryAndRespondWithToken(
+      const std::string& account_id,
+      const std::string& token,
+      const base::Time& expiration);
+
+  // Issues |error| in response to any access token request that either has (a)
   // just occurred in the current iteration of the run loop, or (b) will occur
   // in the future via a task that was posted in the current iteration of the
   // run loop. In the latter case, waits until the access token request occurs.
   // NOTE: This method behaves this way to allow IdentityTestEnvironment to be
   // agnostic with respect to whether access token requests are handled
   // synchronously or asynchronously in the production code.
-  // NOTE: The implementation currently issues errors in response to *all*
-  // pending access token requests. If you need finer granularity, contact
-  // blundell@chromium.org
+  // NOTE: This version is suitable for use in the common context where access
+  // token requests are only being made for one account. If you need to
+  // disambiguate requests coming for different accounts, see the version below.
   void WaitForAccessTokenRequestIfNecessaryAndRespondWithError(
       const GoogleServiceAuthError& error);
 
+  // Issues |error| in response to an access token request for |account_id| that
+  // either has (a) just occurred in the current iteration of the run loop, or
+  // (b) will occur in the future via a task that was posted in the current
+  // iteration of the run loop. In the latter case, waits until the access token
+  // request occurs.
+  // NOTE: Any access token request for a *different* account
+  // that is seen before an access token request for the given account will
+  // be deferred to be handled on the next iteration of the runloop; this
+  // behavior accommodates tests that interleave handling of production access
+  // token requests for multiple accounts in an order different from the order
+  // in which the requests are made.
+  // NOTE: This method behaves this way to allow
+  // IdentityTestEnvironment to be agnostic with respect to whether access token
+  // requests are handled synchronously or asynchronously in the production
+  // code.
+  void WaitForAccessTokenRequestIfNecessaryAndRespondWithError(
+      const std::string& account_id,
+      const GoogleServiceAuthError& error);
+
   // Sets a callback that will be invoked on the next incoming access token
   // request. Note that this can not be combined with the
   // WaitForAccessTokenRequestIfNecessaryAndRespondWith* methods - you must
@@ -124,15 +164,21 @@ class IdentityTestEnvironment : public IdentityManager::DiagnosticsObserver {
       const std::string& consumer_id,
       const OAuth2TokenService::ScopeSet& scopes) override;
 
-  // Handles the notification that an access token request was received by
-  // invoking |on_access_token_request_callback_| if the latter is non-null.
-  void HandleOnAccessTokenRequested();
+  // Handles the notification that an access token request was received for
+  // |account_id|. Invokes |on_access_token_request_callback_| if the latter
+  // is non-null *and* either |*pending_access_token_requester_| equals
+  // |account_id| or |pending_access_token_requester_| is empty.
+  void HandleOnAccessTokenRequested(std::string account_id);
 
-  // Runs a nested runloop until an access token request is observed.
-  void WaitForAccessTokenRequestIfNecessary();
+  // Sets |pending_access_token_requester_| to
+  // |pending_access_token_requester| and runs a nested runloop until an access
+  // token request is observed.
+  void WaitForAccessTokenRequestIfNecessary(
+      base::Optional<std::string> pending_access_token_requester);
 
   std::unique_ptr<IdentityTestEnvironmentInternal> internals_;
   base::OnceClosure on_access_token_requested_callback_;
+  base::Optional<std::string> pending_access_token_requester_;
 
   DISALLOW_COPY_AND_ASSIGN(IdentityTestEnvironment);
 };