diff options
Diffstat (limited to 'chrome')
| -rw-r--r-- | chrome/browser/geolocation/access_token_store.cc | 155 | ||||
| -rw-r--r-- | chrome/browser/geolocation/access_token_store.h | 65 | ||||
| -rw-r--r-- | chrome/browser/geolocation/access_token_store_browsertest.cc | 114 | ||||
| -rw-r--r-- | chrome/browser/geolocation/fake_access_token_store.h | 48 | ||||
| -rw-r--r-- | chrome/browser/geolocation/geolocation_prefs.cc | 14 | ||||
| -rw-r--r-- | chrome/browser/geolocation/geolocation_prefs.h | 14 | ||||
| -rw-r--r-- | chrome/browser/geolocation/location_arbitrator.cc | 93 | ||||
| -rw-r--r-- | chrome/browser/geolocation/location_arbitrator.h | 11 | ||||
| -rw-r--r-- | chrome/browser/geolocation/location_arbitrator_unittest.cc | 171 | ||||
| -rw-r--r-- | chrome/browser/geolocation/location_provider.h | 4 | ||||
| -rw-r--r-- | chrome/browser/geolocation/network_location_provider.cc | 4 | ||||
| -rw-r--r-- | chrome/browser/geolocation/network_location_provider_unittest.cc | 40 | ||||
| -rwxr-xr-x | chrome/chrome_browser.gypi | 2 | ||||
| -rwxr-xr-x | chrome/chrome_tests.gypi | 3 |
14 files changed, 634 insertions, 104 deletions
diff --git a/chrome/browser/geolocation/access_token_store.cc b/chrome/browser/geolocation/access_token_store.cc index 0055103..9c5388f 100644 --- a/chrome/browser/geolocation/access_token_store.cc +++ b/chrome/browser/geolocation/access_token_store.cc @@ -7,45 +7,152 @@ #include "base/utf_string_conversions.h" #include "base/values.h" #include "chrome/browser/browser_process.h" +#include "chrome/browser/chrome_thread.h" #include "chrome/common/pref_names.h" #include "chrome/common/pref_service.h" #include "googleurl/src/gurl.h" namespace { -// Implementation of the geolocation access token store using chrome's prefs -// store (local_state) to persist these tokens. -// TODO(joth): Calls APIs which probably can't be used from the thread that it -// will be run on; needs prefs access bounced via UI thread. class ChromePrefsAccessTokenStore : public AccessTokenStore { public: + ChromePrefsAccessTokenStore( + const GURL& url, bool token_valid, const string16& initial_access_token); + + private: // AccessTokenStore - virtual bool SetAccessToken(const GURL& url, - const string16& access_token) { - DictionaryValue* access_token_dictionary = - g_browser_process->local_state()->GetMutableDictionary( - prefs::kGeolocationAccessToken); - access_token_dictionary->SetWithoutPathExpansion( - UTF8ToWide(url.spec()), - Value::CreateStringValueFromUTF16(access_token)); - return true; + virtual void DoSetAccessToken(const string16& access_token); +}; + + +class ChromePrefsAccessTokenStoreFactory : public AccessTokenStoreFactory { + private: + void LoadDictionaryStoreInUIThread(ChromeThread::ID client_thread_id, + const GURL& default_url); + void NotifyDelegateInClientThread(const TokenStoreSet& token_stores); + + // AccessTokenStoreFactory + virtual void CreateAccessTokenStores( + const base::WeakPtr<AccessTokenStoreFactory::Delegate>& delegate, + const GURL& default_url); + + base::WeakPtr<AccessTokenStoreFactory::Delegate> delegate_; +}; + +ChromePrefsAccessTokenStore::ChromePrefsAccessTokenStore( + const GURL& url, bool token_valid, const string16& initial_access_token) + : AccessTokenStore(url, token_valid, initial_access_token) { +} + +void ChromePrefsAccessTokenStore::DoSetAccessToken( + const string16& access_token) { + if (!ChromeThread::CurrentlyOn(ChromeThread::UI)) { + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, NewRunnableMethod( + this, &ChromePrefsAccessTokenStore::DoSetAccessToken, + access_token)); + return; } + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); + DictionaryValue* access_token_dictionary = + g_browser_process->local_state()->GetMutableDictionary( + prefs::kGeolocationAccessToken); + access_token_dictionary->SetWithoutPathExpansion( + UTF8ToWide(server_url().spec()), + Value::CreateStringValueFromUTF16(access_token)); +} - virtual bool GetAccessToken(const GURL& url, string16* access_token) { - const DictionaryValue* access_token_dictionary = - g_browser_process->local_state()->GetDictionary( - prefs::kGeolocationAccessToken); - // Careful: The returned value could be NULL if the pref has never been set. - if (access_token_dictionary == NULL) - return false; - return access_token_dictionary->GetStringAsUTF16WithoutPathExpansion( - UTF8ToWide(url.spec()), access_token); +void ChromePrefsAccessTokenStoreFactory::LoadDictionaryStoreInUIThread( + ChromeThread::ID client_thread_id, const GURL& default_url) { + DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI)); + const DictionaryValue* token_dictionary = + g_browser_process->local_state()->GetDictionary( + prefs::kGeolocationAccessToken); + TokenStoreSet token_stores; + // Careful: The returned value could be NULL if the pref has never been set. + if (token_dictionary != NULL) { + for (DictionaryValue::key_iterator it = token_dictionary->begin_keys(); + it != token_dictionary->end_keys(); ++it) { + GURL this_url(WideToUTF8(*it)); + if (!this_url.is_valid()) + continue; + string16 token; + const bool token_valid = + token_dictionary->GetStringAsUTF16WithoutPathExpansion(*it, &token); + token_stores[this_url] = + new ChromePrefsAccessTokenStore(this_url, token_valid, token); + } } -}; + if (default_url.is_valid() && token_stores[default_url] == NULL) { + token_stores[default_url] = + new ChromePrefsAccessTokenStore(default_url, false, string16()); + } + ChromeThread::PostTask( + client_thread_id, FROM_HERE, NewRunnableMethod( + this, + &ChromePrefsAccessTokenStoreFactory::NotifyDelegateInClientThread, + token_stores)); +} +void ChromePrefsAccessTokenStoreFactory::NotifyDelegateInClientThread( + const TokenStoreSet& token_stores) { + if (delegate_ != NULL) { + delegate_->OnAccessTokenStoresCreated(token_stores); + } +} + +void ChromePrefsAccessTokenStoreFactory::CreateAccessTokenStores( + const base::WeakPtr<AccessTokenStoreFactory::Delegate>& delegate, + const GURL& default_url) { + // Capture the thread that created the factory, in order to make callbacks + // on the same thread. We could capture a MessageLoop* but in practice we only + // expect to be called from well-known chrome threads, so this is sufficient. + ChromeThread::ID client_thread_id; + bool ok = ChromeThread::GetCurrentThreadIdentifier(&client_thread_id); + CHECK(ok); + DCHECK_NE(ChromeThread::UI, client_thread_id) + << "If I'm only used from the UI thread I don't need to post tasks"; + delegate_ = delegate; + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, NewRunnableMethod( + this, + &ChromePrefsAccessTokenStoreFactory::LoadDictionaryStoreInUIThread, + client_thread_id, default_url)); +} } // namespace void AccessTokenStore::RegisterPrefs(PrefService* prefs) { prefs->RegisterDictionaryPref(prefs::kGeolocationAccessToken); } -AccessTokenStore* NewChromePrefsAccessTokenStore(); +AccessTokenStore::AccessTokenStore( + const GURL& url, bool token_valid, const string16& initial_access_token) + : url_(url), access_token_valid_(token_valid), + access_token_(initial_access_token) { +} + +AccessTokenStore::~AccessTokenStore() { +} + +GURL AccessTokenStore::server_url() const { + return url_; +} + +void AccessTokenStore::SetAccessToken(const string16& access_token) { + access_token_ = access_token; + access_token_valid_ = true; + DoSetAccessToken(access_token); +} + +bool AccessTokenStore::GetAccessToken(string16* access_token) const { + DCHECK(access_token); + *access_token = access_token_; + return access_token_valid_; +} + +AccessTokenStoreFactory::~AccessTokenStoreFactory() { +} + +// Creates a new access token store backed by the global chome prefs. +AccessTokenStoreFactory* NewChromePrefsAccessTokenStoreFactory() { + return new ChromePrefsAccessTokenStoreFactory; +} diff --git a/chrome/browser/geolocation/access_token_store.h b/chrome/browser/geolocation/access_token_store.h index 11733a4..4a081d8 100644 --- a/chrome/browser/geolocation/access_token_store.h +++ b/chrome/browser/geolocation/access_token_store.h @@ -2,32 +2,79 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +// Defines the Geolocation access token store, and associated factory. +// An access token store is responsible for providing the API to persist a +// single network provider's access token. The factory is responsible for +// first loading tokens when required, and creating the associated token stores. +// The API is a little more complex than one might wish, due to the need for +// prefs access to happen asynchronously on the UI thread. +// These APIs are provided as abstract base classes to allow mocking and testing +// of clients, without dependency on browser process singleton objects etc. + #ifndef CHROME_BROWSER_GEOLOCATION_ACCESS_TOKEN_STORE_H_ #define CHROME_BROWSER_GEOLOCATION_ACCESS_TOKEN_STORE_H_ +#include <map> + #include "base/ref_counted.h" #include "base/string16.h" +#include "base/weak_ptr.h" +#include "googleurl/src/gurl.h" class GURL; class PrefService; // Provides storage for the access token used in the network request. -// Provided to allow for mocking for testing, by decoupling deep browser -// dependencies (singleton prefs & threads) from the geolocaiton object. -class AccessTokenStore : public base::RefCounted<AccessTokenStore> { +class AccessTokenStore : public base::RefCountedThreadSafe<AccessTokenStore> { public: static void RegisterPrefs(PrefService* prefs); - virtual bool SetAccessToken(const GURL& url, - const string16& access_token) = 0; - virtual bool GetAccessToken(const GURL& url, string16* access_token) = 0; + GURL server_url() const; + void SetAccessToken(const string16& access_token); + bool GetAccessToken(string16* access_token) const; + + protected: + friend class base::RefCountedThreadSafe<AccessTokenStore>; + AccessTokenStore( + const GURL& url, bool token_valid, const string16& initial_access_token); + virtual ~AccessTokenStore(); + + virtual void DoSetAccessToken(const string16& access_token) = 0; + + private: + const GURL url_; + bool access_token_valid_; + string16 access_token_; +}; + +// Factory for access token stores. Asynchronously loads all access tokens, and +// calls back with a set of token stores one per server URL. +class AccessTokenStoreFactory + : public base::RefCountedThreadSafe<AccessTokenStoreFactory> { + public: + typedef std::map<GURL, scoped_refptr<AccessTokenStore> > TokenStoreSet; + class Delegate { + public: + virtual void OnAccessTokenStoresCreated( + const TokenStoreSet& access_token_store) = 0; + + protected: + virtual ~Delegate() {} + }; + // delegate will recieve a single callback once existing access tokens have + // been loaded from persistent store. + // If default_url is valid, this additional URL will be added to the + // set of access token stores returned. + virtual void CreateAccessTokenStores( + const base::WeakPtr<AccessTokenStoreFactory::Delegate>& delegate, + const GURL& default_url) = 0; protected: - friend class base::RefCounted<AccessTokenStore>; - virtual ~AccessTokenStore() {} + friend class base::RefCountedThreadSafe<AccessTokenStoreFactory>; + ~AccessTokenStoreFactory(); }; // Creates a new access token store backed by the global chome prefs. -AccessTokenStore* NewChromePrefsAccessTokenStore(); +AccessTokenStoreFactory* NewChromePrefsAccessTokenStoreFactory(); #endif // CHROME_BROWSER_GEOLOCATION_ACCESS_TOKEN_STORE_H_ diff --git a/chrome/browser/geolocation/access_token_store_browsertest.cc b/chrome/browser/geolocation/access_token_store_browsertest.cc new file mode 100644 index 0000000..31b62fc --- /dev/null +++ b/chrome/browser/geolocation/access_token_store_browsertest.cc @@ -0,0 +1,114 @@ +// Copyright (c) 2010 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/geolocation/access_token_store.h" + +#include "chrome/browser/chrome_thread.h" +#include "chrome/test/in_process_browser_test.h" +#include "chrome/test/ui_test_utils.h" + +namespace { +// The token store factory implementation expects to be used from any well-known +// chrome thread other than UI. We could use any arbitrary thread; IO is +// a good choice as this is the expected usage. +const ChromeThread::ID kExpectedClientThreadId = ChromeThread::IO; +const char* kRefServerUrl1 = "https://test.domain.example/foo?id=bar.bar"; +const char* kRefServerUrl2 = "http://another.domain.example/foo?id=bar.bar#2"; +} // namespace + +class GeolocationAccessTokenStoreTest + : public InProcessBrowserTest, + public AccessTokenStoreFactory::Delegate, + public base::SupportsWeakPtr<GeolocationAccessTokenStoreTest> { + protected: + GeolocationAccessTokenStoreTest() + : token_to_expect_(NULL), token_to_set_(NULL) {} + + void StartThreadAndWaitForResults( + const char* ref_url, const string16* token_to_expect, + const string16* token_to_set); + + // AccessTokenStoreFactory::Delegate + virtual void OnAccessTokenStoresCreated( + const AccessTokenStoreFactory::TokenStoreSet& access_token_store); + GURL ref_url_; + const string16* token_to_expect_; + const string16* token_to_set_; +}; + +namespace { +// A WeakPtr may only be used on the thread in which it is created, hence we +// defer the call to delegate->AsWeakPtr() into this function rather than pass +// WeakPtr& in. +void StartTestFromClientThread( + GeolocationAccessTokenStoreTest* delegate, + const GURL& ref_url) { + ASSERT_TRUE(ChromeThread::CurrentlyOn(kExpectedClientThreadId)); + + scoped_refptr<AccessTokenStoreFactory> store = + NewChromePrefsAccessTokenStoreFactory(); + store->CreateAccessTokenStores(delegate->AsWeakPtr(), ref_url); +} +} // namespace + +void GeolocationAccessTokenStoreTest::StartThreadAndWaitForResults( + const char* ref_url, const string16* token_to_expect, + const string16* token_to_set) { + ref_url_ = GURL(ref_url); + token_to_expect_ = token_to_expect; + token_to_set_ = token_to_set; + + ChromeThread::PostTask( + kExpectedClientThreadId, FROM_HERE, NewRunnableFunction( + &StartTestFromClientThread, this, ref_url_)); + ui_test_utils::RunMessageLoop(); +} + +void GeolocationAccessTokenStoreTest::OnAccessTokenStoresCreated( + const AccessTokenStoreFactory::TokenStoreSet& access_token_store) { + ASSERT_TRUE(ChromeThread::CurrentlyOn(kExpectedClientThreadId)) + << "Callback from token factory should be from the same thread as the " + "CreateAccessTokenStores request was made on"; + EXPECT_TRUE(token_to_set_ || token_to_expect_) << "No work to do?"; + DCHECK_GE(access_token_store.size(), size_t(1)); + + AccessTokenStoreFactory::TokenStoreSet::const_iterator item = + access_token_store.find(ref_url_); + ASSERT_TRUE(item != access_token_store.end()); + scoped_refptr<AccessTokenStore> store = item->second; + ASSERT_TRUE(NULL != store); + string16 token; + bool read_ok = store->GetAccessToken(&token); + if (!token_to_expect_) { + EXPECT_FALSE(read_ok); + EXPECT_TRUE(token.empty()); + } else { + ASSERT_TRUE(read_ok); + EXPECT_EQ(*token_to_expect_, token); + } + + if (token_to_set_) { + store->SetAccessToken(*token_to_set_); + } + ChromeThread::PostTask( + ChromeThread::UI, FROM_HERE, new MessageLoop::QuitTask); +} + +IN_PROC_BROWSER_TEST_F(GeolocationAccessTokenStoreTest, SetAcrossInstances) { + const string16 ref_token1 = ASCIIToUTF16("jksdfo90,'s#\"#1*("); + const string16 ref_token2 = ASCIIToUTF16("\1\2\3\4\5\6\7\10\11\12=023"); + ASSERT_TRUE(ChromeThread::CurrentlyOn(ChromeThread::UI)); + + StartThreadAndWaitForResults(kRefServerUrl1, NULL, &ref_token1); + // Check it was set, and change to new value. + StartThreadAndWaitForResults(kRefServerUrl1, &ref_token1, &ref_token2); + // And change back. + StartThreadAndWaitForResults(kRefServerUrl1, &ref_token2, &ref_token1); + StartThreadAndWaitForResults(kRefServerUrl1, &ref_token1, NULL); + + // Set a second server URL + StartThreadAndWaitForResults(kRefServerUrl2, NULL, &ref_token2); + StartThreadAndWaitForResults(kRefServerUrl2, &ref_token2, NULL); + StartThreadAndWaitForResults(kRefServerUrl1, &ref_token1, NULL); +} diff --git a/chrome/browser/geolocation/fake_access_token_store.h b/chrome/browser/geolocation/fake_access_token_store.h new file mode 100644 index 0000000..767142e --- /dev/null +++ b/chrome/browser/geolocation/fake_access_token_store.h @@ -0,0 +1,48 @@ +// Copyright (c) 2010 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. + +#ifndef CHROME_BROWSER_GEOLOCATION_FAKE_ACCESS_TOKEN_STORE_H_ +#define CHROME_BROWSER_GEOLOCATION_FAKE_ACCESS_TOKEN_STORE_H_ + +#include "chrome/browser/geolocation/access_token_store.h" +#include "testing/gtest/include/gtest/gtest.h" + +// A fake (non-persisted) access token store instance useful for testing. +class FakeAccessTokenStore : public AccessTokenStore { + public: + explicit FakeAccessTokenStore(const GURL& url) + : AccessTokenStore(url, false, string16()) {} + + virtual void DoSetAccessToken(const string16& access_token) { + access_token_set_ = access_token; + } + + string16 access_token_set_; + + private: + ~FakeAccessTokenStore() {} +}; + +class FakeAccessTokenStoreFactory : public AccessTokenStoreFactory { + public: + void NotifyDelegateStoreCreated() { + ASSERT_TRUE(delegate_ != NULL); + AccessTokenStoreFactory::TokenStoreSet token_stores; + token_stores[default_url_] = new FakeAccessTokenStore(default_url_); + delegate_->OnAccessTokenStoresCreated(token_stores); + } + + // AccessTokenStoreFactory + virtual void CreateAccessTokenStores( + const base::WeakPtr<AccessTokenStoreFactory::Delegate>& delegate, + const GURL& default_url) { + delegate_ = delegate; + default_url_ = default_url; + } + + base::WeakPtr<AccessTokenStoreFactory::Delegate> delegate_; + GURL default_url_; +}; + +#endif // CHROME_BROWSER_GEOLOCATION_FAKE_ACCESS_TOKEN_STORE_H_ diff --git a/chrome/browser/geolocation/geolocation_prefs.cc b/chrome/browser/geolocation/geolocation_prefs.cc new file mode 100644 index 0000000..c99a18f --- /dev/null +++ b/chrome/browser/geolocation/geolocation_prefs.cc @@ -0,0 +1,14 @@ +// Copyright (c) 2010 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/geolocation/geolocation_prefs.h" + +#include "chrome/browser/geolocation/access_token_store.h" + +namespace geolocation { +void RegisterPrefs(PrefService* prefs) { + // Fan out to all geolocation sub-components that use prefs. + AccessTokenStore::RegisterPrefs(prefs); +} +} // namespace geolocation diff --git a/chrome/browser/geolocation/geolocation_prefs.h b/chrome/browser/geolocation/geolocation_prefs.h new file mode 100644 index 0000000..3873274 --- /dev/null +++ b/chrome/browser/geolocation/geolocation_prefs.h @@ -0,0 +1,14 @@ +// Copyright (c) 2010 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. + +#ifndef CHROME_BROWSER_GEOLOCATION_GEOLOCATION_PREFS_H_ +#define CHROME_BROWSER_GEOLOCATION_GEOLOCATION_PREFS_H_ + +class PrefService; + +namespace geolocation { +void RegisterPrefs(PrefService* prefs); +} + +#endif // CHROME_BROWSER_GEOLOCATION_GEOLOCATION_PREFS_H_ diff --git a/chrome/browser/geolocation/location_arbitrator.cc b/chrome/browser/geolocation/location_arbitrator.cc index 6aa7939..04fba9d 100644 --- a/chrome/browser/geolocation/location_arbitrator.cc +++ b/chrome/browser/geolocation/location_arbitrator.cc @@ -18,80 +18,85 @@ #include "googleurl/src/gurl.h" namespace { - const char* kDefaultNetworkProviderUrl = "https://www.google.com/loc/json"; +} // namespace class GeolocationArbitratorImpl : public GeolocationArbitrator, public LocationProviderBase::ListenerInterface, + public AccessTokenStoreFactory::Delegate, public NonThreadSafe { public: - GeolocationArbitratorImpl(AccessTokenStore* access_token_store, + GeolocationArbitratorImpl(AccessTokenStoreFactory* access_token_store_factory, URLRequestContextGetter* context_getter); ~GeolocationArbitratorImpl(); // GeolocationArbitrator - virtual void AddObserver(Delegate* delegate, + virtual void AddObserver(GeolocationArbitrator::Delegate* delegate, const UpdateOptions& update_options); - virtual void RemoveObserver(Delegate* delegate); + virtual bool RemoveObserver(GeolocationArbitrator::Delegate* delegate); virtual void SetUseMockProvider(bool use_mock); // ListenerInterface virtual void LocationUpdateAvailable(LocationProviderBase* provider); virtual void MovementDetected(LocationProviderBase* provider); + // AccessTokenStoreFactory::Delegate + virtual void OnAccessTokenStoresCreated( + const AccessTokenStoreFactory::TokenStoreSet& access_token_store); + private: void CreateProviders(); bool HasHighAccuracyObserver(); - scoped_refptr<AccessTokenStore> access_token_store_; + scoped_refptr<AccessTokenStoreFactory> access_token_store_factory_; scoped_refptr<URLRequestContextGetter> context_getter_; const GURL default_url_; scoped_ptr<LocationProviderBase> provider_; - typedef std::map<Delegate*, UpdateOptions> DelegateMap; + typedef std::map<GeolocationArbitrator::Delegate*, UpdateOptions> DelegateMap; DelegateMap observers_; - bool use_mock_; -}; - -} // namespace -GeolocationArbitrator* GeolocationArbitrator::New( - AccessTokenStore* access_token_store, - URLRequestContextGetter* context_getter) { - return new GeolocationArbitratorImpl(access_token_store, context_getter); -} + base::WeakPtrFactory<GeolocationArbitratorImpl> weak_ptr_factory_; -GeolocationArbitrator::~GeolocationArbitrator() { -} + bool use_mock_; +}; GeolocationArbitratorImpl::GeolocationArbitratorImpl( - AccessTokenStore* access_token_store, + AccessTokenStoreFactory* access_token_store_factory, URLRequestContextGetter* context_getter) - : access_token_store_(access_token_store), context_getter_(context_getter), - default_url_(kDefaultNetworkProviderUrl), use_mock_(false) { + : access_token_store_factory_(access_token_store_factory), + context_getter_(context_getter), + default_url_(kDefaultNetworkProviderUrl), + ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)), + use_mock_(false) { DCHECK(default_url_.is_valid()); } GeolocationArbitratorImpl::~GeolocationArbitratorImpl() { DCHECK(CalledOnValidThread()); + DCHECK(observers_.empty()) << "Not all observers have unregistered"; } void GeolocationArbitratorImpl::AddObserver( - Delegate* delegate, const UpdateOptions& update_options) { + GeolocationArbitrator::Delegate* delegate, + const UpdateOptions& update_options) { DCHECK(CalledOnValidThread()); observers_[delegate] = update_options; CreateProviders(); } -void GeolocationArbitratorImpl::RemoveObserver(Delegate* delegate) { +bool GeolocationArbitratorImpl::RemoveObserver( + GeolocationArbitrator::Delegate* delegate) { DCHECK(CalledOnValidThread()); - observers_.erase(delegate); - if (observers_.empty()) { + size_t remove = observers_.erase(delegate); + if (observers_.empty() && provider_ != NULL) { // TODO(joth): Delayed callback to linger before destroying the provider. + provider_->UnregisterListener(this); provider_.reset(); } + return remove > 0; } void GeolocationArbitratorImpl::SetUseMockProvider(bool use_mock) { @@ -118,15 +123,37 @@ void GeolocationArbitratorImpl::MovementDetected( DCHECK(CalledOnValidThread()); } +void GeolocationArbitratorImpl::OnAccessTokenStoresCreated( + const AccessTokenStoreFactory::TokenStoreSet& access_token_store) { + DCHECK(provider_ == NULL); + // TODO(joth): Once we have arbitration implementation, iterate the whole set + // rather than cherry-pick our defaul url. + AccessTokenStoreFactory::TokenStoreSet::const_iterator item = + access_token_store.find(default_url_); + DCHECK(item != access_token_store.end()); + DCHECK(item->second != NULL); + if (use_mock_) { + provider_.reset(NewMockLocationProvider()); + } else { + // TODO(joth): Check with GLS folks what hostname they'd like sent. + provider_.reset(NewNetworkLocationProvider( + item->second, context_getter_.get(), + default_url_, ASCIIToUTF16("chromium.org"))); + } + DCHECK(provider_ != NULL); + provider_->RegisterListener(this); + const bool ok = provider_->StartProvider(); + DCHECK(ok); +} + void GeolocationArbitratorImpl::CreateProviders() { DCHECK(CalledOnValidThread()); DCHECK(!observers_.empty()); if (provider_ == NULL) { - // TODO(joth): Check with GLS folks what hostname they'd like sent. - provider_.reset(NewNetworkLocationProvider(access_token_store_.get(), - context_getter_.get(), default_url_, ASCIIToUTF16("chromium.org"))); + access_token_store_factory_->CreateAccessTokenStores( + weak_ptr_factory_.GetWeakPtr(), default_url_); } - // TODO(joth): Use high-accuracy to create conditionally create GPS provider. + // TODO(joth): Use high accuracy flag to conditionally create GPS provider. } bool GeolocationArbitratorImpl::HasHighAccuracyObserver() { @@ -138,3 +165,13 @@ bool GeolocationArbitratorImpl::HasHighAccuracyObserver() { } return false; } + +GeolocationArbitrator* GeolocationArbitrator::New( + AccessTokenStoreFactory* access_token_store_factory, + URLRequestContextGetter* context_getter) { + return new GeolocationArbitratorImpl(access_token_store_factory, + context_getter); +} + +GeolocationArbitrator::~GeolocationArbitrator() { +} diff --git a/chrome/browser/geolocation/location_arbitrator.h b/chrome/browser/geolocation/location_arbitrator.h index 46ab448..aaf61a0 100644 --- a/chrome/browser/geolocation/location_arbitrator.h +++ b/chrome/browser/geolocation/location_arbitrator.h @@ -5,7 +5,7 @@ #ifndef CHROME_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_H_ #define CHROME_BROWSER_GEOLOCATION_LOCATION_ARBITRATOR_H_ -class AccessTokenStore; +class AccessTokenStoreFactory; class URLRequestContextGetter; struct Position; @@ -20,8 +20,9 @@ struct Position; class GeolocationArbitrator { public: // Creates and returns a new instance of the location arbitrator. - static GeolocationArbitrator* New(AccessTokenStore* access_token_store, - URLRequestContextGetter* context_getter); + static GeolocationArbitrator* New( + AccessTokenStoreFactory* access_token_store_factory, + URLRequestContextGetter* context_getter); class Delegate { public: @@ -50,8 +51,8 @@ class GeolocationArbitrator { virtual void AddObserver(Delegate* delegate, const UpdateOptions& update_options) = 0; // Remove a previously registered observer. No-op if not previously registered - // via AddObserver() - virtual void RemoveObserver(Delegate* delegate) = 0; + // via AddObserver(). Returns true if the observer was removed. + virtual bool RemoveObserver(Delegate* delegate) = 0; // TODO(joth): This is a stop-gap for testing; once we have decoupled // provider factory we should extract mock creation from the arbitrator. diff --git a/chrome/browser/geolocation/location_arbitrator_unittest.cc b/chrome/browser/geolocation/location_arbitrator_unittest.cc new file mode 100644 index 0000000..6e0c1d8 --- /dev/null +++ b/chrome/browser/geolocation/location_arbitrator_unittest.cc @@ -0,0 +1,171 @@ +// Copyright (c) 2010 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/geolocation/location_arbitrator.h" + +#include "base/scoped_ptr.h" +#include "chrome/browser/geolocation/fake_access_token_store.h" +#include "chrome/browser/geolocation/geoposition.h" +#include "chrome/browser/geolocation/location_provider.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace { +// Mock implementation of a location provider for testing. +class MockLocationProvider : public LocationProviderBase { + public: + MockLocationProvider() : started_count_(0) { + CHECK(instance_ == NULL); + instance_ = this; + } + virtual ~MockLocationProvider() { + CHECK(instance_ == this); + instance_ = NULL; + } + + using LocationProviderBase::UpdateListeners; + using LocationProviderBase::InformListenersOfMovement; + + // LocationProviderBase implementation. + virtual bool StartProvider() { + ++started_count_; + return true; + } + virtual void GetPosition(Position *position) { + *position = position_; + } + + Position position_; + int started_count_; + + static MockLocationProvider* instance_; + + DISALLOW_COPY_AND_ASSIGN(MockLocationProvider); +}; + +MockLocationProvider* MockLocationProvider::instance_ = NULL; + +class MockLocationObserver : public GeolocationArbitrator::Delegate { + public: + void InvalidateLastPosition() { + last_position_.accuracy = -1; + last_position_.error_code = Position::ERROR_CODE_NONE; + ASSERT_FALSE(last_position_.IsInitialized()); + } + // Delegate + virtual void OnLocationUpdate(const Position& position) { + last_position_ = position; + } + + Position last_position_; +}; + +void SetReferencePosition(Position* position) { + position->latitude = 51.0; + position->longitude = -0.1; + position->accuracy = 400; + position->timestamp = 87654321; + ASSERT_TRUE(position->IsInitialized()); + ASSERT_TRUE(position->IsValidFix()); +} + +} // namespace + +LocationProviderBase* NewMockLocationProvider() { + return new MockLocationProvider; +} + +class GeolocationLocationArbitratorTest : public testing::Test { + protected: + virtual void SetUp() { + access_token_factory_ = new FakeAccessTokenStoreFactory; + arbitrator_.reset(GeolocationArbitrator::New(access_token_factory_.get(), + NULL)); + arbitrator_->SetUseMockProvider(true); + } + + virtual void TearDown() { + } + + scoped_refptr<FakeAccessTokenStoreFactory> access_token_factory_; + scoped_ptr<GeolocationArbitrator> arbitrator_; +}; + +TEST_F(GeolocationLocationArbitratorTest, CreateDestroy) { + EXPECT_TRUE(access_token_factory_); + EXPECT_TRUE(arbitrator_ != NULL); + arbitrator_.reset(); + SUCCEED(); +} + +TEST_F(GeolocationLocationArbitratorTest, NormalUsage) { + ASSERT_TRUE(access_token_factory_); + ASSERT_TRUE(arbitrator_ != NULL); + + EXPECT_FALSE(access_token_factory_->default_url_.is_valid()); + EXPECT_FALSE(access_token_factory_->delegate_); + MockLocationObserver observer; + arbitrator_->AddObserver(&observer, GeolocationArbitrator::UpdateOptions()); + + EXPECT_TRUE(access_token_factory_->default_url_.is_valid()); + ASSERT_TRUE(access_token_factory_->delegate_); + + EXPECT_FALSE(MockLocationProvider::instance_); + access_token_factory_->NotifyDelegateStoreCreated(); + ASSERT_TRUE(MockLocationProvider::instance_); + EXPECT_TRUE(MockLocationProvider::instance_->has_listeners()); + EXPECT_EQ(1, MockLocationProvider::instance_->started_count_); + EXPECT_FALSE(observer.last_position_.IsInitialized()); + + SetReferencePosition(&MockLocationProvider::instance_->position_); + MockLocationProvider::instance_->UpdateListeners(); + EXPECT_TRUE(observer.last_position_.IsInitialized()); + EXPECT_EQ(MockLocationProvider::instance_->position_.latitude, + observer.last_position_.latitude); + + EXPECT_TRUE(arbitrator_->RemoveObserver(&observer)); +} + +TEST_F(GeolocationLocationArbitratorTest, MultipleListener) { + MockLocationObserver observer1; + arbitrator_->AddObserver(&observer1, GeolocationArbitrator::UpdateOptions()); + MockLocationObserver observer2; + arbitrator_->AddObserver(&observer2, GeolocationArbitrator::UpdateOptions()); + + access_token_factory_->NotifyDelegateStoreCreated(); + ASSERT_TRUE(MockLocationProvider::instance_); + EXPECT_FALSE(observer1.last_position_.IsInitialized()); + EXPECT_FALSE(observer2.last_position_.IsInitialized()); + + SetReferencePosition(&MockLocationProvider::instance_->position_); + MockLocationProvider::instance_->UpdateListeners(); + EXPECT_TRUE(observer1.last_position_.IsInitialized()); + EXPECT_TRUE(observer2.last_position_.IsInitialized()); + + // Add third observer, and remove the first. + MockLocationObserver observer3; + arbitrator_->AddObserver(&observer3, GeolocationArbitrator::UpdateOptions()); + EXPECT_TRUE(arbitrator_->RemoveObserver(&observer1)); + observer1.InvalidateLastPosition(); + observer2.InvalidateLastPosition(); + observer3.InvalidateLastPosition(); + + MockLocationProvider::instance_->UpdateListeners(); + EXPECT_FALSE(observer1.last_position_.IsInitialized()); + EXPECT_TRUE(observer2.last_position_.IsInitialized()); + EXPECT_TRUE(observer3.last_position_.IsInitialized()); + + EXPECT_TRUE(arbitrator_->RemoveObserver(&observer2)); + EXPECT_TRUE(arbitrator_->RemoveObserver(&observer3)); +} + +TEST_F(GeolocationLocationArbitratorTest, MultipleRegistrations) { + MockLocationObserver observer; + arbitrator_->AddObserver(&observer, GeolocationArbitrator::UpdateOptions()); + GeolocationArbitrator::UpdateOptions high_accuracy_options; + high_accuracy_options.use_high_accuracy = true; + // TODO(joth): Check this causes the GPS provider to fire up. + arbitrator_->AddObserver(&observer, high_accuracy_options); + EXPECT_TRUE(arbitrator_->RemoveObserver(&observer)); + EXPECT_FALSE(arbitrator_->RemoveObserver(&observer)); +} diff --git a/chrome/browser/geolocation/location_provider.h b/chrome/browser/geolocation/location_provider.h index e5e650f..8ff4acd 100644 --- a/chrome/browser/geolocation/location_provider.h +++ b/chrome/browser/geolocation/location_provider.h @@ -76,9 +76,9 @@ class LocationProviderBase : public NonThreadSafe { // Inform listeners that a new position or error is available, using // LocationUpdateAvailable. - virtual void UpdateListeners(); + void UpdateListeners(); // Inform listeners that movement has been detected, using MovementDetected. - virtual void InformListenersOfMovement(); + void InformListenersOfMovement(); private: // The listeners registered to this provider. For each listener, we store a diff --git a/chrome/browser/geolocation/network_location_provider.cc b/chrome/browser/geolocation/network_location_provider.cc index 932ff0e..8d0c69e 100644 --- a/chrome/browser/geolocation/network_location_provider.cc +++ b/chrome/browser/geolocation/network_location_provider.cc @@ -193,7 +193,7 @@ void NetworkLocationProvider::LocationResponseAvailable( // Record access_token if it's set. if (!access_token.empty() && access_token_ != access_token) { access_token_ = access_token; - access_token_store_->SetAccessToken(request_->url(), access_token); + access_token_store_->SetAccessToken(access_token); } // If new data arrived whilst request was pending reissue the request. @@ -271,7 +271,7 @@ void NetworkLocationProvider::RequestPosition() { is_new_data_available_ = false; if (access_token_.empty()) - access_token_store_->GetAccessToken(request_->url(), &access_token_); + access_token_store_->GetAccessToken(&access_token_); AutoLock data_lock(data_mutex_); request_->MakeRequest(access_token_, radio_data_, wifi_data_, diff --git a/chrome/browser/geolocation/network_location_provider_unittest.cc b/chrome/browser/geolocation/network_location_provider_unittest.cc index 6a56c01..f8af128 100644 --- a/chrome/browser/geolocation/network_location_provider_unittest.cc +++ b/chrome/browser/geolocation/network_location_provider_unittest.cc @@ -4,13 +4,11 @@ #include "chrome/browser/geolocation/network_location_provider.h" -#include <map> - #include "base/json/json_reader.h" #include "base/scoped_ptr.h" #include "base/string_util.h" #include "base/values.h" -#include "chrome/browser/geolocation/access_token_store.h" +#include "chrome/browser/geolocation/fake_access_token_store.h" #include "chrome/browser/net/test_url_fetcher_factory.h" #include "net/url_request/url_request_status.h" #include "testing/gtest/include/gtest/gtest.h" @@ -46,32 +44,6 @@ class MessageLoopQuitListener LocationProviderBase* movement_provider_; }; -class FakeAccessTokenStore : public AccessTokenStore { - public: - FakeAccessTokenStore() : allow_set_(true) {} - - virtual bool SetAccessToken(const GURL& url, - const string16& access_token) { - if (!allow_set_) - return false; - token_map_[url] = access_token; - return true; - } - virtual bool GetAccessToken(const GURL& url, string16* access_token) { - std::map<GURL, string16>::iterator item = token_map_.find(url); - if (item == token_map_.end()) - return false; - *access_token = item->second; - return true; - } - bool allow_set_; - std::map<GURL, string16> token_map_; - - private: - ~FakeAccessTokenStore() {} -}; - - // A mock implementation of DeviceDataProviderImplBase for testing. Adapted from // http://gears.googlecode.com/svn/trunk/gears/geolocation/geolocation_test.cc template<typename DataType> @@ -136,7 +108,7 @@ class GeolocationNetworkProviderTest : public testing::Test { public: virtual void SetUp() { URLFetcher::set_factory(&url_fetcher_factory_); - access_token_store_ = new FakeAccessTokenStore; + access_token_store_ = new FakeAccessTokenStore(test_server_url_); } virtual void TearDown() { @@ -320,9 +292,10 @@ TEST_F(GeolocationNetworkProviderTest, MultipleWifiScansComplete) { ResponseCookies(), kNoFixNetworkResponse); // This should have set the access token anyhow - EXPECT_EQ(1, static_cast<int>(access_token_store_->token_map_.size())); + EXPECT_EQ(access_token_store_->access_token_set_, + ASCIIToUTF16(REFERENCE_ACCESS_TOKEN)); string16 token; - EXPECT_TRUE(access_token_store_->GetAccessToken(test_server_url_, &token)); + EXPECT_TRUE(access_token_store_->GetAccessToken(&token)); EXPECT_EQ(REFERENCE_ACCESS_TOKEN, UTF16ToUTF8(token)); Position position; @@ -366,8 +339,7 @@ TEST_F(GeolocationNetworkProviderTest, MultipleWifiScansComplete) { EXPECT_TRUE(position.IsValidFix()); // Token should still be in the store. - EXPECT_EQ(1, static_cast<int>(access_token_store_->token_map_.size())); - EXPECT_TRUE(access_token_store_->GetAccessToken(test_server_url_, &token)); + EXPECT_TRUE(access_token_store_->GetAccessToken(&token)); EXPECT_EQ(REFERENCE_ACCESS_TOKEN, UTF16ToUTF8(token)); // Wifi updated again, with one less AP. This is 'close enough' to the diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index 15c0037..fc0ba76 100755 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -930,6 +930,8 @@ 'browser/geolocation/device_data_provider.h', 'browser/geolocation/empty_device_data_provider.cc', 'browser/geolocation/empty_device_data_provider.h', + 'browser/geolocation/geolocation_prefs.cc', + 'browser/geolocation/geolocation_prefs.h', 'browser/geolocation/geoposition.cc', 'browser/geolocation/geoposition.h', 'browser/geolocation/location_arbitrator.cc', diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index b151ce3..a45ef70 100755 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -707,6 +707,8 @@ 'browser/extensions/user_script_listener_unittest.cc', 'browser/extensions/user_script_master_unittest.cc', 'browser/find_backend_unittest.cc', + 'browser/geolocation/fake_access_token_store.h', + 'browser/geolocation/location_arbitrator_unittest.cc', 'browser/geolocation/network_location_provider_unittest.cc', 'browser/geolocation/wifi_data_provider_common_unittest.cc', 'browser/geolocation/wifi_data_provider_unittest_win.cc', @@ -1152,6 +1154,7 @@ 'browser/extensions/page_action_apitest.cc', 'browser/extensions/permissions_apitest.cc', 'browser/extensions/stubs_apitest.cc', + 'browser/geolocation/access_token_store_browsertest.cc', 'browser/gtk/bookmark_manager_browsertest.cc', 'browser/net/cookie_policy_browsertest.cc', 'browser/net/ftp_browsertest.cc', |