Experimenting with the first run experience and the navigator.mozTime interface I found out that the new date does not persist after a restart of the device.

Modified AdjustSystemClock(), set time using alarm driver.

Comment on attachment 663974 [details] [diff] [review] Patch, set to alarm rtc Review of attachment 663974 [details] [diff] [review]: ----------------------------------------------------------------- ::: hal/gonk/GonkHal.cpp @@ +639,5 @@ > now.tv_nsec += NsecPerSec; > now.tv_sec -= 1; > } > +#ifdef SUPPORT_ANDROID_ALARM_RTC > + int fd; Would be best to just use ScopedClose here. @@ +642,5 @@ > +#ifdef SUPPORT_ANDROID_ALARM_RTC > + int fd; > + int res; > + > + errno = 0; We only check errno if there's an error, so I don't think there's a need to clear it.. @@ +644,5 @@ > + int res; > + > + errno = 0; > + fd = open("/dev/alarm", O_RDWR); > + if(fd < 0) { Space after if. @@ +650,5 @@ > + return; > + } > + errno = 0; > + res = ioctl(fd, ANDROID_ALARM_SET_RTC, &now); > + if(res < 0) { We can just inline the ioctl call into the if check.

Comment on attachment 663974 [details] [diff] [review] Patch, set to alarm rtc We're in a gonk-specific file, so no reason to add the ifdef here. Let's just change the implementation. I think we also need to handle EINTR from open() and ioctl() here.

> I think we also need to handle EINTR from open() and ioctl() here. Hi Chris, Thanks for your comments. I have two questions here. How many times do we need to retry when we get EINTR calling open() ? Do we need to retry forever ? Or just one or two times is enough ? When I checked the ioctl() with "man 2 ioctl", I didn't find EINTR in ERRORS list. I was wondering if we have to handle EINTR calling ioctl() ?

(In reply to Vincent Chang from comment #4) > > I think we also need to handle EINTR from open() and ioctl() here. > Hi Chris, > > Thanks for your comments. I have two questions here. > > How many times do we need to retry when we get EINTR calling open() ? Do we > need to retry forever ? Or just one or two times is enough ? > EINTR means a signal interrupted the call, so you can retry indefinitely. The usual pattern is int fd; do { fd = open(...); } while (fd == -1 && errno == EINTR); > When I checked the ioctl() with "man 2 ioctl", I didn't find EINTR in ERRORS > list. I was wondering if we have to handle EINTR calling ioctl() ? Ah, you're correct. If the man page says no, then no :).

1. Addressed the change that argument type of AdjustSystemClock(aDeltaMilliseconds) is modified from int32 to int64. 2. Addressed comments 2,3,5.

Comment on attachment 665277 [details] [diff] [review] Patch, set system time using alarm rtc driver, v1.1 Review of attachment 665277 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the log messages changed. ::: hal/gonk/GonkHal.cpp @@ +632,5 @@ > + fd = open("/dev/alarm", O_RDWR); > + } while (fd == -1 && errno == EINTR); > + ScopedClose autoClose(fd); > + if (fd < 0) { > + HAL_LOG(("Unable to open alarm driver: %s\n", strerror(errno))); Let's not copy the android message if possible. "Failed to open /dev/alarm: %s" @@ +637,5 @@ > + return; > + } > + > + if (ioctl(fd, ANDROID_ALARM_SET_RTC, &now) < 0) { > + HAL_LOG(("Unable to set rtc to %ld: %s\n", now.tv_sec, strerror(errno))); "ANDROID_ALARM_SET_RTC failed: %s"

Hi Michael, Thanks for reviewing this patch. I updated the patch to address your comments.

https://hg.mozilla.org/integration/mozilla-inbound/rev/c0ab627c99dd Should this have a test?

> Should this have a test? The bug involves restarting the device? Also, it's a B2G-specific bug and we currently do not have any unit tests for B2G on Tinderbox.