Closed Bug 809954 Opened 7 years ago Closed 7 years ago

Handle forward skewed clocks properly

Categories

(Firefox Health Report Graveyard :: Client: Desktop, defect)

defect
Not set

Tracking

(firefox18 fixed, firefox19 fixed, firefox20 fixed)

RESOLVED FIXED
Firefox 20
Tracking Status
firefox18 --- fixed
firefox19 --- fixed
firefox20 --- fixed

People

(Reporter: gps, Assigned: gps)

References

Details

Attachments

(1 file)

mreid found an issue with the policy's handling of forward-skewed clocks.

Essentially, if the system clock is set to something way out in the future (say 2020) then readjusted to sanity, our nextScheduledDate will remain in the future.

We should detect when the nextScheduledDate is way ahead of "now" and readjust it.
Pretty straightforward.
Assignee: nobody → gps
Status: NEW → ASSIGNED
Attachment #679906 - Flags: review?(rnewman)
Comment on attachment 679906 [details] [diff] [review]
Handle future dates, v1

Review of attachment 679906 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with boundary tests.

::: services/healthreport/policy.jsm
@@ +645,5 @@
> +    // If the system clock were ever set to a time in the distant future,
> +    // it's possible our next schedule date is far out as well. We know
> +    // we shouldn't schedule for more than a day out, so we reset the next
> +    // scheduled date appropriately.
> +    if (nextSubmissionDate.getTime() > nowT + 3 * MILLISECONDS_PER_DAY) {

Why 3?

::: services/healthreport/tests/xpcshell/test_policy.js
@@ +364,5 @@
> +  policy.recordUserAcceptance();
> +  now = new Date();
> +  defineNow(policy, now);
> +
> +  policy.nextDataSubmissionDate = new Date(Date.now() + 365 * 24 * 60 * 60 * 1000);

Boundary testing, man, boundary testing! I want to see a test for the limit valid interval and the limit invalid interval.
Attachment #679906 - Flags: review?(rnewman) → review+
https://hg.mozilla.org/projects/larch/rev/7614f36ddf90

With boundary tests and comment explaining 3 days satisfies the 3 bears test.
Whiteboard: [fixed-in-larch]
Comment on attachment 679906 [details] [diff] [review]
Handle future dates, v1

Bulk-setting approval flags for FHR landing for FxOS ADU ping (Bug 788894).
Attachment #679906 - Flags: approval-mozilla-beta?
Attachment #679906 - Flags: approval-mozilla-aurora?
Target Milestone: --- → mozilla20
https://hg.mozilla.org/mozilla-central/rev/6b2eb103766a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment on attachment 679906 [details] [diff] [review]
Handle future dates, v1

FHR for B2G ADU ping, won't be built/enabled for Mobile/Desktop.
Attachment #679906 - Flags: approval-mozilla-beta?
Attachment #679906 - Flags: approval-mozilla-beta+
Attachment #679906 - Flags: approval-mozilla-aurora?
Attachment #679906 - Flags: approval-mozilla-aurora+
Component: Metrics and Firefox Health Report → Client: Desktop
Flags: in-testsuite+
Product: Mozilla Services → Firefox Health Report
Target Milestone: mozilla20 → Firefox 20
Product: Firefox Health Report → Firefox Health Report Graveyard
You need to log in before you can comment on or make changes to this bug.