Fix bug where TokenServiceProvider is used after it has been destroyed.

There was a race condition where GetTokenService could be called on an
already destroyed TokenServiceProvider (see linked bug for details).
Update Core to ensure TokenServiceProvider is not destroyed out from
under the token service task runner thread.

BUG=401119

Review URL: https://codereview.chromium.org/458753006

Cr-Commit-Position: refs/heads/master@{#289222}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289222 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 49 insertions, 24 deletions

diff --git a/google_apis/gaia/oauth2_token_service_request.cc b/google_apis/gaia/oauth2_token_service_request.cc
index 6720152..b652c2b 100644
--- a/google_apis/gaia/oauth2_token_service_request.cc
+++ b/google_apis/gaia/oauth2_token_service_request.cc
@@ -40,7 +40,8 @@ class OAuth2TokenServiceRequest::Core
  public:
   // Note the thread where an instance of Core is constructed is referred to as
   // the "owner thread" here.
-  Core(OAuth2TokenServiceRequest* owner, TokenServiceProvider* provider);
+  Core(OAuth2TokenServiceRequest* owner,
+       const scoped_refptr<TokenServiceProvider>& provider);
 
   // Starts the core.  Must be called on the owner thread.
   void Start();
@@ -75,12 +76,17 @@ class OAuth2TokenServiceRequest::Core
 
   scoped_refptr<base::SingleThreadTaskRunner> token_service_task_runner_;
   OAuth2TokenServiceRequest* owner_;
-  TokenServiceProvider* provider_;
+
+  // It is important that provider_ is destroyed on the owner thread, not the
+  // token_service_task_runner_ thread.
+  scoped_refptr<TokenServiceProvider> provider_;
+
   DISALLOW_COPY_AND_ASSIGN(Core);
 };
 
