Fix DeviceStatusCollectorTest.MaxStoredPeriods.

chrome/browser/policy/device_status_collector.cc

Index: chrome/browser/policy/device_status_collector.cc
diff --git a/chrome/browser/policy/device_status_collector.cc b/chrome/browser/policy/device_status_collector.cc
index 4505c5546a1af6816a6d0c9b242be129c0476d7a..151b1c59b17e143ec622fd56791a2ca63b4e3dc5 100644
--- a/chrome/browser/policy/device_status_collector.cc
+++ b/chrome/browser/policy/device_status_collector.cc
@@ -93,7 +93,18 @@ DeviceStatusCollector::DeviceStatusCollector(
       report_version_info_(false),
       report_activity_times_(false),
       report_boot_mode_(false),
-      report_location_(false) {
+      report_location_(false) {}
+
+DeviceStatusCollector::~DeviceStatusCollector() {
+  cros_settings_->RemoveSettingsObserver(chromeos::kReportDeviceVersionInfo,
+                                         this);
+  cros_settings_->RemoveSettingsObserver(chromeos::kReportDeviceActivityTimes,
+                                         this);
+  cros_settings_->RemoveSettingsObserver(chromeos::kReportDeviceBootMode, this);
+  cros_settings_->RemoveSettingsObserver(chromeos::kReportDeviceLocation, this);
+}
+
+void DeviceStatusCollector::Init() {
   if (!location_update_requester_)
     location_update_requester_ = &content::RequestLocationUpdate;
   idle_poll_timer_.Start(FROM_HERE,
@@ -144,15 +155,6 @@ DeviceStatusCollector::DeviceStatusCollector(
       &tracker_);
 }
 
-DeviceStatusCollector::~DeviceStatusCollector() {
-  cros_settings_->RemoveSettingsObserver(chromeos::kReportDeviceVersionInfo,
-                                         this);
-  cros_settings_->RemoveSettingsObserver(chromeos::kReportDeviceActivityTimes,
-                                         this);
-  cros_settings_->RemoveSettingsObserver(chromeos::kReportDeviceBootMode, this);
-  cros_settings_->RemoveSettingsObserver(chromeos::kReportDeviceLocation, this);
-}
-
 // static
 void DeviceStatusCollector::RegisterPrefs(PrefService* local_state) {
   local_state->RegisterDictionaryPref(prefs::kDeviceActivityTimes,
@@ -269,11 +271,12 @@ void DeviceStatusCollector::IdleStateCallback(IdleState state) {
     // interval of activity.
     int active_seconds = (now - last_idle_check_).InSeconds();
     if (active_seconds < 0 ||
-        active_seconds >= static_cast<int>((2 * kIdlePollIntervalSeconds)))
+        active_seconds >= static_cast<int>((2 * kIdlePollIntervalSeconds))) {
       AddActivePeriod(now - TimeDelta::FromSeconds(kIdlePollIntervalSeconds),
                       now);
-    else
+    } else {
       AddActivePeriod(last_idle_check_, now);
+    }
 
     PruneStoredActivityPeriods(now);
   }
@@ -390,7 +393,7 @@ void DeviceStatusCollector::ScheduleGeolocationUpdateRequest() {
     return;
 
   if (position_.Validate()) {
-    TimeDelta elapsed = Time::Now() - position_.timestamp;
+    TimeDelta elapsed = GetCurrentTime() - position_.timestamp;

[jar (doing other things), 2012/11/16 20:29:23]

The old code used Time::Now(), which is not a monotonic time source (it can jump
when we go in/out of daylight savings, and it jumps when a user "fixes" their
clock).

Is this the same flavor of time?  I wasn't sure what was being called... but it
seems to mask the probable issue.  

Most commonly, we use base::TimeTicks, which is guaranteed to be monotonically
increasing.... but I wasn't sure where postion_.timestamp came from.

[Ryan Sleevi, 2012/11/16 22:20:48]

I have to agree with Jim here. I think conflating the timestamp received from
the Geolocation source (line 438?) with the system time might be a mistake. It
seems much more reasonable to use base::TimeTicks to manage the polling interval
- to keep it at a true 30 minute interval - and then include in the report the
timesource according to GPS (presumably, a UTC timestamp)

Just a $.02 drive by

[Joao da Silva, 2012/11/20 09:10:52]

It's the same; GetCurrentTime() is a virtual that returns Time::Now() but can be
overridden in tests.
Those are good points. I'm not familiar with this code, adding @bartfab to
comment on that.

     TimeDelta interval =
         TimeDelta::FromSeconds(kGeolocationPollIntervalSeconds);
     if (elapsed > interval) {