Refactor ProfileOAuth2TokenServiceRequest into OAuth2TokenServiceRequest

google_apis/gaia/oauth2_token_service_request.cc

-// Copyright 2012 The Chromium Authors. All rights reserved.
+// Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/signin/profile_oauth2_token_service_request.h"
+#include "google_apis/gaia/oauth2_token_service_request.h"
 
 #include "base/bind.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
-#include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
-#include "chrome/browser/signin/signin_manager_factory.h"
-#include "components/signin/core/browser/profile_oauth2_token_service.h"
-#include "components/signin/core/browser/signin_manager.h"
-#include "content/public/browser/browser_thread.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 #include "google_apis/gaia/oauth2_access_token_consumer.h"
 
-class ProfileOAuth2TokenServiceRequest::Core
-    : public base::RefCountedThreadSafe<ProfileOAuth2TokenServiceRequest::Core>,
-      public OAuth2TokenService::Consumer {
+OAuth2TokenServiceRequest::TokenServiceProvider::TokenServiceProvider() {
+}
+
+OAuth2TokenServiceRequest::TokenServiceProvider::~TokenServiceProvider() {
+}
+
+// Core serves as the base class for OAuth2TokenService operations.  Each
+// operation should be modeled as a derived type.
+//
+// Core is used like this:
+//
+// 1. Constructed on owner thread.
+//
+// 2. Start() is called on owner thread, which calls StartOnTokenServiceThread()
+// on token service thread.
+//
+// 3. ...

[msarda, 2014/05/30 14:37:31]

Maybe:
s/.../Request is executed.

[maniscalco, 2014/05/30 17:48:08]

Done.

+//
+// 4. Stop() is called on owner thread, which calls StopOnTokenServiceThread()
+// on token service thread.
+//
+// 5. Core is destroyed on owner thread.
+class OAuth2TokenServiceRequest::Core
+    : public base::RefCountedThreadSafe<OAuth2TokenServiceRequest::Core> {
  public:
   // Note the thread where an instance of Core is constructed is referred to as
   // the "owner thread" here. This will be the thread of |owner_task_runner_|.
-  Core(Profile* profile,
-       ProfileOAuth2TokenServiceRequest* owner);
-  // Starts fetching an OAuth2 access token for |account_id| and |scopes|. It
-  // should be called on the owner thread.
-  void Start(
+  Core(const scoped_refptr<base::SingleThreadTaskRunner>& owner_task_runner,
+       OAuth2TokenServiceRequest* owner,
+       TokenServiceProvider* provider);
+
+  // Starts the core.  Must be called on the owner thread.
+  void Start();
+
+  // Stops the core.  Must be called on the owner thread.
+  void Stop();
+
+ protected:
+  // Core must be destroyed on the owner thread.  If data members must be
+  // cleaned up or destroyed on the token service thread, do so in the
+  // StopOnTokenServiceThread method.
+  virtual ~Core();
+
+  // Called on the token service thread.
+  virtual void StartOnTokenServiceThread() = 0;
+
+  // Called on the token service thread.
+  virtual void StopOnTokenServiceThread() = 0;
+
+  base::SingleThreadTaskRunner* owner_task_runner();

[msarda, 2014/05/30 14:37:31]

It seems that owner_task_runner() is just used to check the current thread.
Could you just use  CalledOnValidThread() for that?

[maniscalco, 2014/05/30 17:48:08]

Good idea.  Done.  Core now inherits from NonThreadSafe and I've removed
owner_task_runner from all but RequestCore.

+  base::SingleThreadTaskRunner* token_service_task_runner();
+  OAuth2TokenService* token_service();
+  OAuth2TokenServiceRequest* owner();
+
+ private:
+  friend class base::RefCountedThreadSafe<OAuth2TokenServiceRequest::Core>;
+
+  scoped_refptr<base::SingleThreadTaskRunner> owner_task_runner_;
+  scoped_refptr<base::SingleThreadTaskRunner> token_service_task_runner_;
+  OAuth2TokenServiceRequest* owner_;
+  TokenServiceProvider* provider_;
+  DISALLOW_COPY_AND_ASSIGN(Core);
+};
+
+OAuth2TokenServiceRequest::Core::Core(
+    const scoped_refptr<base::SingleThreadTaskRunner>& owner_task_runner,

[Roger Tawa OOO till Jul 10th, 2014/05/30 15:39:00]

|owner_task_runner| does not seem to be used.  I think you might have wanted to
use it to initialize |owner_task_runner_|, but probably better to remove the arg
and initialize |owner_task_runner_| the way you do it at line 83 below.

[maniscalco, 2014/05/30 17:48:08]

Good catch.  Parameter removed.

+    OAuth2TokenServiceRequest* owner,
+    TokenServiceProvider* provider)
+    : owner_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      owner_(owner),
+      provider_(provider) {
+  DCHECK(owner_);
+  DCHECK(provider_);
+  token_service_task_runner_ = provider_->GetTokenServiceTaskRunner();
+  DCHECK(token_service_task_runner_);
+}
+
+OAuth2TokenServiceRequest::Core::~Core() {
+  DCHECK(owner_task_runner_->BelongsToCurrentThread());
+}
+
+void OAuth2TokenServiceRequest::Core::Start() {
+  DCHECK(owner_task_runner_->BelongsToCurrentThread());
+  token_service_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&OAuth2TokenServiceRequest::Core::StartOnTokenServiceThread,
+                 this));
+}
+
+void OAuth2TokenServiceRequest::Core::Stop() {
+  DCHECK(owner_task_runner_->BelongsToCurrentThread());
+  // Detaches |owner_| from this instance so |owner_| will be called back only
+  // if |Stop()| has never been called.
+  owner_ = NULL;
+  token_service_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&OAuth2TokenServiceRequest::Core::StopOnTokenServiceThread,
+                 this));
+}
+
+base::SingleThreadTaskRunner*
+OAuth2TokenServiceRequest::Core::owner_task_runner() {
+  return owner_task_runner_;
+}
+
+base::SingleThreadTaskRunner*
+OAuth2TokenServiceRequest::Core::token_service_task_runner() {
+  return token_service_task_runner_;
+}
+
+OAuth2TokenService* OAuth2TokenServiceRequest::Core::token_service() {
+  DCHECK(token_service_task_runner_->BelongsToCurrentThread());
+  return provider_->GetTokenService();
+}
+
+OAuth2TokenServiceRequest* OAuth2TokenServiceRequest::Core::owner() {
+  DCHECK(owner_task_runner_->BelongsToCurrentThread());
+  return owner_;
+}

