Hello playmobil,
I'd like you to do a code review. Please execute
g4 diff -c 8939408
or point your web browser to
http://mondrian/8939408
to review the following code:
Change 8939408 by [EMAIL PROTECTED] on 2008/11/10 12:28:25 *pending*
Updated geolocation to properly use browsing_context with AsyncTask.
R=playmobil
[EMAIL PROTECTED]
DELTA=63 (28 added, 0 deleted, 35 changed)
OCL=8939408
Affected files ...
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#59
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/location_provider.h#13
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/location_provider_pool.cc#13
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/location_provider_pool.h#9
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/network_location_provider.cc#27
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/network_location_provider.h#16
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/network_location_request.cc#31
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/network_location_request.h#9
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/reverse_geocoder.cc#3
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/reverse_geocoder.h#2
edit
63 delta lines: 28 added, 0 deleted, 35 changed
Also consider running:
g4 lint -c 8939408
which verifies that the changelist doesn't introduce new style violations.
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [EMAIL PROTECTED]
Change 8939408 by [EMAIL PROTECTED] on 2008/11/10 12:28:25 *pending*
Updated geolocation to properly use browsing_context with AsyncTask.
Affected files ...
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#59
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/location_provider.h#13
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/location_provider_pool.cc#13
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/location_provider_pool.h#9
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/network_location_provider.cc#27
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/network_location_provider.h#16
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/network_location_request.cc#31
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/network_location_request.h#9
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/reverse_geocoder.cc#3
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/reverse_geocoder.h#2
edit
==== //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#59
- c:\src-gears3/googleclient/gears/opensource/gears/geolocation/geolocation.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/geolocation.cc
2008-11-10 12:22:34.000000000 -0800
+++ googleclient/gears/opensource/gears/geolocation/geolocation.cc
2008-11-10 12:07:02.000000000 -0800
@@ -458,7 +458,8 @@
// Mock provider
LocationProviderBase *mock_provider =
- pool->Register(STRING16(L"MOCK"),
+ pool->Register(EnvPageBrowsingContext(),
+ STRING16(L"MOCK"),
STRING16(L""), // url
STRING16(L""), // host_name,
info->request_address,
@@ -478,7 +479,8 @@
STRING16(L"") :
resolved_urls[0];
LocationProviderBase *gps_provider =
- pool->Register(STRING16(L"GPS"),
+ pool->Register(EnvPageBrowsingContext(),
+ STRING16(L"GPS"),
reverse_geocoder_url,
host_name,
info->request_address && !reverse_geocoder_url.empty(),
@@ -493,7 +495,8 @@
// Network providers
for (int i = 0; i < static_cast<int>(resolved_urls.size()); ++i) {
LocationProviderBase *network_provider =
- pool->Register(STRING16(L"NETWORK"),
+ pool->Register(EnvPageBrowsingContext(),
+ STRING16(L"NETWORK"),
resolved_urls[i],
host_name,
info->request_address,
====
//depot/googleclient/gears/opensource/gears/geolocation/location_provider.h#13
-
c:\src-gears3/googleclient/gears/opensource/gears/geolocation/location_provider.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/location_provider.h
2008-11-10 12:22:34.000000000 -0800
+++ googleclient/gears/opensource/gears/geolocation/location_provider.h
2008-11-10 12:00:02.000000000 -0800
@@ -35,6 +35,7 @@
#define GEARS_GEOLOCATION_LOCATION_PROVIDER_H__
#include <map>
+#include "gears/base/common/base_class.h"
#include "gears/base/common/mutex.h"
#include "gears/base/common/string16.h"
@@ -95,8 +96,10 @@
const std::string16 &reverse_geocode_url,
const std::string16 &host_name,
const std::string16 &address_language);
-LocationProviderBase *NewNetworkLocationProvider(const std::string16 &url,
- const std::string16
&host_name,
- const std::string16
&language);
+LocationProviderBase *NewNetworkLocationProvider(
+ BrowsingContext *browsing_context,
+ const std::string16 &url,
+ const std::string16 &host_name,
+ const std::string16 &language);
#endif // GEARS_GEOLOCATION_LOCATION_PROVIDER_H__
====
//depot/googleclient/gears/opensource/gears/geolocation/location_provider_pool.cc#13
-
c:\src-gears3/googleclient/gears/opensource/gears/geolocation/location_provider_pool.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/location_provider_pool.cc
2008-11-10 12:22:34.000000000 -0800
+++ googleclient/gears/opensource/gears/geolocation/location_provider_pool.cc
2008-11-10 12:10:18.000000000 -0800
@@ -59,18 +59,20 @@
}
LocationProviderBase *LocationProviderPool::Register(
- const std::string16 &type,
- const std::string16 &url,
- const std::string16 &host,
- bool request_address,
- const std::string16 &language,
- LocationProviderBase::ListenerInterface *listener) {
+ BrowsingContext *browsing_context,
+ const std::string16 &type,
+ const std::string16 &url,
+ const std::string16 &host,
+ bool request_address,
+ const std::string16 &language,
+ LocationProviderBase::ListenerInterface *listener) {
assert(listener);
MutexLock lock(&providers_mutex_);
std::string16 key = MakeKey(type, url, host, language);
ProviderMap::iterator iter = providers_.find(key);
if (iter == providers_.end()) {
- LocationProviderBase *provider = NewProvider(type, url, host, language);
+ LocationProviderBase *provider = NewProvider(browsing_context, type, url,
+ host, language);
if (!provider) {
return NULL;
}
@@ -121,6 +123,7 @@
}
LocationProviderBase *LocationProviderPool::NewProvider(
+ BrowsingContext *browsing_context,
const std::string16 &type,
const std::string16 &url,
const std::string16 &host,
@@ -140,7 +143,7 @@
} else if (type == kGpsString) {
return NewGpsLocationProvider(url, host, language);
} else if (type == kNetworkString) {
- return NewNetworkLocationProvider(url, host, language);
+ return NewNetworkLocationProvider(browsing_context, url, host, language);
}
assert(false);
return NULL;
====
//depot/googleclient/gears/opensource/gears/geolocation/location_provider_pool.h#9
-
c:\src-gears3/googleclient/gears/opensource/gears/geolocation/location_provider_pool.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/location_provider_pool.h
2008-11-10 12:22:34.000000000 -0800
+++ googleclient/gears/opensource/gears/geolocation/location_provider_pool.h
2008-11-10 12:03:22.000000000 -0800
@@ -44,6 +44,7 @@
// provider if the pool does not alrerady contain an instance of that
// provider. Returns the provider, or NULL if it cannot be created.
LocationProviderBase *Register(
+ BrowsingContext *browsing_context,
const std::string16 &type,
const std::string16 &url,
const std::string16 &host,
@@ -61,7 +62,8 @@
// This method is used only by the Gears test object.
void UseMockLocationProvider(bool use_mock_location_provider);
- LocationProviderBase *NewProvider(const std::string16 &type,
+ LocationProviderBase *NewProvider(BrowsingContext *browsing_context,
+ const std::string16 &type,
const std::string16 &url,
const std::string16 &host,
const std::string16 &language);
====
//depot/googleclient/gears/opensource/gears/geolocation/network_location_provider.cc#27
-
c:\src-gears3/googleclient/gears/opensource/gears/geolocation/network_location_provider.cc
====
# action=edit type=text
---
googleclient/gears/opensource/gears/geolocation/network_location_provider.cc
2008-11-10 12:22:34.000000000 -0800
+++
googleclient/gears/opensource/gears/geolocation/network_location_provider.cc
2008-11-07 15:45:59.000000000 -0800
@@ -25,6 +25,7 @@
#include "gears/geolocation/network_location_provider.h"
+#include "gears/base/common/base_class.h"
#include "gears/base/common/stopwatch.h" // For GetCurrentTimeMillis
#include "gears/geolocation/access_token_manager.h"
#include "gears/geolocation/backoff_manager.h"
@@ -35,16 +36,20 @@
LocationProviderBase *NewNetworkLocationProvider(
+ BrowsingContext *browsing_context,
const std::string16 &url,
const std::string16 &host_name,
const std::string16 &language) {
- return new NetworkLocationProvider(url, host_name, language);
-}
-
-
-NetworkLocationProvider::NetworkLocationProvider(const std::string16 &url,
- const std::string16
&host_name,
- const std::string16 &language)
+ return new NetworkLocationProvider(browsing_context, url, host_name,
+ language);
+}
+
+
+NetworkLocationProvider::NetworkLocationProvider(
+ BrowsingContext *browsing_context,
+ const std::string16 &url,
+ const std::string16 &host_name,
+ const std::string16 &language)
: url_(url),
host_name_(host_name),
request_address_(false),
@@ -52,7 +57,8 @@
address_language_(language),
is_shutting_down_(false),
is_new_data_available_(false),
- is_new_listener_waiting_(false) {
+ is_new_listener_waiting_(false),
+ browsing_context_(browsing_context) {
// TODO(steveblock): Consider allowing multiple values for "address_language"
// in the network protocol to allow better sharing of network location
// providers.
@@ -200,7 +206,8 @@
void NetworkLocationProvider::Run() {
// Create the network request object. We must do this on the same thread from
// which we'll call Start().
- request_ = NetworkLocationRequest::Create(url_, host_name_, this);
+ request_ = NetworkLocationRequest::Create(browsing_context_, url_,
+ host_name_, this);
if (!request_) {
LOG(("Failed to create NetworkLocationRequest object.\n"));
assert(false);
====
//depot/googleclient/gears/opensource/gears/geolocation/network_location_provider.h#16
-
c:\src-gears3/googleclient/gears/opensource/gears/geolocation/network_location_provider.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/network_location_provider.h
2008-11-10 12:22:34.000000000 -0800
+++ googleclient/gears/opensource/gears/geolocation/network_location_provider.h
2008-11-07 15:46:01.000000000 -0800
@@ -41,7 +41,8 @@
public NetworkLocationRequest::ListenerInterface,
public Thread {
public:
- NetworkLocationProvider(const std::string16 &url,
+ NetworkLocationProvider(BrowsingContext *browsing_context,
+ const std::string16 &url,
const std::string16 &host_name,
const std::string16 &language);
virtual ~NetworkLocationProvider();
@@ -124,6 +125,8 @@
// milliseconds.
int64 earliest_next_request_time_;
+ BrowsingContext *browsing_context_;
+
DISALLOW_EVIL_CONSTRUCTORS(NetworkLocationProvider);
};
====
//depot/googleclient/gears/opensource/gears/geolocation/network_location_request.cc#31
-
c:\src-gears3/googleclient/gears/opensource/gears/geolocation/network_location_request.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/network_location_request.cc
2008-11-10 12:22:34.000000000 -0800
+++ googleclient/gears/opensource/gears/geolocation/network_location_request.cc
2008-11-07 15:48:47.000000000 -0800
@@ -75,11 +75,12 @@
// static
NetworkLocationRequest *NetworkLocationRequest::Create(
+ BrowsingContext *browsing_context,
const std::string16 &url,
const std::string16 &host_name,
ListenerInterface *listener) {
scoped_ptr<NetworkLocationRequest> request(
- new NetworkLocationRequest(url, host_name, listener));
+ new NetworkLocationRequest(browsing_context, url, host_name, listener));
assert(request.get());
if (!request.get() || !request->Init() || !request->Start()) {
return NULL;
@@ -87,10 +88,12 @@
return request.release();
}
-NetworkLocationRequest::NetworkLocationRequest(const std::string16 &url,
- const std::string16 &host_name,
- ListenerInterface *listener)
- : AsyncTask(NULL),
+NetworkLocationRequest::NetworkLocationRequest(
+ BrowsingContext *browsing_context,
+ const std::string16 &url,
+ const std::string16 &host_name,
+ ListenerInterface *listener)
+ : AsyncTask(browsing_context),
listener_(listener),
url_(url),
host_name_(host_name),
====
//depot/googleclient/gears/opensource/gears/geolocation/network_location_request.h#9
-
c:\src-gears3/googleclient/gears/opensource/gears/geolocation/network_location_request.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/network_location_request.h
2008-11-10 12:22:34.000000000 -0800
+++ googleclient/gears/opensource/gears/geolocation/network_location_request.h
2008-11-07 15:19:25.000000000 -0800
@@ -55,7 +55,8 @@
// Creates the object and starts its worker thread running. Returns NULL if
// creation or initialisation fails.
- static NetworkLocationRequest* Create(const std::string16 &url,
+ static NetworkLocationRequest* Create(BrowsingContext *browsing_context,
+ const std::string16 &url,
const std::string16 &host_name,
ListenerInterface *listener);
bool MakeRequest(const std::string16 &access_token,
@@ -76,7 +77,8 @@
private:
// Private constructor and destructor. Callers should use Create() and
// StopThreadAndDelete().
- NetworkLocationRequest(const std::string16 &url,
+ NetworkLocationRequest(BrowsingContext *browsing_context,
+ const std::string16 &url,
const std::string16 &host_name,
ListenerInterface *listener);
virtual ~NetworkLocationRequest() {}
====
//depot/googleclient/gears/opensource/gears/geolocation/reverse_geocoder.cc#3 -
c:\src-gears3/googleclient/gears/opensource/gears/geolocation/reverse_geocoder.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/reverse_geocoder.cc
2008-11-10 12:22:34.000000000 -0800
+++ googleclient/gears/opensource/gears/geolocation/reverse_geocoder.cc
2008-11-10 11:31:06.000000000 -0800
@@ -52,11 +52,13 @@
AccessTokenManager::GetInstance()->Unregister();
}
-bool ReverseGeocoder::MakeRequest(const Position &position) {
+bool ReverseGeocoder::MakeRequest(BrowsingContext *browsing_context,
+ const Position &position) {
// Lazily create the network request object. We must do this on the same
// thread from which we'll call MakeRequest().
if (request_ == NULL) {
- request_ = NetworkLocationRequest::Create(url_, host_name_, this);
+ request_ = NetworkLocationRequest::Create(browsing_context, url_,
+ host_name_, this);
}
assert(request_);
====
//depot/googleclient/gears/opensource/gears/geolocation/reverse_geocoder.h#2 -
c:\src-gears3/googleclient/gears/opensource/gears/geolocation/reverse_geocoder.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/reverse_geocoder.h
2008-11-10 12:22:34.000000000 -0800
+++ googleclient/gears/opensource/gears/geolocation/reverse_geocoder.h
2008-11-10 11:29:42.000000000 -0800
@@ -45,7 +45,7 @@
ReverseGeocoderListenerInterface *listener);
virtual ~ReverseGeocoder();
- bool MakeRequest(const Position &position);
+ bool MakeRequest(BrowsingContext *browsing_context, const Position
&position);
private:
// NetworkLocationRequest::ListenerInterface implementation.
