[iOS][Signin] Fixing crash related to the token cache.

When a token is requested, HasCacheEntry() is called in
StartCacheLookupRequest(). This call can potentially remove the cache
entry. Therefore when getting the cache entry right after, it would
be not available anymore.
HasCacheEntry() method should never exist, since there is no warranty
that the same cache entry would be returned by GetCacheEntry() on the
next line of code.

To solve the problem, StartCacheLookupRequest() is renamed to
InformConsumerWithCacheEntry(), and only GetCacheEntry() is used.

Bug: 791695
Change-Id: Ie22c520cb0f2e2feec84781908b33ba2d0e791e5
Reviewed-on: https://chromium-review.googlesource.com/817200Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Commit-Queue: Jérôme Lebel <jlebel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523399}

google_apis/gaia/oauth2_token_service.cc

@@ -495,8 +495,10 @@ OAuth2TokenService::StartRequestForClientWithContext(
   RequestParameters request_parameters(client_id,
                                        account_id,
                                        scopes);
-  if (HasCacheEntry(request_parameters)) {
-    StartCacheLookupRequest(request.get(), request_parameters, consumer);
+  const CacheEntry* cache_entry = GetCacheEntry(request_parameters);
+  if (cache_entry && cache_entry->access_token.length()) {
+    InformConsumerWithCacheEntry(cache_entry, request.get(),
+                                 request_parameters);
   } else {
     FetchOAuth2Token(request.get(),
                      account_id,
@@ -543,15 +545,14 @@ OAuth2AccessTokenFetcher* OAuth2TokenService::CreateAccessTokenFetcher(
   return delegate_->CreateAccessTokenFetcher(account_id, getter, consumer);
 }
 
-void OAuth2TokenService::StartCacheLookupRequest(
+void OAuth2TokenService::InformConsumerWithCacheEntry(
+    const CacheEntry* cache_entry,
     RequestImpl* request,
-    const OAuth2TokenService::RequestParameters& request_parameters,
-    OAuth2TokenService::Consumer* consumer) {
-  CHECK(HasCacheEntry(request_parameters));
-  const CacheEntry* cache_entry = GetCacheEntry(request_parameters);
+    const RequestParameters& request_parameters) {
+  DCHECK(cache_entry && cache_entry->access_token.length());
   for (auto& observer : diagnostics_observer_list_) {
     observer.OnFetchAccessTokenComplete(
-        request_parameters.account_id, consumer->id(),
+        request_parameters.account_id, request->GetConsumerId(),
         request_parameters.scopes, GoogleServiceAuthError::AuthErrorNone(),
         cache_entry->expiration_date);
   }
@@ -670,12 +671,6 @@ void OAuth2TokenService::OnFetchComplete(Fetcher* fetcher) {
   pending_fetchers_.erase(iter);
 }
 
-bool OAuth2TokenService::HasCacheEntry(
-    const RequestParameters& request_parameters) {
-  const CacheEntry* cache_entry = GetCacheEntry(request_parameters);
-  return cache_entry && cache_entry->access_token.length();
-}
-
 const OAuth2TokenService::CacheEntry* OAuth2TokenService::GetCacheEntry(
     const RequestParameters& request_parameters) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

google_apis/gaia/oauth2_token_service.h

@@ -342,15 +342,10 @@ class OAuth2TokenService {
       const ScopeSet& scopes,
       Consumer* consumer);
 
-  // Returns true if GetCacheEntry would return a valid cache entry for the
-  // given scopes.
-  bool HasCacheEntry(const RequestParameters& client_scopes);
-
-  // Posts a task to fire the Consumer callback with the cached token.  Must
-  // Must only be called if HasCacheEntry() returns true.
-  void StartCacheLookupRequest(RequestImpl* request,
-                               const RequestParameters& client_scopes,
-                               Consumer* consumer);
+  // Posts a task to fire the Consumer callback with the cached token.
+  void InformConsumerWithCacheEntry(const CacheEntry* cache_entry,
+                                    RequestImpl* request,
+                                    const RequestParameters& client_scopes);
 
   // Returns a currently valid OAuth2 access token for the given set of scopes,
   // or NULL if none have been cached. Note the user of this method should