[msarda, 2014/05/30 14:37:31]

I would add a method IsStopped() here and use it below instead of checking
owner() != NULL.

[maniscalco, 2014/05/30 17:48:08]

Done.

+
+namespace {
+
+// An implementation of Core for getting an access token.
+class RequestCore : public OAuth2TokenServiceRequest::Core,
+                    public OAuth2TokenService::Consumer {
+ public:
+  RequestCore(
+      const scoped_refptr<base::SingleThreadTaskRunner>& owner_task_runner,
+      OAuth2TokenServiceRequest* owner,
+      OAuth2TokenServiceRequest::TokenServiceProvider* provider,
+      OAuth2TokenService::Consumer* consumer,
       const std::string& account_id,
       const OAuth2TokenService::ScopeSet& scopes);
-  // Stops the OAuth2 access token fetching. It should be called on the owner
-  // thread.
-  void Stop();
 
-  OAuth2TokenService::Request* request();
-
-  // OAuth2TokenService::Consumer. It should be called on the UI thread.
+  // OAuth2TokenService::Consumer.  Must be called on the token service thread.
   virtual void OnGetTokenSuccess(const OAuth2TokenService::Request* request,
                                  const std::string& access_token,
                                  const base::Time& expiration_time) OVERRIDE;
   virtual void OnGetTokenFailure(const OAuth2TokenService::Request* request,
                                  const GoogleServiceAuthError& error) OVERRIDE;
 
  private:
-  friend class
-      base::RefCountedThreadSafe<ProfileOAuth2TokenServiceRequest::Core>;
-
-  // Note this can be destructed on the owner thread or on the UI thread,
-  // depending on the reference count.
-  virtual ~Core();
-
-  // Starts an OAuth2TokenService::Request on the UI thread.
-  void StartOnUIThread(
-      const std::string& account_id,
-      const OAuth2TokenService::ScopeSet& scopes);
-  // Stops the OAuth2TokenService::Request on the UI thread.
-  void StopOnUIThread();
-
-  // Method posted to the owner thread to call back |owner_|. Note when this
-  // posted task is actually running on the owner thread, it is possible that
-  // |owner_| has been reset NULL.
+  friend class base::RefCountedThreadSafe<RequestCore>;
+
+  // Must be destroyed on the owner thread.
+  virtual ~RequestCore();
+
+  // Core implementation.
+  virtual void StartOnTokenServiceThread() OVERRIDE;
+  virtual void StopOnTokenServiceThread() OVERRIDE;
+
   void InformOwnerOnGetTokenSuccess(std::string access_token,
                                     base::Time expiration_time);
   void InformOwnerOnGetTokenFailure(GoogleServiceAuthError error);
 
-  // The profile with which this instance was initialized.
-  Profile* const profile_;
-
-  // The object to call back when fetching completes. |owner_| should be
-  // called back only on the owner thread.
-  ProfileOAuth2TokenServiceRequest* owner_;
-  // Task runner on which |owner_| should be called back.
-  scoped_refptr<base::SingleThreadTaskRunner> owner_task_runner_;
+  OAuth2TokenService::Consumer* const consumer_;

[msarda, 2014/05/30 14:37:31]

I think we prefer: const OAuth2TokenService::Consumer*

[maniscalco, 2014/05/30 17:48:08]

In this case, the consumer object itself must be non-const.  The const is there
just to make sure the pointer itself doesn't change.  LMK if I should drop the
const entirely.

+  std::string account_id_;
+  OAuth2TokenService::ScopeSet scopes_;
 
   // OAuth2TokenService request for fetching OAuth2 access token; it should be
-  // created, reset and accessed only on the UI thread.
+  // created, reset and accessed only on the token service thread.
   scoped_ptr<OAuth2TokenService::Request> request_;
 
-  DISALLOW_COPY_AND_ASSIGN(Core);
+  DISALLOW_COPY_AND_ASSIGN(RequestCore);
 };
 
-ProfileOAuth2TokenServiceRequest::Core::Core(
-    Profile* profile,
-    ProfileOAuth2TokenServiceRequest* owner)
-    : OAuth2TokenService::Consumer("oauth2_token_service"),
-      profile_(profile),
-      owner_(owner),
-      owner_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
-  DCHECK(profile);
-  DCHECK(owner);
-}
-
-ProfileOAuth2TokenServiceRequest::Core::~Core() {
-}
-
-void ProfileOAuth2TokenServiceRequest::Core::Start(
-    const std::string& account_id,
-    const OAuth2TokenService::ScopeSet& scopes) {
-  DCHECK(owner_task_runner_->BelongsToCurrentThread());
-
-  if (content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) {
-    StartOnUIThread(account_id, scopes);
-  } else {
-    content::BrowserThread::PostTask(
-        content::BrowserThread::UI,
-        FROM_HERE,
-        base::Bind(&ProfileOAuth2TokenServiceRequest::Core::StartOnUIThread,
-                   this, account_id, scopes));
+RequestCore::RequestCore(
+    const scoped_refptr<base::SingleThreadTaskRunner>& owner_task_runner,
+    OAuth2TokenServiceRequest* owner,
+    OAuth2TokenServiceRequest::TokenServiceProvider* provider,
+    OAuth2TokenService::Consumer* consumer,
+    const std::string& account_id,
+    const OAuth2TokenService::ScopeSet& scopes)
+    : OAuth2TokenServiceRequest::Core(owner_task_runner, owner, provider),
+      OAuth2TokenService::Consumer("oauth2_token_service"),
+      consumer_(consumer),
+      account_id_(account_id),
+      scopes_(scopes) {

[msarda, 2014/05/30 14:37:31]

Roger: I remember there were some problems on Android when using empty scopes.
Maybe we should DCHECK here that scopes cannot be empty.

Same for |account_id|.

[Roger Tawa OOO till Jul 10th, 2014/05/30 15:39:00]

Adding a DCHECK for empty scopes is a good idea.  Also in InvalidateCore.

[maniscalco, 2014/05/30 17:48:08]

Done.  I've added DCHECKs for empty scopes and made the DCHECKing more
consistent.  We now DCHECK in all the ctors when we copy/assign parameters into
member variables.  

[maniscalco, 2014/05/30 17:48:08]

Done.

+  DCHECK(consumer_);
+}
+
+RequestCore::~RequestCore() {
+  DCHECK(owner_task_runner()->BelongsToCurrentThread());

[Roger Tawa OOO till Jul 10th, 2014/05/30 15:39:00]

Already done in base class dtor.

[maniscalco, 2014/05/30 17:48:08]

Removed here and elsewhere.

+}
+
+void RequestCore::StartOnTokenServiceThread() {
+  DCHECK(token_service_task_runner()->BelongsToCurrentThread());
+  request_ = token_service()->StartRequest(account_id_, scopes_, this).Pass();
+}
+
+void RequestCore::StopOnTokenServiceThread() {
+  DCHECK(token_service_task_runner()->BelongsToCurrentThread());
+  request_.reset();
+}
+
+void RequestCore::OnGetTokenSuccess(const OAuth2TokenService::Request* request,
+                                    const std::string& access_token,
+                                    const base::Time& expiration_time) {
+  DCHECK(token_service_task_runner()->BelongsToCurrentThread());
+  DCHECK_EQ(request_.get(), request);
+  owner_task_runner()->PostTask(

[msarda, 2014/05/30 14:37:31]

There is slight difference with the previous implementation: if
|owner_task_runner| runs on the current thread, you're actually posting the task
later instead of calling |RequestCore::InformOwnerOnGetTokenSuccess|
synchronously.

I do not think that's an issue, I just want to point out the difference in
behavior.

[maniscalco, 2014/05/30 17:48:08]

Yes, I should have pointed that out.  I removed the synchronous call branches
because I figure having one path (always post) is simpler and will help expose
any hidden bugs.

+      FROM_HERE,
+      base::Bind(&RequestCore::InformOwnerOnGetTokenSuccess,
+                 this,
+                 access_token,
+                 expiration_time));
+  request_.reset();
+}
+
+void RequestCore::OnGetTokenFailure(const OAuth2TokenService::Request* request,
+                                    const GoogleServiceAuthError& error) {
+  DCHECK(token_service_task_runner()->BelongsToCurrentThread());
+  DCHECK_EQ(request_.get(), request);
+  owner_task_runner()->PostTask(
+      FROM_HERE,
+      base::Bind(&RequestCore::InformOwnerOnGetTokenFailure, this, error));
+  request_.reset();
+}
+
+void RequestCore::InformOwnerOnGetTokenSuccess(std::string access_token,
+                                               base::Time expiration_time) {
+  DCHECK(owner_task_runner()->BelongsToCurrentThread());
+  if (owner()) {

[msarda, 2014/05/30 14:37:31]

If you add IsStopped() to Core, then you could use here if (!IsStopped())

[maniscalco, 2014/05/30 17:48:08]

Done.

+    consumer_->OnGetTokenSuccess(owner(), access_token, expiration_time);
   }
 }
 
-void ProfileOAuth2TokenServiceRequest::Core::Stop() {
-  DCHECK(owner_task_runner_->BelongsToCurrentThread());
-
-  // Detaches |owner_| from this instance so |owner_| will be called back only
-  // if |Stop()| has never been called.
-  owner_ = NULL;
-  if (content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)) {
-    StopOnUIThread();
-  } else {
-    content::BrowserThread::PostTask(
-        content::BrowserThread::UI,
-        FROM_HERE,
-        base::Bind(&ProfileOAuth2TokenServiceRequest::Core::StopOnUIThread,
-                   this));
+void RequestCore::InformOwnerOnGetTokenFailure(GoogleServiceAuthError error) {
+  DCHECK(owner_task_runner()->BelongsToCurrentThread());
+  if (owner()) {
+    consumer_->OnGetTokenFailure(owner(), error);
   }
 }
 
-OAuth2TokenService::Request* ProfileOAuth2TokenServiceRequest::Core::request() {
-  return request_.get();
-}
-
-void ProfileOAuth2TokenServiceRequest::Core::StopOnUIThread() {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  request_.reset();
-}
-
-void ProfileOAuth2TokenServiceRequest::Core::StartOnUIThread(
-    const std::string& account_id,
-    const OAuth2TokenService::ScopeSet& scopes) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-
-  ProfileOAuth2TokenService* service =
-      ProfileOAuth2TokenServiceFactory::GetForProfile(profile_);
-  DCHECK(service);
-  SigninManagerBase* signin_manager =
-      SigninManagerFactory::GetForProfile(profile_);
-  DCHECK(signin_manager);
-  std::string account_id_to_use =
-      account_id.empty() ? signin_manager->GetAuthenticatedAccountId()
-                         : account_id;
-  request_.reset(
-      service->StartRequest(account_id_to_use, scopes, this).release());
-}
-
-void ProfileOAuth2TokenServiceRequest::Core::OnGetTokenSuccess(
-    const OAuth2TokenService::Request* request,
+// An implementation of Core for invalidating an access token.
+class InvalidateCore : public OAuth2TokenServiceRequest::Core {
+ public:
+  InvalidateCore(
+      const scoped_refptr<base::SingleThreadTaskRunner>& owner_task_runner,
+      OAuth2TokenServiceRequest* owner,
+      OAuth2TokenServiceRequest::TokenServiceProvider* provider,
+      const std::string& access_token,
+      const std::string& account_id,
+      const OAuth2TokenService::ScopeSet& scopes);
+
+ private:
+  friend class base::RefCountedThreadSafe<InvalidateCore>;
+
+  // Must be destroyed on the owner thread.
+  virtual ~InvalidateCore();
+
+  // Core implementation.
+  virtual void StartOnTokenServiceThread() OVERRIDE;
+  virtual void StopOnTokenServiceThread() OVERRIDE;
+
+  std::string access_token_;
+  std::string account_id_;
+  OAuth2TokenService::ScopeSet scopes_;
+
+  DISALLOW_COPY_AND_ASSIGN(InvalidateCore);
+};
+
+InvalidateCore::InvalidateCore(
+    const scoped_refptr<base::SingleThreadTaskRunner>& owner_task_runner,
+    OAuth2TokenServiceRequest* owner,
+    OAuth2TokenServiceRequest::TokenServiceProvider* provider,
     const std::string& access_token,
-    const base::Time& expiration_time) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  DCHECK_EQ(request_.get(), request);
-  owner_task_runner_->PostTask(FROM_HERE, base::Bind(
-      &ProfileOAuth2TokenServiceRequest::Core::InformOwnerOnGetTokenSuccess,
-      this,
-      access_token,
-      expiration_time));
-  request_.reset();
-}
-
-void ProfileOAuth2TokenServiceRequest::Core::OnGetTokenFailure(
-    const OAuth2TokenService::Request* request,
-    const GoogleServiceAuthError& error) {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-  DCHECK_EQ(request_.get(), request);
-  owner_task_runner_->PostTask(FROM_HERE, base::Bind(
-      &ProfileOAuth2TokenServiceRequest::Core::InformOwnerOnGetTokenFailure,
-      this,
-      error));
-  request_.reset();
-}
-
-void ProfileOAuth2TokenServiceRequest::Core::InformOwnerOnGetTokenSuccess(
-    std::string access_token,
-    base::Time expiration_time) {
-  DCHECK(owner_task_runner_->BelongsToCurrentThread());
-
-  if (owner_)
-    owner_->consumer_->OnGetTokenSuccess(owner_, access_token, expiration_time);
-}
-
-void ProfileOAuth2TokenServiceRequest::Core::InformOwnerOnGetTokenFailure(
-    GoogleServiceAuthError error) {
-  DCHECK(owner_task_runner_->BelongsToCurrentThread());
-
-  if (owner_)
-    owner_->consumer_->OnGetTokenFailure(owner_, error);
-}
+    const std::string& account_id,
+    const OAuth2TokenService::ScopeSet& scopes)
+    : OAuth2TokenServiceRequest::Core(owner_task_runner, owner, provider),
+      access_token_(access_token),
+      account_id_(account_id),
+      scopes_(scopes) {
+  DCHECK(!access_token.empty());
+  DCHECK(!account_id.empty());
+}
+
+InvalidateCore::~InvalidateCore() {
+  DCHECK(owner_task_runner()->BelongsToCurrentThread());

[Roger Tawa OOO till Jul 10th, 2014/05/30 15:39:00]

Already done in base class dtor.

[maniscalco, 2014/05/30 17:48:08]

Removed.

+}
+
+void InvalidateCore::StartOnTokenServiceThread() {
+  DCHECK(token_service_task_runner()->BelongsToCurrentThread());
+  token_service()->InvalidateToken(account_id_, scopes_, access_token_);
+}
+
+void InvalidateCore::StopOnTokenServiceThread() {
+  DCHECK(token_service_task_runner()->BelongsToCurrentThread());
+  // Nothing to do.
+}
+
+}  // namespace
 
 // static
