[Signin] Fix forever waiting in signin-internals ui

Currently requests may get deleted before signin-internals page gets
notified of the response through identity manager. This cl fixes the
issue.

Change-Id: Ic75594c60f52fc0ee7fe3470627a83dfa672b52e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6286426
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Tanmoy Mollik <triploblastic@google.com>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1427036}

google_apis/gaia/oauth2_access_token_manager.cc

@@ -736,18 +736,23 @@ void OAuth2AccessTokenManager::ProcessOnFetchComplete(
     error = std::move(response).error();
   }
 
+  // Save consumer ids before as delegate callback may make consumers delete
+  // their requests.
+  std::vector<std::string> consumer_ids;
+  for (const base::WeakPtr<RequestImpl>& request : waiting_requests) {
+    if (request) {
+      consumer_ids.push_back(request->GetConsumerId());
+    }
+  }
   delegate_->OnAccessTokenFetched(request_parameters.account_id, error);
 
   const OAuth2AccessTokenConsumer::TokenResponse* entry =
       GetCachedTokenResponse(request_parameters);
-  for (const base::WeakPtr<RequestImpl>& req : waiting_requests) {
-    if (req) {
-      for (auto& observer : diagnostics_observer_list_) {
-        observer.OnFetchAccessTokenComplete(
-            request_parameters.account_id, req->GetConsumerId(),
-            request_parameters.scopes, error,
-            entry ? entry->expiration_time : base::Time());
-      }
+  for (const std::string& consumer_id : consumer_ids) {
+    for (auto& observer : diagnostics_observer_list_) {
+      observer.OnFetchAccessTokenComplete(
+          request_parameters.account_id, consumer_id, request_parameters.scopes,
+          error, entry ? entry->expiration_time : base::Time());
     }
   }
 

google_apis/gaia/oauth2_access_token_manager_unittest.cc

@@ -599,6 +599,31 @@ TEST_F(OAuth2AccessTokenManagerTest,
   token_manager_->RemoveDiagnosticsObserver(&observer);
 }
 
+// Test that DiagnosticsObserver::OnFetchAccessTokenComplete is invoked when a
+// request is completed and then deleted by the delegate.
+TEST_F(OAuth2AccessTokenManagerTest,
+       OnFetchAccessTokenCompleteWhenRequestIsDeletedByDelegate) {
+  DiagnosticsObserverForTesting observer;
+  OAuth2AccessTokenManager::ScopeSet scopeset;
+  scopeset.insert("scope");
+  base::RunLoop run_loop;
+  GoogleServiceAuthError error(GoogleServiceAuthError::NONE);
+  observer.SetOnFetchAccessTokenComplete(account_id_, consumer_.id(), scopeset,
+                                         error, run_loop.QuitClosure());
+  token_manager_->AddDiagnosticsObserver(&observer);
+  SimulateOAuthTokenResponse(GetValidTokenResponse("token", 3600));
+
+  std::unique_ptr<OAuth2AccessTokenManager::Request> request(
+      token_manager_->StartRequest(account_id_, scopeset, &consumer_));
+  // Move request ownership to delegate_ lambda which will delete the object
+  // upon return.
+  delegate_.SetOnAccessTokenFetched(
+      account_id_, error,
+      base::BindLambdaForTesting([request = std::move(request)]() {}));
+  run_loop.Run();
+  token_manager_->RemoveDiagnosticsObserver(&observer);
+}
+
 // Test that DiagnosticsObserver::OnFetchAccessTokenComplete is invoked when
 // StartRequest is called for an account without a refresh token.
 TEST_F(OAuth2AccessTokenManagerTest,