Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 9069369
or point your web browser to
http://mondrian/9069369
to review the following code:
Change 9069369 by [EMAIL PROTECTED] on 2008/11/19 12:02:12 *pending*
Adds timeout parameter to Geolocation API to match W3C spec.
R=andreip
[EMAIL PROTECTED]
DELTA=452 (323 added, 30 deleted, 99 changed)
OCL=9069369
Affected files ...
... //depot/googleclient/gears/opensource/gears/base/common/position_table.cc#7
edit
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#63
edit
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.h#29
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/geolocation_test.cc#34
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/location_provider.cc#18
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/location_provider.h#15
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/network_location_provider.cc#28
edit
... //depot/googleclient/gears/opensource/gears/sdk/api_geolocation.html#10 edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/geolocation_tests.js#3
edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js#10
edit
452 delta lines: 323 added, 30 deleted, 99 changed
Also consider running:
g4 lint -c 9069369
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 9069369 by [EMAIL PROTECTED] on 2008/11/19 12:02:12 *pending*
Adds timeout parameter to Geolocation API to match W3C spec.
Affected files ...
... //depot/googleclient/gears/opensource/gears/base/common/position_table.cc#7
edit
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#63
edit
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.h#29
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/geolocation_test.cc#34
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/location_provider.cc#18
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/location_provider.h#15
edit
...
//depot/googleclient/gears/opensource/gears/geolocation/network_location_provider.cc#28
edit
... //depot/googleclient/gears/opensource/gears/sdk/api_geolocation.html#10 edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/geolocation_tests.js#3
edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js#10
edit
====
//depot/googleclient/gears/opensource/gears/base/common/position_table.cc#7 -
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/base/common/position_table.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/base/common/position_table.cc
2008-11-21 14:56:40.000000000 +0000
+++ googleclient/gears/opensource/gears/base/common/position_table.cc
2008-11-20 16:19:28.000000000 +0000
@@ -229,6 +229,9 @@
case Position::ERROR_CODE_POSITION_UNAVAILABLE:
position->error_code = Position::ERROR_CODE_POSITION_UNAVAILABLE;
break;
+ case Position::ERROR_CODE_TIMEOUT:
+ position->error_code = Position::ERROR_CODE_TIMEOUT;
+ break;
default:
// Note that in previous versions, error_code was set to kint32min to
// signify no error.
==== //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#63
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/geolocation/geolocation.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/geolocation.cc
2008-11-20 20:09:16.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/geolocation.cc
2008-11-21 14:29:43.000000000 +0000
@@ -53,6 +53,7 @@
// API options constants.
static const char16 *kEnableHighAccuracy = STRING16(L"enableHighAccuracy");
+static const char16 *kTimeout = STRING16(L"timeout");
static const char16 *kGearsRequestAddress = STRING16(L"gearsRequestAddress");
static const char16 *kGearsAddressLanguage = STRING16(L"gearsAddressLanguage");
static const char16 *kGearsLocationProviderUrls =
@@ -68,6 +69,10 @@
// MessageService constants.
static const char16 *kLocationAvailableObserverTopic =
STRING16(L"location available");
+static const char16 *kMovementDetectedObserverTopic =
+ STRING16(L"movement detected");
+static const char16 *kTimeoutExpiredObserverTopic =
+ STRING16(L"timeout expired");
static const char16 *kCallbackRequiredObserverTopic =
STRING16(L"callback required");
@@ -84,12 +89,18 @@
assert(notification_data);
timed_callback_.reset(
new TimedCallback(this, timeout_milliseconds, notification_data));
+ constructor_complete_event_.Signal();
}
// TimedCallback::ListenerInterface implementation
void TimedMessage::OnTimeout(TimedCallback *caller, void *user_data) {
+ assert(caller);
assert(user_data);
- assert(timed_callback_.get());
+
+ // Wait for the constructor to complete so that we know that timed_callback_
+ // has been assigned.
+ constructor_complete_event_.Wait();
+ assert(caller == timed_callback_.get());
// We can't delete the TimedCallback here because we can't call
Thread::Join()
// from the worker thread. Instead we delete the TimedCallback when this
@@ -137,38 +148,40 @@
DISALLOW_EVIL_CONSTRUCTORS(NotificationDataGeoBase);
};
-// Used for messages of type kLocationAvailableObserverTopic.
-class LocationAvailableNotificationData : public NotificationDataGeoBase {
+// Used for messages of types kLocationAvailableObserverTopic and
+// kMovmementDetectedObserverTopic.
+class ProviderNotificationData : public NotificationDataGeoBase {
public:
- LocationAvailableNotificationData(GearsGeolocation *object_in,
- LocationProviderBase *provider_in)
+ ProviderNotificationData(GearsGeolocation *object_in,
+ LocationProviderBase *provider_in)
: NotificationDataGeoBase(object_in),
provider(provider_in) {}
- virtual ~LocationAvailableNotificationData() {}
+ virtual ~ProviderNotificationData() {}
friend class GearsGeolocation;
private:
LocationProviderBase *provider;
- DISALLOW_EVIL_CONSTRUCTORS(LocationAvailableNotificationData);
+ DISALLOW_EVIL_CONSTRUCTORS(ProviderNotificationData);
};
-// Used for messages of type kCallbackRequiredObserverTopic.
-struct CallbackRequiredNotificationData : public NotificationDataGeoBase {
+// Used for messages of types kTimeoutExpiredObserverTopic and
+// kCallbackRequiredObserverTopic.
+struct FixRequestIdNotificationData : public NotificationDataGeoBase {
public:
- CallbackRequiredNotificationData(GearsGeolocation *object_in,
- int fix_request_id_in)
+ FixRequestIdNotificationData(GearsGeolocation *object_in,
+ int fix_request_id_in)
: NotificationDataGeoBase(object_in),
fix_request_id(fix_request_id_in) {}
- virtual ~CallbackRequiredNotificationData() {}
+ virtual ~FixRequestIdNotificationData() {}
friend class GearsGeolocation;
private:
int fix_request_id;
- DISALLOW_EVIL_CONSTRUCTORS(CallbackRequiredNotificationData);
+ DISALLOW_EVIL_CONSTRUCTORS(FixRequestIdNotificationData);
};
// Helper function that checks if the caller had the required permissions
@@ -228,10 +241,12 @@
return;
}
- MessageService::GetInstance()->AddObserver(this,
- kLocationAvailableObserverTopic);
- MessageService::GetInstance()->AddObserver(this,
- kCallbackRequiredObserverTopic);
+ MessageService *message_service = MessageService::GetInstance();
+ assert(message_service);
+ message_service->AddObserver(this, kLocationAvailableObserverTopic);
+ message_service->AddObserver(this, kMovementDetectedObserverTopic);
+ message_service->AddObserver(this, kTimeoutExpiredObserverTopic);
+ message_service->AddObserver(this, kCallbackRequiredObserverTopic);
// Retrieve the cached last known position, if available.
GeolocationDB *db = GeolocationDB::GetDB();
@@ -258,12 +273,12 @@
assert(fix_requests_.empty());
#endif // defined(OS_WINCE) && defined(BROWSER_IE)
- MessageService::GetInstance()->RemoveObserver(
- this,
- kLocationAvailableObserverTopic);
- MessageService::GetInstance()->RemoveObserver(
- this,
- kCallbackRequiredObserverTopic);
+ MessageService *message_service = MessageService::GetInstance();
+ assert(message_service);
+ message_service->RemoveObserver(this, kLocationAvailableObserverTopic);
+ message_service->RemoveObserver(this, kMovementDetectedObserverTopic);
+ message_service->RemoveObserver(this, kTimeoutExpiredObserverTopic);
+ message_service->RemoveObserver(this, kCallbackRequiredObserverTopic);
// Store the last known position.
if (last_position_.IsGoodFix()) {
@@ -382,7 +397,23 @@
// from the JavaScript thread.
MessageService::GetInstance()->NotifyObservers(
kLocationAvailableObserverTopic,
- new LocationAvailableNotificationData(this, provider));
+ new ProviderNotificationData(this, provider));
+ return true;
+}
+
+bool GearsGeolocation::MovementDetected(LocationProviderBase *provider) {
+ assert(provider);
+
+ // We check that the provider that invoked the callback is still in use, in
+ // MovementDetectedImpl. Checking here would require a mutex to guard
+ // providers_.
+
+ // We marshall this callback onto the JavaScript thread. This simplifies
+ // issuing new fix requests and calling back to JavaScript, which must be
done
+ // from the JavaScript thread.
+ MessageService::GetInstance()->NotifyObservers(
+ kMovementDetectedObserverTopic,
+ new ProviderNotificationData(this, provider));
return true;
}
@@ -403,13 +434,30 @@
std::string16 topic_string(topic);
if (topic_string == kLocationAvailableObserverTopic) {
// A location provider reported a new position.
- const LocationAvailableNotificationData *location_available_data =
- reinterpret_cast<const LocationAvailableNotificationData*>(data);
+ const ProviderNotificationData *location_available_data =
+ reinterpret_cast<const ProviderNotificationData*>(data);
LocationUpdateAvailableImpl(location_available_data->provider);
+ } else if (topic_string == kMovementDetectedObserverTopic) {
+ // A location provider reported movement.
+ const ProviderNotificationData *location_available_data =
+ reinterpret_cast<const ProviderNotificationData*>(data);
+ MovementDetectedImpl(location_available_data->provider);
+ } else if (topic_string == kTimeoutExpiredObserverTopic) {
+ // The timeout for a new position being obtained expired.
+ const FixRequestIdNotificationData *timeout_expired_data =
+ reinterpret_cast<const FixRequestIdNotificationData*>(data);
+ // Check that the fix request still exists and that the timeout timer has
+ // not been cancelled.
+ FixRequestInfo *fix_info =
+ MaybeGetFixRequest(timeout_expired_data->fix_request_id);
+ if (fix_info && fix_info->timeout_timer.get()) {
+ fix_info->timeout_timer.reset();
+ MakeTimeoutExpiredCallback(timeout_expired_data->fix_request_id);
+ }
} else if (topic_string == kCallbackRequiredObserverTopic) {
// The timeout for a watch callback expired.
- const CallbackRequiredNotificationData *callback_required_data =
- reinterpret_cast<const CallbackRequiredNotificationData*>(data);
+ const FixRequestIdNotificationData *callback_required_data =
+ reinterpret_cast<const FixRequestIdNotificationData*>(data);
// Check that the fix request still exists
FixRequestInfo *fix_info =
MaybeGetFixRequest(callback_required_data->fix_request_id);
@@ -441,11 +489,7 @@
fix_request_ids.push_back(iter->first);
}
for (int i = 0; i < static_cast<int>(fix_request_ids.size()); ++i) {
- // Cache the pointer to the fix request because RemoveFixRequest will
- // invalidate the iterator.
- FixRequestInfo *fix_request = GetFixRequest(fix_request_ids[i]);
- RemoveFixRequest(fix_request_ids[i]);
- DeleteFixRequest(fix_request);
+ RemoveAndDeleteFixRequest(fix_request_ids[i]);
}
}
@@ -545,17 +589,26 @@
}
}
- // Store and return the ID of this fix if it repeats.
- if (info->repeats) {
- context->SetReturnValue(JSPARAM_INT, &next_watch_id_);
- }
-
// Record this fix. This updates the map of providers and fix requests. The
- // map takes ownership of the info structure.
- if (!RecordNewFixRequest(info.release())) {
+ // map takes ownership of the info structure on success.
+ int fix_request_id = 0;
+ if (!RecordNewFixRequest(info.get(), &fix_request_id)) {
context->SetException(STRING16(L"Exceeded maximum number of fix "
L"requests."));
- }
+ return;
+ }
+ assert(fix_request_id != 0);
+
+ // Start the timeout timer for this fix request.
+ StartTimeoutTimer(fix_request_id);
+
+ // Set the return value if this fix repeats.
+ if (info->repeats) {
+ context->SetReturnValue(JSPARAM_INT, &fix_request_id);
+ }
+
+ // The map has ownership of this fix request.
+ info.release();
}
bool GearsGeolocation::CancelWatch(const int &watch_id) {
@@ -571,18 +624,17 @@
assert(watch_id > 0);
// Update the map of providers that this fix request has been deleted.
- RemoveFixRequest(watch_id);
- DeleteFixRequest(info);
+ RemoveAndDeleteFixRequest(watch_id);
return true;
}
-void GearsGeolocation::HandleRepeatingRequestUpdate(int id,
+void GearsGeolocation::HandleRepeatingRequestUpdate(int fix_request_id,
const Position &position) {
ASSERT_SINGLE_THREAD();
assert(position.IsInitialized());
- FixRequestInfo *fix_info = GetFixRequest(id);
+ FixRequestInfo *fix_info = GetFixRequest(fix_request_id);
assert(fix_info->repeats);
// If this is an error, make a callback.
@@ -591,7 +643,10 @@
return;
}
- // This is a position update.
+ // This is a position update. If there's currently a timer running for a
+ // movement timeout, stop it.
+ fix_info->timeout_timer.reset();
+
if (IsNewPositionMovement(fix_info->last_position, position) ||
IsNewPositionMoreAccurate(fix_info->last_position, position)) {
// The position has changed significantly. See if there's currently a
@@ -617,33 +672,62 @@
} else {
// Start an asynchronous timer which will post a message back to this
// thread once the minimum time period has elapsed.
- MakeFutureSuccessCallback(static_cast<int>(time_remaining), id,
position);
- }
+ MakeFutureSuccessCallback(static_cast<int>(time_remaining),
+ fix_request_id,
+ position);
+ }
+ }
+}
+
+void GearsGeolocation::MakeTimeoutExpiredCallback(int fix_request_id) {
+ ASSERT_SINGLE_THREAD();
+
+ // Confirm that this request uses a timeout.
+ FixRequestInfo *fix_request_info = GetFixRequest(fix_request_id);
+ assert(fix_request_info->timeout >= 0);
+
+ // Call back to JavaScript with an error.
+ Position position;
+ position.error_code = Position::ERROR_CODE_TIMEOUT;
+ position.error_message = STRING16(L"A position fix was not obtained within "
+ L"the specified time limit.");
+ if (!MakeErrorCallback(fix_request_info, position)) {
+ LOG(("GearsGeolocation::MakeTimeoutExpiredCallback() : JavaScript error "
+ "callback failed.\n"));
+ }
+
+ if (!fix_request_info->repeats) {
+ // If it's a non-repeating request, remove the fix request.
+ RemoveAndDeleteFixRequest(fix_request_id);
}
}
void GearsGeolocation::HandleSingleRequestUpdate(LocationProviderBase
*provider,
- int id,
+ int fix_request_id,
const Position &position) {
ASSERT_SINGLE_THREAD();
assert(position.IsInitialized());
- FixRequestInfo *fix_info = GetFixRequest(id);
+ FixRequestInfo *fix_info = GetFixRequest(fix_request_id);
assert(!fix_info->repeats);
assert(fix_info->last_success_callback_time == 0);
// Remove this provider from the this fix so that future callbacks to this
// Geolocation object don't trigger handling for this fix.
- RemoveProvider(provider, id);
+ RemoveProvider(provider, fix_request_id);
// We callback in two cases ...
// - This response gives a good position and we haven't yet called back
// - The fix has no remaining providers, so we'll never get a valid position
// We then cancel any pending requests and delete the fix request.
if (position.IsGoodFix() || fix_info->providers.empty()) {
+ // This is a good fix (or a failure from the last provider). If there's
+ // currently a timer running for a movement timeout, stop it.
+ fix_info->timeout_timer.reset();
+
// Remove the fix request from our map, so that position updates which
occur
// while the callback to JavaScript is in process do not trigger handling
// for this fix request.
- RemoveFixRequest(id);
+ RemoveFixRequest(fix_request_id);
if (position.IsGoodFix()) {
if (!MakeSuccessCallback(fix_info, position)) {
LOG(("GearsGeolocation::HandleSingleRequestUpdate() : JavaScript "
@@ -747,6 +831,57 @@
}
Unref();
+}
+
+void GearsGeolocation::StartTimeoutTimer(int fix_request_id) {
+ ASSERT_SINGLE_THREAD();
+
+ // If this request already has a timer running, we don't update the timer
+ // because we still require a position fix within the timeout period from the
+ // first detection of movement.
+ FixRequestInfo *fix_request_info = GetFixRequest(fix_request_id);
+ if (fix_request_info->timeout_timer.get()) {
+ return;
+ }
+
+ // If this fix request has a timeout specified, set a timer for the specified
+ // interval.
+ if (fix_request_info->timeout >= 0) {
+ fix_request_info->timeout_timer.reset(new TimedMessage(
+ fix_request_info->timeout,
+ kTimeoutExpiredObserverTopic,
+ new FixRequestIdNotificationData(this, fix_request_id)));
+ }
+}
+
+void GearsGeolocation::MovementDetectedImpl(LocationProviderBase *provider) {
+ ASSERT_SINGLE_THREAD();
+
+ // Check that we're still using this provider.
+ ProviderMap::iterator provider_iter = providers_.find(provider);
+ if (provider_iter == providers_.end()) {
+ return;
+ }
+
+ // This provider has detected movement so is in the process of obtaining a
new
+ // fix. Go through all of the fix requests of which this provider is a part
+ // and register this fact. This is to allow the arbitrator to apply the
+ // timeout constraint.
+ //
+ // See comment in LocationUpdateAvailableImpl for why we take a copy of the
+ // fix request IDs.
+ IdList fix_request_ids = provider_iter->second;
+ while (!fix_request_ids.empty()) {
+ int id = fix_request_ids.back();
+ fix_request_ids.pop_back();
+ FixRequestInfoMap::const_iterator iter = fix_requests_.find(id);
+ if (iter != fix_requests_.end()) {
+ // Start the timeout timer for this fix request. We don't care which
+ // provider detected the movement.
+ StartTimeoutTimer(id);
+ }
+ }
+
}
bool GearsGeolocation::MakeSuccessCallback(FixRequestInfo *fix_info,
@@ -864,6 +999,7 @@
// Set default values for options.
info->enable_high_accuracy = false;
+ info->timeout = -1; // No limit is applied
info->request_address = false;
urls->clear();
// We have to check that options is present because it's not valid to use an
@@ -897,6 +1033,19 @@
context->SetException(error);
return false;
}
+ if (options->GetPropertyType(kTimeout) != JSPARAM_UNDEFINED) {
+ int timeout = -1;
+ if (!options->GetPropertyAsInt(kTimeout, &timeout) ||
+ timeout < 0) {
+ std::string16 error = STRING16(L"options.");
+ error += kTimeout;
+ error += STRING16(L" should be a non-negative 32 bit signed integer.");
+ context->SetException(error);
+ return false;
+ }
+ info->timeout = timeout;
+ assert(info->timeout >= 0);
+ }
if (options->GetPropertyType(kGearsRequestAddress) != JSPARAM_UNDEFINED &&
!options->GetPropertyAsBool(kGearsRequestAddress,
&(info->request_address))) {
@@ -1050,22 +1199,23 @@
return iter == fix_requests_.end() ? NULL : iter->second;
}
-bool GearsGeolocation::RecordNewFixRequest(FixRequestInfo *fix_request) {
- ASSERT_SINGLE_THREAD();
-
- int id;
+bool GearsGeolocation::RecordNewFixRequest(FixRequestInfo *fix_request,
+ int *fix_request_id) {
+ ASSERT_SINGLE_THREAD();
+ assert(fix_request_id);
+
if (fix_request->repeats) {
if (next_watch_id_ == kLastRepeatingRequestId) {
return false;
}
- id = next_watch_id_++;
+ *fix_request_id = next_watch_id_++;
} else {
if (next_watch_id_ == kLastSingleRequestId) {
return false;
}
- id = next_single_request_id_--;
- }
- fix_requests_[id] = fix_request;
+ *fix_request_id = next_single_request_id_--;
+ }
+ fix_requests_[*fix_request_id] = fix_request;
// For each location provider used by this request, update the provider's
// list of fix requests in the map.
@@ -1076,7 +1226,7 @@
LocationProviderBase *provider = *iter;
// If providers_ does not yet have an entry for this provider, this will
// create one.
- providers_[provider].push_back(id);
+ providers_[provider].push_back(*fix_request_id);
}
// Increment our ref count to keep this object in scope until we make the
@@ -1093,11 +1243,11 @@
return true;
}
-void GearsGeolocation::RemoveFixRequest(int id) {
- ASSERT_SINGLE_THREAD();
-
- FixRequestInfo *fix_request = GetFixRequest(id);
- fix_requests_.erase(id);
+void GearsGeolocation::RemoveFixRequest(int fix_request_id) {
+ ASSERT_SINGLE_THREAD();
+
+ FixRequestInfo *fix_request = GetFixRequest(fix_request_id);
+ fix_requests_.erase(fix_request_id);
// For each location provider used by this request, update the provider's
// list of fix requests in the map.
@@ -1113,7 +1263,8 @@
// Find this fix request in the list of fix requests for this provider.
IdList *ids = &(provider_iter->second);
- IdList::iterator id_iterator = std::find(ids->begin(), ids->end(), id);
+ IdList::iterator id_iterator =
+ std::find(ids->begin(), ids->end(), fix_request_id);
// If we can't find this request the list, something has gone wrong.
assert(id_iterator != ids->end());
@@ -1145,10 +1296,19 @@
Unref();
}
-void GearsGeolocation::RemoveProvider(LocationProviderBase *provider, int id) {
- ASSERT_SINGLE_THREAD();
-
- FixRequestInfo *fix_request = GetFixRequest(id);
+void GearsGeolocation::RemoveAndDeleteFixRequest(int fix_request_id) {
+ // Cache the pointer to the fix request because RemoveFixRequest will
+ // invalidate the iterator.
+ FixRequestInfo *fix_request = GetFixRequest(fix_request_id);
+ RemoveFixRequest(fix_request_id);
+ DeleteFixRequest(fix_request);
+}
+
+void GearsGeolocation::RemoveProvider(LocationProviderBase *provider,
+ int fix_request_id) {
+ ASSERT_SINGLE_THREAD();
+
+ FixRequestInfo *fix_request = GetFixRequest(fix_request_id);
assert(!fix_request->repeats);
ProviderVector *member_providers = &fix_request->providers;
@@ -1165,7 +1325,8 @@
ProviderMap::iterator provider_iter = providers_.find(provider);
assert(provider_iter != providers_.end());
IdList *ids = &(provider_iter->second);
- IdList::iterator id_iterator = std::find(ids->begin(), ids->end(), id);
+ IdList::iterator id_iterator =
+ std::find(ids->begin(), ids->end(), fix_request_id);
assert(id_iterator != ids->end());
ids->erase(id_iterator);
@@ -1196,7 +1357,7 @@
fix_info->success_callback_timer.reset(new TimedMessage(
timeout_milliseconds,
kCallbackRequiredObserverTopic,
- new CallbackRequiredNotificationData(this, fix_request_id)));
+ new FixRequestIdNotificationData(this, fix_request_id)));
}
// Local functions
==== //depot/googleclient/gears/opensource/gears/geolocation/geolocation.h#29 -
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/geolocation/geolocation.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/geolocation.h
2008-11-20 20:09:16.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/geolocation.h
2008-11-21 14:29:21.000000000 +0000
@@ -89,8 +89,7 @@
enum ErrorCode {
ERROR_CODE_NONE = -1, // Gears addition
ERROR_CODE_POSITION_UNAVAILABLE = 2,
- // TODO(steveblock): Implement PositionOptions.timeout.
- //ERROR_CODE_TIMEOUT = 3,
+ ERROR_CODE_TIMEOUT = 3,
};
Position()
@@ -145,6 +144,7 @@
private:
scoped_ptr<TimedCallback> timed_callback_;
std::string16 message_type_;
+ Event constructor_complete_event_;
DISALLOW_EVIL_CONSTRUCTORS(TimedMessage);
};
@@ -217,6 +217,9 @@
FixRequestInfo() : last_success_callback_time(0) {}
ProviderVector providers;
bool enable_high_accuracy;
+ // The maximum time period in which this request must obtain a fix. A value
+ // of -1 implies no limit is applied. Must be non-negative otherwise.
+ int timeout;
bool request_address;
std::string16 address_language;
bool repeats;
@@ -234,11 +237,15 @@
// The position that will be used used for a pending future success
callback
// in a watch.
Position pending_position;
+ // The timer used for a enforcing the timeout in the case of movement
+ // reported by a provider.
+ linked_ptr<TimedMessage> timeout_timer;
};
private:
// LocationProviderBase::ListenerInterface implementation.
virtual bool LocationUpdateAvailable(LocationProviderBase *provider);
+ virtual bool MovementDetected(LocationProviderBase *provider);
// MessageObserverInterface implementation.
virtual void OnNotify(MessageService *service,
@@ -249,8 +256,9 @@
// event.
void HandleEvent(JsEventType event_type);
- // Internal method used by OnNotify.
+ // Internal methods used by OnNotify.
void LocationUpdateAvailableImpl(LocationProviderBase *provider);
+ void MovementDetectedImpl(LocationProviderBase *provider);
// Internal method used by GetCurrentPosition and WatchPosition to get a
// position fix.
@@ -265,14 +273,17 @@
// Internal method used by LocationUpdateAvailable to handle an update for a
// repeating fix request.
- void HandleRepeatingRequestUpdate(int id,
+ void HandleRepeatingRequestUpdate(int fix_request_id,
const Position &position);
// Internal method used by LocationUpdateAvailable to handle an update for a
// non-repeating fix request.
void HandleSingleRequestUpdate(LocationProviderBase *provider,
- int id,
+ int fix_request_id,
const Position &position);
+
+ void StartTimeoutTimer(int fix_request_id);
+ void MakeTimeoutExpiredCallback(int fix_request_id);
// Internal method to make the callback to JavaScript once we have a postion
// fix.
@@ -314,18 +325,19 @@
// Takes a pointer to a new fix request and records it in our map. Returns
// false if the maximum number of fix requests has been reached. Otherwise
// return true.
- bool RecordNewFixRequest(FixRequestInfo *fix_request);
+ bool RecordNewFixRequest(FixRequestInfo *fix_request, int *fix_request_id);
// Removes a fix request. Cancels any pending requests to the location
// providers it uses. Note that this does not delete the FixRequestInfo
// object.
- void RemoveFixRequest(int id);
+ void RemoveFixRequest(int fix_request_id);
// Deletes a fix request and decrements our ref count.
void DeleteFixRequest(FixRequestInfo *fix_request);
+ void RemoveAndDeleteFixRequest(int fix_request_id);
// Removes a location provider from a fix request.
- void RemoveProvider(LocationProviderBase *provider, int id);
+ void RemoveProvider(LocationProviderBase *provider, int fix_request_id);
// Causes a callback to JavaScript to be made at the specified number of
// milliseconds in the future.
====
//depot/googleclient/gears/opensource/gears/geolocation/geolocation_test.cc#34
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/geolocation/geolocation_test.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/geolocation_test.cc
2008-11-21 14:56:40.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/geolocation_test.cc
2008-11-21 14:04:00.000000000 +0000
@@ -258,9 +258,12 @@
return;
}
}
+ // TODO(steveblock): Consider sharing these string constants with those in
+ // geolocation.cc.
if (!return_object->SetPropertyBool(STRING16(L"repeats"), info.repeats) ||
!return_object->SetPropertyBool(STRING16(L"enableHighAccuracy"),
info.enable_high_accuracy) ||
+ !return_object->SetPropertyInt(STRING16(L"timeout"), info.timeout) ||
!return_object->SetPropertyBool(STRING16(L"gearsRequestAddress"),
info.request_address) ||
!return_object->SetPropertyString(STRING16(L"gearsAddressLanguage"),
@@ -567,6 +570,9 @@
case Position::ERROR_CODE_POSITION_UNAVAILABLE:
position->error_code = Position::ERROR_CODE_POSITION_UNAVAILABLE;
break;
+ case Position::ERROR_CODE_TIMEOUT:
+ position->error_code = Position::ERROR_CODE_TIMEOUT;
+ break;
}
}
====
//depot/googleclient/gears/opensource/gears/geolocation/location_provider.cc#18
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/geolocation/location_provider.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/location_provider.cc
2008-11-21 14:56:40.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/location_provider.cc
2008-11-21 12:05:45.000000000 +0000
@@ -72,6 +72,15 @@
}
}
+void LocationProviderBase::InformListenersOfMovement() {
+ MutexLock lock(&listeners_mutex_);
+ for (ListenerMap::const_iterator iter = listeners_.begin();
+ iter != listeners_.end();
+ ++iter) {
+ iter->first->MovementDetected(this);
+ }
+}
+
LocationProviderBase::ListenerMap *LocationProviderBase::GetListeners() {
return &listeners_;
}
====
//depot/googleclient/gears/opensource/gears/geolocation/location_provider.h#15
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/geolocation/location_provider.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/location_provider.h
2008-11-21 14:56:40.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/location_provider.h
2008-11-21 12:13:25.000000000 +0000
@@ -47,7 +47,17 @@
public:
class ListenerInterface {
public:
+ // Used to inform listener that a new position fix is available or that a
+ // fatal error has occurred. Providers should call this for new listeners
+ // as soon as a position is available.
virtual bool LocationUpdateAvailable(LocationProviderBase *provider) = 0;
+ // Used to inform listener that movement has been detected. If obtaining
the
+ // position succeeds, this will be followed by a call to
+ // LocationUpdateAvailable. Some providers may not be able to detect
+ // movement before a new fix is obtained, so will never call this method.
+ // Note that this is not called in response to registration of a new
+ // listener.
+ virtual bool MovementDetected(LocationProviderBase *provider) = 0;
virtual ~ListenerInterface() {}
};
@@ -80,7 +90,11 @@
Mutex *GetListenersMutex();
protected:
+ // Inform listeners that a new position or error is available, using
+ // LocationUpdateAvailable.
virtual void UpdateListeners();
+ // Inform listeners that movement has been detected, using MovementDetected.
+ virtual void InformListenersOfMovement();
private:
// The listeners registered to this provider. For each listener, we store a
====
//depot/googleclient/gears/opensource/gears/geolocation/network_location_provider.cc#28
-
c:\MyDocs\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-21 14:56:41.000000000 +0000
+++
googleclient/gears/opensource/gears/geolocation/network_location_provider.cc
2008-11-21 12:18:37.000000000 +0000
@@ -245,6 +245,10 @@
}
// The event should be due to new device data or a new listener.
assert(is_new_data_available_ || is_new_listener_waiting_);
+ // If we have new data available, inform listeners of movement.
+ if (is_new_data_available_) {
+ InformListenersOfMovement();
+ }
// Lock the data mutex to test is_radio_data_complete_ and
// is_wifi_data_complete_ on the next loop.
data_mutex_.Lock();
@@ -322,6 +326,11 @@
make_request = true;
}
+ // If we have new data available, inform listeners of movement.
+ if (is_new_data_available_) {
+ InformListenersOfMovement();
+ }
+
// TODO(steveblock): If the request does not complete within some maximum
// time period, we should kill it and start a new request.
if (make_request) {
==== //depot/googleclient/gears/opensource/gears/sdk/api_geolocation.html#10 -
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/sdk/api_geolocation.html
====
# action=edit type=text
--- googleclient/gears/opensource/gears/sdk/api_geolocation.html
2008-11-21 14:56:41.000000000 +0000
+++ googleclient/gears/opensource/gears/sdk/api_geolocation.html
2008-11-21 14:33:36.000000000 +0000
@@ -127,6 +127,7 @@
<pre><code><a href="#positionoptions">PositionOptions class</a>
readwrite attribute bool enableHighAccuracy
+ readwrite attribute int timeout
readwrite attribute bool gearsRequestAddress
readwrite attribute string gearsAddressLanguage
readwrite attribute string[] gearsLocationProviderUrls</code></pre>
@@ -163,18 +164,20 @@
<td>Parameters:</td>
<td class="odd"><code>successCallback(Position position)</code>
<br />
- <code>errorCallback</code> <code>(PositionError error)</code> - optional,
+ <code>errorCallback(PositionError error)</code> - optional,
pass <code>null</code> if you do not want to make use of the callback.
<br />
<code>options</code> - optional, specifies the options to use for this
request, see <code><a
href="#positionoptions">PositionOptions</a></code>.</td>
</tr><tr class="odd">
<td>Description:</td>
- <td class="odd">Obtains a new position. Obtains a new position. If
-successful, the success callback function is called exactly once with a
-good position. If none of the location providers find a good fix, the
-error callback is called exactly once with an error message unless you
-have passed <code>null</code> for <code>errorCallback</code>. <br />
+ <td class="odd">Attempts to obtain a new position. If a good fix is
obtained
+within the time specified by <code>PositionOptions.timeout</code>, the success
+callback function is called exactly once with a good position. If the timeout
+expires or none of the location providers are able to provide a good fix, the
+error callback is called exactly, unless <code>null</code> was passed for
+<code>errorCallback</code>.
+<br />
<br />
The signature of the success callback is:<br />
<code>function successCallback(Position position)</code><br />
@@ -196,7 +199,7 @@
<tr class="odd">
<td>Parameters:</td>
<td class="odd"><code>successCallback(Position position)</code><br />
-<code>errorCallback</code> <code>(PositionError error)</code> - optional,
+<code>errorCallback(PositionError error)</code> - optional,
pass <code>null</code> if you do not want to make use of the callback.<br />
<code>options</code> - optional, specifies the options to use for this
request, see <code><a href="#positionoptions">PositionOptions</a></code>.</td>
@@ -204,11 +207,14 @@
<tr class="odd">
<td>Description:</td>
<td class="odd">Repeatedly obtains a new position. The success callback
-function is called as soon as a good position is available, or whenever
-the position changes significantly, subject to a maximum callback
frequency.<br />
-The error callback is called if a fatal error occurs which will prevent
-a position fix from ever being obtained by this watch unless you
-have passed <code>null</code> for <code>errorCallback</code>.<br />
+function is called as soon as a good position is available, or whenever the
+position changes significantly, subject to a maximum callback frequency. The
+error callback is called if a fatal error occurs which will prevent a position
+fix from ever being obtained by this watch, unless <code>null</code> was passed
+for <code>errorCallback</code>. The error callback, if supplied, is also called
+if Gears detects that the device has moved, but is unable to obtain a new
+position fix within the time specifed by <code>PositionOptions.timeout</code>.
+<br />
<br />
The signature of the success callback is:<br />
<code>function successCallback(Position position)</code><br />
@@ -365,6 +371,15 @@
is false.</td>
</tr>
<tr class="odd">
+ <td><strong>timeout</strong></td>
+ <td class="odd">readwrite int</td>
+ <td class="odd">Optional, specifies the maximum time period for which Gears
+ should attempt to obtain a position fix. If a position fix can not be
obtained
+ within this period, the error callback is invoked with code 3. The value is
+ limited to a non-negative signed 32 bit signed integer. If this value is not
+ set, no time limit is applied.</td>
+</tr>
+<tr class="odd">
<td><strong>gearsRequestAddress</strong></td>
<td class="odd">readwrite bool</td>
<td class="odd">Optional, requests reverse geocoded address information
@@ -400,7 +415,7 @@
<tr class="odd">
<th width="158">Attribute</th>
<th width="109">Type</th>
- <th width="432">Description</th>
+ <th width="500">Description</th>
</tr>
<tr class="odd">
<td><strong>code</strong></td>
@@ -413,6 +428,11 @@
<td><strong>2</strong></td>
<td>No position fix could be obtained.</td>
</tr>
+ <tr>
+ <td><strong>3</strong></td>
+ <td>The specified maximum time elapsed before a position could be
+ obtained.</td>
+ </tr>
</table>
</td>
</tr>
====
//depot/googleclient/gears/opensource/gears/test/testcases/geolocation_tests.js#3
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/test/testcases/geolocation_tests.js
====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/testcases/geolocation_tests.js
2008-11-21 14:56:41.000000000 +0000
+++ googleclient/gears/opensource/gears/test/testcases/geolocation_tests.js
2008-11-21 14:43:07.000000000 +0000
@@ -23,16 +23,28 @@
// OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
// ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-function testGeolocationArguments() {
- var geolocation = google.gears.factory.create('beta.geolocation');
- var dummyFunction = function() {};
+// Error code values. These values must match those in geolocation.h.
+var ERROR_CODE_TIMEOUT = 3;
+var geolocation = google.gears.factory.create('beta.geolocation');
+var dummyFunction = function() {};
+
+function testArguments() {
// All good.
+ var goodOptions = {
+ enableHighAccuracy: false,
+ timeout: 0,
+ gearsRequestAddress: false,
+ gearsAddressLanguage: 'test',
+ gearsLocationProviderUrls: ['test']
+ };
geolocation.getCurrentPosition(dummyFunction);
geolocation.getCurrentPosition(dummyFunction, dummyFunction);
geolocation.getCurrentPosition(dummyFunction, null);
geolocation.getCurrentPosition(dummyFunction, null, {});
+ geolocation.getCurrentPosition(dummyFunction, null, goodOptions);
geolocation.getCurrentPosition(dummyFunction, dummyFunction, {});
+ geolocation.getCurrentPosition(dummyFunction, dummyFunction, goodOptions);
// Test correct types.
// Missing success callback.
@@ -57,6 +69,22 @@
dummyFunction,
{enableHighAccuracy: 42});
}, 'options.enableHighAccuracy should be a boolean.');
+ // Wrong type for timeout.
+ assertError(function() {
+ geolocation.getCurrentPosition(dummyFunction,
+ dummyFunction,
+ {timeout: 42.9});
+ }, 'options.timeout should be a non-negative 32 bit signed integer.');
+ assertError(function() {
+ geolocation.getCurrentPosition(dummyFunction,
+ dummyFunction,
+ {timeout: -1});
+ }, 'options.timeout should be a non-negative 32 bit signed integer.');
+ assertError(function() {
+ geolocation.getCurrentPosition(dummyFunction,
+ dummyFunction,
+ {timeout: 2147483648}); // 2^31
+ }, 'options.timeout should be a non-negative 32 bit signed integer.');
// Wrong type for gearsRequestAddress.
assertError(
function() {
@@ -90,8 +118,9 @@
},
'options.gearsLocationProviderUrls should be null or an array of ' +
'strings');
+}
- // No providers.
+function testNoProviders() {
assertError(
function() {
geolocation.getCurrentPosition(
@@ -103,3 +132,16 @@
'Calling getCurrentPosition() should fail if no location providers are '
+
'specified.');
}
+
+function testZeroTimeout() {
+ // A request with a zero timeout should call the error callback immediately.
+ function errorCallback(error) {
+ assertEqual(
+ ERROR_CODE_TIMEOUT,
+ error.code,
+ 'Error callback should be called with code ERROR_CODE_TIMEOUT (3).');
+ completeAsync();
+ }
+ startAsync();
+ geolocation.getCurrentPosition(dummyFunction, errorCallback, {timeout: 0});
+}
====
//depot/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js#10
-
c:\MyDocs\Gears3/googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js
====
# action=edit type=text
---
googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js
2008-11-21 14:56:41.000000000 +0000
+++
googleclient/gears/opensource/gears/test/testcases/internal_geolocation_tests.js
2008-11-21 14:55:46.000000000 +0000
@@ -37,44 +37,37 @@
var dummyFunction = function() {};
// Test correct parsing.
- var defaultUrlArray = ['http://www.google.com/loc/json'];
+ var correctOptions = {
+ repeats: false,
+ enableHighAccuracy: false,
+ timeout: -1,
+ gearsRequestAddress: false,
+ gearsAddressLanguage: '',
+ gearsLocationProviderUrls: ['http://www.google.com/loc/json']
+ };
+
var urls = ['url1', 'url2'];
var parsedOptions;
// No options.
parsedOptions = internalTests.testParseGeolocationOptions(dummyFunction);
- assertEqual(false, parsedOptions.repeats);
- assertEqual(false, parsedOptions.enableHighAccuracy);
- assertEqual(false, parsedOptions.gearsRequestAddress);
- assertEqual('', parsedOptions.gearsAddressLanguage);
- assertArrayEqual(defaultUrlArray, parsedOptions.gearsLocationProviderUrls);
+ assertObjectEqual(correctOptions, parsedOptions);
// Empty options.
parsedOptions = internalTests.testParseGeolocationOptions(
dummyFunction, dummyFunction, {});
- assertEqual(false, parsedOptions.repeats);
- assertEqual(false, parsedOptions.enableHighAccuracy);
- assertEqual(false, parsedOptions.gearsRequestAddress);
- assertEqual('', parsedOptions.gearsAddressLanguage);
- assertArrayEqual(defaultUrlArray, parsedOptions.gearsLocationProviderUrls);
+ assertObjectEqual(correctOptions, parsedOptions);
// Empty provider URLs.
parsedOptions = internalTests.testParseGeolocationOptions(
dummyFunction, dummyFunction, {gearsLocationProviderUrls: []});
- assertEqual(false, parsedOptions.repeats);
- assertEqual(false, parsedOptions.enableHighAccuracy);
- assertEqual(false, parsedOptions.gearsRequestAddress);
- assertEqual('', parsedOptions.gearsAddressLanguage);
- assertArrayEqual([], parsedOptions.gearsLocationProviderUrls);
+ correctOptions.gearsLocationProviderUrls = [];
+ assertObjectEqual(correctOptions, parsedOptions);
// Null provider URLs.
parsedOptions = internalTests.testParseGeolocationOptions(
dummyFunction, dummyFunction, {gearsLocationProviderUrls: null});
- assertEqual(false, parsedOptions.repeats);
- assertEqual(false, parsedOptions.enableHighAccuracy);
- assertEqual(false, parsedOptions.gearsRequestAddress);
- assertEqual('', parsedOptions.gearsAddressLanguage);
- assertArrayEqual([], parsedOptions.gearsLocationProviderUrls);
+ assertObjectEqual(correctOptions, parsedOptions);
// All properties false, provider URLs set.
parsedOptions = internalTests.testParseGeolocationOptions(
@@ -86,11 +79,8 @@
gearsAddressLanguage: '',
gearsLocationProviderUrls: urls
});
- assertEqual(false, parsedOptions.repeats);
- assertEqual(false, parsedOptions.enableHighAccuracy);
- assertEqual(false, parsedOptions.gearsRequestAddress);
- assertEqual('', parsedOptions.gearsAddressLanguage);
- assertArrayEqual(urls, parsedOptions.gearsLocationProviderUrls);
+ correctOptions.gearsLocationProviderUrls = urls;
+ assertObjectEqual(correctOptions, parsedOptions);
// All properties true, provider URLs set.
parsedOptions = internalTests.testParseGeolocationOptions(
@@ -102,11 +92,11 @@
gearsAddressLanguage: 'test',
gearsLocationProviderUrls: urls
});
- assertEqual(false, parsedOptions.repeats);
- assertEqual(true, parsedOptions.enableHighAccuracy);
- assertEqual(true, parsedOptions.gearsRequestAddress);
- assertEqual('test', parsedOptions.gearsAddressLanguage);
- assertArrayEqual(urls, parsedOptions.gearsLocationProviderUrls);
+ correctOptions.enableHighAccuracy = true;
+ correctOptions.gearsRequestAddress = true;
+ correctOptions.gearsAddressLanguage = 'test';
+ correctOptions.gearsLocationProviderUrls = urls;
+ assertObjectEqual(correctOptions, parsedOptions);
}
}
@@ -558,3 +548,30 @@
}
}
}
+
+function testTimeout() {
+ // A zero timeout is tested in testZeroTimeout() in geolocation_tests.js.
Here
+ // we test that the success callback is invoked if a non-zero timeout is
used.
+ // We use the mock location provider to do this.
+ if (isUsingCCTests) {
+ var options = {
+ timeout: 1000, // Should be plenty of time for the mock provider to act.
+ gearsLocationProviderUrls: []
+ };
+ var mockPosition = {
+ latitude: 51.0,
+ longitude: -0.1,
+ accuracy: 100.1
+ };
+
internalTests.configureGeolocationMockLocationProviderForTest(mockPosition);
+ function locationAvailable(position) {
+ delete position.timestamp;
+ assertObjectEqual(mockPosition, position);
+ internalTests.removeGeolocationMockLocationProvider();
+ completeAsync();
+ };
+ var geolocation = google.gears.factory.create('beta.geolocation');
+ startAsync();
+ geolocation.getCurrentPosition(locationAvailable, null, options);
+ }
+}