-ProfileOAuth2TokenServiceRequest*
-    ProfileOAuth2TokenServiceRequest::CreateAndStart(
-        Profile* profile,
-        const std::string& account_id,
-        const OAuth2TokenService::ScopeSet& scopes,
-        OAuth2TokenService::Consumer* consumer) {
-  return new ProfileOAuth2TokenServiceRequest(profile, account_id, scopes,
-      consumer);
-}
-
-ProfileOAuth2TokenServiceRequest::ProfileOAuth2TokenServiceRequest(
-    Profile* profile,
+OAuth2TokenServiceRequest* OAuth2TokenServiceRequest::CreateAndStart(
+    TokenServiceProvider* provider,
     const std::string& account_id,
     const OAuth2TokenService::ScopeSet& scopes,
-    OAuth2TokenService::Consumer* consumer)
-    : consumer_(consumer),
-      core_(new Core(profile, this)) {
-  core_->Start(account_id, scopes);
-}
-
-ProfileOAuth2TokenServiceRequest::~ProfileOAuth2TokenServiceRequest() {
+    OAuth2TokenService::Consumer* consumer) {
+  DCHECK(provider);
+  DCHECK(!account_id.empty());
+  DCHECK(consumer);
+  return new OAuth2TokenServiceRequest(base::ThreadTaskRunnerHandle::Get(),
+                                       provider,
+                                       consumer,
+                                       account_id,
+                                       scopes);
+}
+
+OAuth2TokenServiceRequest::OAuth2TokenServiceRequest(

[msarda, 2014/05/30 14:37:31]

Constructors / destructor are at the top of the class.
Static methods come after the constructors.

[maniscalco, 2014/05/30 17:48:08]

Done.  I've reordered the method definitions to match their declaration order.

+    const scoped_refptr<base::SingleThreadTaskRunner>& owner_task_runner,
+    OAuth2TokenServiceRequest::TokenServiceProvider* provider,
+    OAuth2TokenService::Consumer* consumer,
+    const std::string& account_id,
+    const OAuth2TokenService::ScopeSet& scopes)
+    : account_id_(account_id),
+      core_(new RequestCore(owner_task_runner,

[msarda, 2014/05/30 14:37:31]

The two constructors take very similar arguments, but the core they build is
very different.

I vote for having a single constructor that takes as argument |core| and just
create and pass the |core| from the static methods. You may need to have a
SetCore method as |Core|'s constructor takes "this" as argument.

[maniscalco, 2014/05/30 17:48:08]

Done.  OA2TSR now has just one ctor that takes a Core.  I added private
StartWithCore method to kick things off.

On 2014/05/30 14:37:31, msarda wrote:

+                            this,
+                            provider,
+                            consumer,
+                            account_id,
+                            scopes)) {
+  DCHECK(!account_id.empty());
+  core_->Start();
+}
+
+// static
+OAuth2TokenServiceRequest* OAuth2TokenServiceRequest::CreateAndStartInvalidate(
+    OAuth2TokenServiceRequest::TokenServiceProvider* provider,
+    const std::string& account_id,
+    const OAuth2TokenService::ScopeSet& scopes,
+    const std::string& access_token) {
+  DCHECK(provider);
+  DCHECK(!account_id.empty());
+  DCHECK(!access_token.empty());
+  return new OAuth2TokenServiceRequest(base::ThreadTaskRunnerHandle::Get(),
+                                       provider,
+                                       access_token,
+                                       account_id,
+                                       scopes);
+}
+
+OAuth2TokenServiceRequest::OAuth2TokenServiceRequest(
+    const scoped_refptr<base::SingleThreadTaskRunner>& owner_task_runner,
+    OAuth2TokenServiceRequest::TokenServiceProvider* provider,
+    const std::string& access_token,
+    const std::string& account_id,
+    const OAuth2TokenService::ScopeSet& scopes)
+    : account_id_(account_id),
+      core_(new InvalidateCore(owner_task_runner,
+                               this,
+                               provider,
+                               access_token,
+                               account_id,
+                               scopes)) {
+  DCHECK(!account_id.empty());
+  core_->Start();
+}
+
+OAuth2TokenServiceRequest::~OAuth2TokenServiceRequest() {
   DCHECK(CalledOnValidThread());
   core_->Stop();
 }
 
-std::string ProfileOAuth2TokenServiceRequest::GetAccountId() const {
-  return core_->request()->GetAccountId();
-}
-
+std::string OAuth2TokenServiceRequest::GetAccountId() const {
+  return account_id_;
+}