Hello andreip,
I'd like you to do a code review. Please execute
g4 diff -c 9074203
or point your web browser to
http://mondrian/9074203
to review the following code:
Change 9074203 by [EMAIL PROTECTED] on 2008/11/19 19:06:22 *pending*
Adds a TimedMessage class for use in the Geolocation arbitrator.
This class uses TimedCallback and MessageService to post a message
after a specified time interval has elapsed. This change will simplify the code
paths in the arbitrator when support for PositionOptions.timeout is added.
R=andreip
[EMAIL PROTECTED]
DELTA=65 (47 added, 14 deleted, 4 changed)
OCL=9074203
Affected files ...
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#61
edit
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.h#27
edit
65 delta lines: 47 added, 14 deleted, 4 changed
Also consider running:
g4 lint -c 9074203
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 9074203 by [EMAIL PROTECTED] on 2008/11/19 19:06:22 *pending*
Adds a TimedMessage class for use in the Geolocation arbitrator.
This class uses TimedCallback and MessageService to post a message
after a specified time interval has elapsed. This change will simplify the code
paths in the arbitrator when support for PositionOptions.timeout is added.
Affected files ...
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#61
edit
... //depot/googleclient/gears/opensource/gears/geolocation/geolocation.h#27
edit
==== //depot/googleclient/gears/opensource/gears/geolocation/geolocation.cc#61
-
c:\MyDocs\Gears4/googleclient/gears/opensource/gears/geolocation/geolocation.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/geolocation.cc
2008-11-20 12:04:14.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/geolocation.cc
2008-11-20 11:54:01.000000000 +0000
@@ -75,6 +75,32 @@
static const int kLastRepeatingRequestId = kint32max; // Repeating IDs
positive
static const int kLastSingleRequestId = kint32min; // Single IDs negative
+
+TimedMessage::TimedMessage(int timeout_milliseconds,
+ const std::string16 &message_type,
+ NotificationData *notification_data)
+ : message_type_(message_type) {
+ assert(!message_type_.empty());
+ assert(notification_data);
+ timed_callback_.reset(
+ new TimedCallback(this, timeout_milliseconds, notification_data));
+}
+
+// TimedCallback::ListenerInterface implementation
+void TimedMessage::OnTimeout(TimedCallback *caller, void *user_data) {
+ assert(user_data);
+ assert(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
+ // object is deleted in the message handler.
+ NotificationData *notification_data =
+ reinterpret_cast<NotificationData*>(user_data);
+ MessageService::GetInstance()->NotifyObservers(message_type_.c_str(),
+ notification_data);
+}
+
+
// Data classes for use with MessageService.
class NotificationDataGeoBase : public NotificationData {
public:
@@ -387,22 +413,12 @@
const CallbackRequiredNotificationData *callback_required_data =
reinterpret_cast<const CallbackRequiredNotificationData*>(data);
- // Delete this callback timer.
+ // Delete this timer.
FixRequestInfo *fix_info = callback_required_data->fix_info;
assert(fix_info->success_callback_timer.get());
fix_info->success_callback_timer.reset();
MakeSuccessCallback(fix_info, fix_info->pending_position);
}
-}
-
-// TimedCallback::ListenerInterface implementation.
-void GearsGeolocation::OnTimeout(TimedCallback *caller, void *user_data) {
- assert(user_data);
- // Send a message to the JavaScriptThread to make the callback.
- FixRequestInfo *fix_info = reinterpret_cast<FixRequestInfo*>(user_data);
- MessageService::GetInstance()->NotifyObservers(
- kCallbackRequiredObserverTopic,
- new CallbackRequiredNotificationData(this, fix_info));
}
// JsEventHandlerInterface implementation.
@@ -1175,8 +1191,10 @@
assert(!fix_info->success_callback_timer.get());
fix_info->pending_position = position;
- fix_info->success_callback_timer.reset(
- new TimedCallback(this, timeout_milliseconds, fix_info));
+ fix_info->success_callback_timer.reset(new TimedMessage(
+ timeout_milliseconds,
+ kCallbackRequiredObserverTopic,
+ new CallbackRequiredNotificationData(this, fix_info)));
}
// Local functions
==== //depot/googleclient/gears/opensource/gears/geolocation/geolocation.h#27 -
c:\MyDocs\Gears4/googleclient/gears/opensource/gears/geolocation/geolocation.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/geolocation/geolocation.h
2008-11-20 12:04:14.000000000 +0000
+++ googleclient/gears/opensource/gears/geolocation/geolocation.h
2008-11-20 11:33:15.000000000 +0000
@@ -130,12 +130,30 @@
std::string16 error_message; // Human-readable error message
};
+// A wrapper around TimedCallback that posts a message to the MessageService
+// when the timer expires.
+class TimedMessage : public TimedCallback::ListenerInterface {
+ public:
+ TimedMessage(int timeout_milliseconds,
+ const std::string16 &message_type,
+ NotificationData *notification_data);
+ virtual ~TimedMessage() {}
+
+ // TimedCallback::ListenerInterface implementation
+ virtual void OnTimeout(TimedCallback *caller, void *user_data);
+
+ private:
+ scoped_ptr<TimedCallback> timed_callback_;
+ std::string16 message_type_;
+
+ DISALLOW_EVIL_CONSTRUCTORS(TimedMessage);
+};
+
// The principal class of the Geolocation API.
class GearsGeolocation
: public ModuleImplBaseClass,
public LocationProviderBase::ListenerInterface,
public MessageObserverInterface,
- public TimedCallback::ListenerInterface,
public JsEventHandlerInterface {
public:
#ifdef USING_CCTESTS
@@ -212,7 +230,7 @@
// since the epoch.
int64 last_success_callback_time;
// The timer used for a pending future success callback in a watch.
- linked_ptr<TimedCallback> success_callback_timer;
+ linked_ptr<TimedMessage> success_callback_timer;
// The position that will be used used for a pending future success
callback
// in a watch.
Position pending_position;
@@ -226,9 +244,6 @@
virtual void OnNotify(MessageService *service,
const char16 *topic,
const NotificationData *data);
-
- // TimedCallback::ListenerInterface implementation.
- virtual void OnTimeout(TimedCallback *caller, void *user_data);
// JsEventHandlerInterface implementation used to handle the 'JSEVENT_UNLOAD'
// event.
