Switch to PostTaskAndReply for AccessTokenStore.

This removes the only usage of the CancelableRequest class in
content/, and is in preparation for moving the class back to
chrome/browser.

The ability to cancel requests to the AccessTokenStore was never used in production, so the interface now changes to a simpler non-cancelable one, and the browsertest GeolocationAccessTokenStoreTest.CancelRequest which tested the functionality is removed.

BUG=98716

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114992
Reverted (ThreadChecker DCHECK on debug bots): r114997

Review URL: http://codereview.chromium.org/8996006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@115325 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 39 insertions, 46 deletions

diff --git a/content/browser/geolocation/access_token_store.cc b/content/browser/geolocation/access_token_store.cc
index 45d0203..5345661 100644
--- a/content/browser/geolocation/access_token_store.cc
+++ b/content/browser/geolocation/access_token_store.cc
@@ -9,15 +9,3 @@ AccessTokenStore::AccessTokenStore() {
 
 AccessTokenStore::~AccessTokenStore() {
 }
-
-AccessTokenStore::Handle AccessTokenStore::LoadAccessTokens(
-    CancelableRequestConsumerBase* consumer,
-    const LoadAccessTokensCallbackType& callback) {
-  scoped_refptr<CancelableRequest<LoadAccessTokensCallbackType> > request(
-      new CancelableRequest<LoadAccessTokensCallbackType>(callback));
-  AddRequest(request, consumer);
-  DCHECK(request->handle());
-
-  DoLoadAccessTokens(request);
-  return request->handle();
-}
diff --git a/content/browser/geolocation/access_token_store.h b/content/browser/geolocation/access_token_store.h
index 2089dfb..5fc2c57 100644
--- a/content/browser/geolocation/access_token_store.h
+++ b/content/browser/geolocation/access_token_store.h
@@ -19,7 +19,6 @@
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
 #include "base/string16.h"
-#include "content/browser/cancelable_request.h"
 #include "content/common/content_export.h"
 #include "googleurl/src/gurl.h"
 
@@ -30,10 +29,8 @@ class URLRequestContextGetter;
 }
 
 // Provides storage for the access token used in the network request.