-OAuth2TokenServiceRequest::Core::Core(OAuth2TokenServiceRequest* owner,
-                                      TokenServiceProvider* provider)
+OAuth2TokenServiceRequest::Core::Core(
+    OAuth2TokenServiceRequest* owner,
+    const scoped_refptr<TokenServiceProvider>& provider)
     : owner_(owner), provider_(provider) {
   DCHECK(owner_);
   DCHECK(provider_);
@@ -149,7 +155,8 @@ class RequestCore : public OAuth2TokenServiceRequest::Core,
                     public OAuth2TokenService::Consumer {
  public:
   RequestCore(OAuth2TokenServiceRequest* owner,
-              OAuth2TokenServiceRequest::TokenServiceProvider* provider,
+              const scoped_refptr<
+                  OAuth2TokenServiceRequest::TokenServiceProvider>& provider,
               OAuth2TokenService::Consumer* consumer,
               const std::string& account_id,
               const OAuth2TokenService::ScopeSet& scopes);
@@ -189,7 +196,8 @@ class RequestCore : public OAuth2TokenServiceRequest::Core,
 
 RequestCore::RequestCore(
     OAuth2TokenServiceRequest* owner,
-    OAuth2TokenServiceRequest::TokenServiceProvider* provider,
+    const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
+        provider,
     OAuth2TokenService::Consumer* consumer,
     const std::string& account_id,
     const OAuth2TokenService::ScopeSet& scopes)
@@ -260,7 +268,8 @@ void RequestCore::InformOwnerOnGetTokenFailure(GoogleServiceAuthError error) {
 class InvalidateCore : public OAuth2TokenServiceRequest::Core {
  public:
   InvalidateCore(OAuth2TokenServiceRequest* owner,
-                 OAuth2TokenServiceRequest::TokenServiceProvider* provider,
+                 const scoped_refptr<
+                     OAuth2TokenServiceRequest::TokenServiceProvider>& provider,
                  const std::string& access_token,
                  const std::string& account_id,
                  const OAuth2TokenService::ScopeSet& scopes);
@@ -284,7 +293,8 @@ class InvalidateCore : public OAuth2TokenServiceRequest::Core {
 
 InvalidateCore::InvalidateCore(
     OAuth2TokenServiceRequest* owner,
-    OAuth2TokenServiceRequest::TokenServiceProvider* provider,
+    const scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider>&
+        provider,
     const std::string& access_token,
     const std::string& account_id,
     const OAuth2TokenService::ScopeSet& scopes)
@@ -314,7 +324,7 @@ void InvalidateCore::StopOnTokenServiceThread() {
 
 // static
 scoped_ptr<OAuth2TokenServiceRequest> OAuth2TokenServiceRequest::CreateAndStart(
-    TokenServiceProvider* provider,
+    const scoped_refptr<TokenServiceProvider>& provider,
     const std::string& account_id,
     const OAuth2TokenService::ScopeSet& scopes,
     OAuth2TokenService::Consumer* consumer) {
@@ -328,7 +338,7 @@ scoped_ptr<OAuth2TokenServiceRequest> OAuth2TokenServiceRequest::CreateAndStart(
 
 // static
 void OAuth2TokenServiceRequest::InvalidateToken(
-    OAuth2TokenServiceRequest::TokenServiceProvider* provider,
+    const scoped_refptr<TokenServiceProvider>& provider,
     const std::string& account_id,
     const OAuth2TokenService::ScopeSet& scopes,
     const std::string& access_token) {
diff --git a/google_apis/gaia/oauth2_token_service_request.h b/google_apis/gaia/oauth2_token_service_request.h
index 972e518..b1f0e57 100644
--- a/google_apis/gaia/oauth2_token_service_request.h
+++ b/google_apis/gaia/oauth2_token_service_request.h
@@ -8,6 +8,7 @@
 #include <set>
 #include <string>
 
+#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/non_thread_safe.h"
@@ -23,10 +24,16 @@ class OAuth2TokenServiceRequest : public OAuth2TokenService::Request,
   class Core;
 
   // Interface for providing an OAuth2TokenService.
-  class TokenServiceProvider {
+  //
+  // Ref-counted so that OAuth2TokenServiceRequest can ensure this object isn't
+  // destroyed out from under the token service task runner thread.  Because
+  // OAuth2TokenServiceRequest has a reference, implementations of
+  // TokenServiceProvider must be capable of being destroyed on the same thread
+  // on which the OAuth2TokenServiceRequest was created.
+  class TokenServiceProvider
+      : public base::RefCountedThreadSafe<TokenServiceProvider> {
    public:
     TokenServiceProvider();
-    virtual ~TokenServiceProvider();
 
     // Returns the task runner on which the token service lives.
     //
@@ -42,12 +49,15 @@ class OAuth2TokenServiceRequest : public OAuth2TokenService::Request,
     // This method may only be called from the task runner returned by
     // |GetTokenServiceTaskRunner|.
     virtual OAuth2TokenService* GetTokenService() = 0;
+
+   protected:
+    friend class base::RefCountedThreadSafe<TokenServiceProvider>;
+    virtual ~TokenServiceProvider();
   };
 
   // Creates and starts an access token request for |account_id| and |scopes|.
   //
-  // |provider| is used to get the OAuth2TokenService and must outlive the
-  // returned request object.
+  // |provider| is used to get the OAuth2TokenService.
   //
   // |account_id| must not be empty.
   //
@@ -59,23 +69,23 @@ class OAuth2TokenServiceRequest : public OAuth2TokenService::Request,
   // network activities may not be canceled and the cache in OAuth2TokenService
   // may be populated with the fetched results.
   static scoped_ptr<OAuth2TokenServiceRequest> CreateAndStart(
-      TokenServiceProvider* provider,
+      const scoped_refptr<TokenServiceProvider>& provider,
       const std::string& account_id,
       const OAuth2TokenService::ScopeSet& scopes,
       OAuth2TokenService::Consumer* consumer);
 
   // Invalidates |access_token| for |account_id| and |scopes|.
   //
-  // |provider| is used to get the OAuth2TokenService and must outlive the
-  // returned request object.
+  // |provider| is used to get the OAuth2TokenService.
   //
   // |account_id| must not be empty.
   //
   // |scopes| must not be empty.
-  static void InvalidateToken(TokenServiceProvider* provider,
-                              const std::string& account_id,
-                              const OAuth2TokenService::ScopeSet& scopes,
-                              const std::string& access_token);
+  static void InvalidateToken(
+      const scoped_refptr<TokenServiceProvider>& provider,
+      const std::string& account_id,
+      const OAuth2TokenService::ScopeSet& scopes,
+      const std::string& access_token);
 
   virtual ~OAuth2TokenServiceRequest();
 
diff --git a/google_apis/gaia/oauth2_token_service_request_unittest.cc b/google_apis/gaia/oauth2_token_service_request_unittest.cc
index 0a86cfd..c2683a8 100644
--- a/google_apis/gaia/oauth2_token_service_request_unittest.cc
+++ b/google_apis/gaia/oauth2_token_service_request_unittest.cc
@@ -159,6 +159,8 @@ class OAuth2TokenServiceRequestTest : public testing::Test {
     virtual OAuth2TokenService* GetTokenService() OVERRIDE;
 
    private:
+    virtual ~Provider();
+
     scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
     OAuth2TokenService* token_service_;
   };
@@ -166,7 +168,7 @@ class OAuth2TokenServiceRequestTest : public testing::Test {
   base::MessageLoop ui_loop_;
   OAuth2TokenService::ScopeSet scopes_;
   scoped_ptr<MockOAuth2TokenService> oauth2_service_;
-  scoped_ptr<OAuth2TokenServiceRequest::TokenServiceProvider> provider_;
+  scoped_refptr<OAuth2TokenServiceRequest::TokenServiceProvider> provider_;
   TestingOAuth2TokenServiceConsumer consumer_;
 };
 
@@ -174,8 +176,8 @@ void OAuth2TokenServiceRequestTest::SetUp() {
   scopes_.insert(kScope);
   oauth2_service_.reset(new MockOAuth2TokenService);
   oauth2_service_->AddAccount(kAccountId);
-  provider_.reset(
-      new Provider(base::MessageLoopProxy::current(), oauth2_service_.get()));
+  provider_ =
+      new Provider(base::MessageLoopProxy::current(), oauth2_service_.get());
 }
 
 void OAuth2TokenServiceRequestTest::TearDown() {
@@ -198,6 +200,9 @@ OAuth2TokenService* OAuth2TokenServiceRequestTest::Provider::GetTokenService() {
   return token_service_;
 }
 
+OAuth2TokenServiceRequestTest::Provider::~Provider() {
+}
+
 TEST_F(OAuth2TokenServiceRequestTest, CreateAndStart_Failure) {
   oauth2_service_->SetResponse(
       GoogleServiceAuthError(GoogleServiceAuthError::SERVICE_UNAVAILABLE),