Closed Bug 848004 Opened 7 years ago Closed 6 years ago

[System] if the system fails to set the time, the default time should be set to the build time

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:-)

RESOLVED FIXED
1.4 S1 (14feb)
blocking-b2g -

People

(Reporter: nhirata, Assigned: mwu)

References

Details

Attachments

(1 file)

At times, the time from the flash script will fail.
When it does the default time is set to a date with 1980.
This causes an issue with failed certificates.  Also see bug 826890

Please set the default system time to 1/1/2013.

We should probably not have the system be able to be set earlier than 1/1/2013?
affects the initial flashing of the phone.
Whiteboard: NPOTB
(clearing tef?, not a mozilla deliverable)
blocking-b2g: tef? → -
This sets a somewhat sane default time when we have nothing else. This will prevent us from unnecessarily reporting that we're in 1970. It checks the reported build time and the last modified time of /system/b2g/b2g, and sets the time accordingly if our time is implausible.
Assignee: nobody → mwu
Attachment #8377243 - Flags: review?(gene.lian)
Summary: [System] if the system fails to set the time, the default time should be set to Jan 1, 2013 → [System] if the system fails to set the time, the default time should be set to the build time
Comment on attachment 8377243 [details] [diff] [review]
Set a minimum time based on build time

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

Hi Michael, could you please clarify me for the questions below? Thanks!

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +3275,5 @@
>              // Or refresh the SNTP.
>              this._sntp.request();
>            }
> +        } else {
> +          // Set a sane minimum time

Nit: please add a period at the end of the comment.

@@ +3276,5 @@
>              this._sntp.request();
>            }
> +        } else {
> +          // Set a sane minimum time
> +          let mintime = libcutils.property_get("ro.build.date.utc", "0") * 1000;

Nit: I don't want to be picky but I'd prefer s/mintime/minTime (and for the codes below) because they are actually two words. ;)

Btw, can we just use |buildTime|? |minTime| sounds a bit ambiguous.

@@ +3279,5 @@
> +          // Set a sane minimum time
> +          let mintime = libcutils.property_get("ro.build.date.utc", "0") * 1000;
> +          let file = FileUtils.File("/system/b2g/b2g");
> +          if (file.lastModifiedTime > mintime) {
> +              mintime = file.lastModifiedTime;

Nit: please use 2-space indent.

@@ +3282,5 @@
> +          if (file.lastModifiedTime > mintime) {
> +              mintime = file.lastModifiedTime;
> +          }
> +          if (mintime > Date.now()) {
> +              gTimeService.set(mintime);

Nit: please use 2-space indent.

So, minTime can be bigger than now time? How could it happen? The time we built for B2G should always be earlier than the current time when users use B2G. Right?

Another concern is: this setting |kSettingsClockAutoUpdateEnabled| is aimed to set the system clock automatically, which must reflect the current time in reality. If the time we're going to adjust is not accurate when users turn on the auto time update, they would complain the function is broken. Is the minimum time really what we want to reflect?
Attachment #8377243 - Flags: review?(gene.lian)
(In reply to Gene Lian [:gene] (needinfo? encouraged) from comment #4)
> @@ +3276,5 @@
> >              this._sntp.request();
> >            }
> > +        } else {
> > +          // Set a sane minimum time
> > +          let mintime = libcutils.property_get("ro.build.date.utc", "0") * 1000;
> 
> Nit: I don't want to be picky but I'd prefer s/mintime/minTime (and for the
> codes below) because they are actually two words. ;)
> 
> Btw, can we just use |buildTime|? |minTime| sounds a bit ambiguous.
> 

Nope, because it's not necessarily the build time. It's a minimum plausible time.

> So, minTime can be bigger than now time? How could it happen? The time we
> built for B2G should always be earlier than the current time when users use
> B2G. Right?
> 

That is the *whole* point of the patch. If your battery falls out, the time will be zero or near zero. This patch uses some heuristics to determine what a good "starting" time is before we can connect to the network and get a correct time.

> Another concern is: this setting |kSettingsClockAutoUpdateEnabled| is aimed
> to set the system clock automatically, which must reflect the current time
> in reality. If the time we're going to adjust is not accurate when users
> turn on the auto time update, they would complain the function is broken. Is
> the minimum time really what we want to reflect?

It is less broken than reporting that it is 1970, and breaks fewer https sites.
Whiteboard: NPOTB
Comment on attachment 8377243 [details] [diff] [review]
Set a minimum time based on build time

I'll fix any indentation/grammar/capitalization nits on checkin.
Attachment #8377243 - Flags: review?(gene.lian)
Attachment #8377243 - Flags: review?(gene.lian) → review+
https://hg.mozilla.org/integration/b2g-inbound/rev/503ddb7e125e

I ended up also changing minTime to buildTime. In theory, we do want to calculate the minimum plausible time by also considering other sources like the modified time of files in the profile. In practice, I think just trying to get the latest build time is enough, so let's just call it what it is until we start checking other timestamps.
https://hg.mozilla.org/mozilla-central/rev/503ddb7e125e
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S1 (14feb)
You need to log in before you can comment on or make changes to this bug.