-class AccessTokenStore : public base::RefCountedThreadSafe<AccessTokenStore>,
-                         public CancelableRequestProvider {
+class AccessTokenStore : public base::RefCountedThreadSafe<AccessTokenStore> {
  public:
-
   // Map of server URLs to associated access token.
   typedef std::map<GURL, string16> AccessTokenSet;
   typedef base::Callback<void(AccessTokenSet, net::URLRequestContextGetter*)>
@@ -45,10 +42,8 @@ class AccessTokenStore : public base::RefCountedThreadSafe<AccessTokenStore>,
   // in Chrome the call to obtain this must also be performed on the UI thread
   // so it is efficient to piggyback it onto this request.
   // Takes ownership of |callback|.
-  // Returns a handle which can subsequently be used with CancelRequest().
-  CONTENT_EXPORT Handle LoadAccessTokens(
-      CancelableRequestConsumerBase* consumer,
-      const LoadAccessTokensCallbackType& callback);
+  CONTENT_EXPORT virtual void LoadAccessTokens(
+      const LoadAccessTokensCallbackType& callback) = 0;
 
   virtual void SaveAccessToken(
       const GURL& server_url, const string16& access_token) = 0;
@@ -58,9 +53,6 @@ class AccessTokenStore : public base::RefCountedThreadSafe<AccessTokenStore>,
   CONTENT_EXPORT AccessTokenStore();
   CONTENT_EXPORT virtual ~AccessTokenStore();
 
-  virtual void DoLoadAccessTokens(
-      scoped_refptr<CancelableRequest<LoadAccessTokensCallbackType> > req) = 0;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(AccessTokenStore);
 };
diff --git a/content/browser/geolocation/arbitrator_dependency_factory.cc b/content/browser/geolocation/arbitrator_dependency_factory.cc
index 25f3954..88fccfc 100644
--- a/content/browser/geolocation/arbitrator_dependency_factory.cc
+++ b/content/browser/geolocation/arbitrator_dependency_factory.cc
@@ -4,6 +4,7 @@
 
 #include "content/browser/geolocation/arbitrator_dependency_factory.h"
 
+#include "base/time.h"
 #include "content/browser/geolocation/access_token_store.h"
 #include "content/browser/geolocation/location_provider.h"
 #include "content/public/browser/content_browser_client.h"
diff --git a/content/browser/geolocation/fake_access_token_store.cc b/content/browser/geolocation/fake_access_token_store.cc
index 4b2dc53..257c8e5 100644
--- a/content/browser/geolocation/fake_access_token_store.cc
+++ b/content/browser/geolocation/fake_access_token_store.cc
@@ -4,30 +4,42 @@
 
 #include "content/browser/geolocation/fake_access_token_store.h"
 
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/logging.h"
+#include "base/message_loop_proxy.h"
+
+using base::MessageLoopProxy;
 using testing::_;
 using testing::Invoke;
 
-FakeAccessTokenStore::FakeAccessTokenStore() {
-  ON_CALL(*this, DoLoadAccessTokens(_))
+FakeAccessTokenStore::FakeAccessTokenStore()
+    : originating_message_loop_(NULL) {
+  ON_CALL(*this, LoadAccessTokens(_))
       .WillByDefault(Invoke(this,
-                            &FakeAccessTokenStore::DefaultDoLoadAccessTokens));
+                            &FakeAccessTokenStore::DefaultLoadAccessTokens));
   ON_CALL(*this, SaveAccessToken(_, _))
       .WillByDefault(Invoke(this,
                             &FakeAccessTokenStore::DefaultSaveAccessToken));
 }
 
 void FakeAccessTokenStore::NotifyDelegateTokensLoaded() {
-  CHECK(request_ != NULL);
+  DCHECK(originating_message_loop_);
+  if (!originating_message_loop_->BelongsToCurrentThread()) {
+    originating_message_loop_->PostTask(
+        FROM_HERE,
+        base::Bind(&FakeAccessTokenStore::NotifyDelegateTokensLoaded, this));
+    return;
+  }
+
   net::URLRequestContextGetter* context_getter = NULL;
-  request_->ForwardResult(access_token_set_, context_getter);
-  request_ = NULL;
+  callback_.Run(access_token_set_, context_getter);
 }
 
-void FakeAccessTokenStore::DefaultDoLoadAccessTokens(
-    scoped_refptr<CancelableRequest<LoadAccessTokensCallbackType> > request) {
-  DCHECK(request_ == NULL)
-      << "Fake token store currently only allows one request at a time";
-  request_ = request;
+void FakeAccessTokenStore::DefaultLoadAccessTokens(
+    const LoadAccessTokensCallbackType& callback) {
+  originating_message_loop_ = MessageLoopProxy::current();
+  callback_ = callback;
 }
 
 void FakeAccessTokenStore::DefaultSaveAccessToken(
diff --git a/content/browser/geolocation/fake_access_token_store.h b/content/browser/geolocation/fake_access_token_store.h
index a3ee08a..4426e59 100644
--- a/content/browser/geolocation/fake_access_token_store.h
+++ b/content/browser/geolocation/fake_access_token_store.h
@@ -6,6 +6,7 @@
 #define CONTENT_BROWSER_GEOLOCATION_FAKE_ACCESS_TOKEN_STORE_H_
 #pragma once
 
+#include "base/message_loop_proxy.h"
 #include "content/browser/geolocation/access_token_store.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -18,24 +19,27 @@ class FakeAccessTokenStore : public AccessTokenStore {
   void NotifyDelegateTokensLoaded();
 
   // AccessTokenStore
-  MOCK_METHOD1(DoLoadAccessTokens,
-               void(scoped_refptr<CancelableRequest
-                    <LoadAccessTokensCallbackType> > request));
+  MOCK_METHOD1(LoadAccessTokens,
+               void(const LoadAccessTokensCallbackType& callback));
   MOCK_METHOD2(SaveAccessToken,
                void(const GURL& server_url, const string16& access_token));
 
-  void DefaultDoLoadAccessTokens(
-      scoped_refptr<CancelableRequest<LoadAccessTokensCallbackType> > request);
+  void DefaultLoadAccessTokens(const LoadAccessTokensCallbackType& callback);
 
   void DefaultSaveAccessToken(const GURL& server_url,
                               const string16& access_token);
 
   AccessTokenSet access_token_set_;
-  scoped_refptr<CancelableRequest<LoadAccessTokensCallbackType> > request_;
+  LoadAccessTokensCallbackType callback_;
 
  private:
   virtual ~FakeAccessTokenStore();
 
+  // In some tests, NotifyDelegateTokensLoaded() is called on a thread
+  // other than the originating thread, in which case we must post
+  // back to it.
+  base::MessageLoopProxy* originating_message_loop_;
+
   DISALLOW_COPY_AND_ASSIGN(FakeAccessTokenStore);
 };
 
diff --git a/content/browser/geolocation/geolocation_provider_unittest.cc b/content/browser/geolocation/geolocation_provider_unittest.cc
index 2b899c1..089de94 100644
--- a/content/browser/geolocation/geolocation_provider_unittest.cc
+++ b/content/browser/geolocation/geolocation_provider_unittest.cc
@@ -141,10 +141,10 @@ TEST_F(GeolocationProviderTest, StartStop) {
       new MockDependencyFactory(&message_loop_, fake_access_token_store.get());
   base::WaitableEvent event(false, false);
 
-  EXPECT_CALL(*(fake_access_token_store.get()), DoLoadAccessTokens(_))
+  EXPECT_CALL(*(fake_access_token_store.get()), LoadAccessTokens(_))
       .Times(1)
       .WillOnce(DoAll(Invoke(fake_access_token_store.get(),
-                             &FakeAccessTokenStore::DefaultDoLoadAccessTokens),
+                             &FakeAccessTokenStore::DefaultLoadAccessTokens),
                       InvokeWithoutArgs(&event, &base::WaitableEvent::Signal)));
 
   GeolocationArbitrator::SetDependencyFactoryForTest(dependency_factory.get());
diff --git a/content/browser/geolocation/location_arbitrator.cc b/content/browser/geolocation/location_arbitrator.cc
index 2765cdc..1c06244 100644
--- a/content/browser/geolocation/location_arbitrator.cc
+++ b/content/browser/geolocation/location_arbitrator.cc
@@ -64,7 +64,6 @@ void GeolocationArbitrator::StartProviders(
   if (providers_.empty()) {
     DCHECK(GURL(kDefaultNetworkProviderUrl).is_valid());
     access_token_store_->LoadAccessTokens(
-        &request_consumer_,
         base::Bind(&GeolocationArbitrator::OnAccessTokenStoresLoaded,
                    base::Unretained(this)));
   } else {
diff --git a/content/browser/geolocation/location_arbitrator.h b/content/browser/geolocation/location_arbitrator.h
index b020191..68d6e11 100644
--- a/content/browser/geolocation/location_arbitrator.h
+++ b/content/browser/geolocation/location_arbitrator.h
@@ -96,7 +96,6 @@ class CONTENT_EXPORT GeolocationArbitrator
   // The provider which supplied the current |position_|
   const LocationProviderBase* position_provider_;
   GURL most_recent_authorized_frame_;
-  CancelableRequestConsumer request_consumer_;
   // The current best estimate of our position.
   Geoposition position_;
 
diff --git a/content/browser/geolocation/location_arbitrator_unittest.cc b/content/browser/geolocation/location_arbitrator_unittest.cc
index b87b600..f730dc5 100644
--- a/content/browser/geolocation/location_arbitrator_unittest.cc
+++ b/content/browser/geolocation/location_arbitrator_unittest.cc
@@ -167,9 +167,7 @@ TEST_F(GeolocationLocationArbitratorTest, NormalUsage) {
   arbitrator_->StartProviders(GeolocationObserverOptions(false));
 
   EXPECT_TRUE(access_token_store_->access_token_set_.empty());
-  EXPECT_TRUE(access_token_store_->request_);
   EXPECT_TRUE(access_token_store_->access_token_set_.empty());
-  ASSERT_TRUE(access_token_store_->request_);
 
   access_token_store_->NotifyDelegateTokensLoaded();
   ASSERT_TRUE(cell());