Date and Time dialog for when the clock isn't synced.

chrome/browser/resources/chromeos/set_time.js

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+cr.define('setTime', function() {

[Dan Beam, 2014/04/23 17:51:45]

nit: "namespaces" are all lowercase, e.g. settime

[michaelpg, 2014/04/24 01:32:23]

Done.

+  /**
+   * TimeSetter class

[Dan Beam, 2014/04/23 17:51:45]

^ better description of what this does or how it works

[michaelpg, 2014/04/24 01:32:23]

Done.

+   * @class

[Dan Beam, 2014/04/23 17:51:45]

s/@class/@constructor

[michaelpg, 2014/04/24 01:32:23]

Done.

+   */
+

[Dan Beam, 2014/04/23 17:51:45]

remove blank line

[michaelpg, 2014/04/24 01:32:23]

Done.

+  function TimeSetter() {}
+
+  cr.addSingletonGetter(TimeSetter);
+
+  var BODY_PADDING_PX = 20;

[Dan Beam, 2014/04/23 17:51:45]

/** @const */ var BODY_PADDING_PX (as well as on LABEL_PADDING_PX)

[michaelpg, 2014/04/24 01:32:23]

Done.

+  var LABEL_PADDING_PX = 5;
+
+  TimeSetter.prototype = {
+    /**
+     * Performs initial setup.
+     */
+    initialize: function() {
+      // Store values for reverting inputs when the user's date/time is invalid.
+      this.prevValues_ = {};
+
+      // The build time doesn't include a timezone, so subtract 1 day to get a
+      // safe minimum date.
+      this.minDate_ = new Date(loadTimeData.getValue('buildTime'));
+      this.minDate_.setDate(this.minDate_.getDate() - 1);
+
+      // Set the max date to the min date plus 20 years.
+      this.maxDate_ = new Date(this.minDate_);
+      this.maxDate_.setYear(this.minDate_.getFullYear() + 20);

[Dan Beam, 2014/04/23 17:51:45]

what happens at 2018-01-19 03:14:07 UTC?

[michaelpg, 2014/04/24 01:32:23]

tlsdate accepts times up to 20 years after the build time. I've asked about this
but haven't heard back. I'll see if I can get an explanation.

But to answer your question, Skynet goes live.

[Dan Beam, 2014/04/24 21:32:23]

i assume you mean the year changes to 1970 because Date overflows?

[michaelpg, 2014/04/24 22:42:47]

No, Date doesn't overflow. The message will be passed correctly to tlsdate,
which will refuse to set the time because it's more than 20 years from now.
Since the system time can't be set past this time, it makes sense to set this
time as the max for the input field.

[michaelpg, 2014/04/25 20:05:14]

I've suggested a hard cutoff to drewry@. He liked the idea, so this will be
updated when tlsdate is updated.

The cutoff is 12/31/2179. 2180 K is the melting point of Chromium.

+
+      // Make sure the ostensible date is within this range.
+      var now = new Date(Date.now());

[Dan Beam, 2014/04/23 17:51:45]

this is the same as

  var now = new Date();

[michaelpg, 2014/04/24 01:32:23]

sadly, I know this...

+      if (now > this.maxDate_)
+        this.maxDate_ = now;
+      else if (now < this.minDate_)
+        this.minDate_ = now;
+
+      $('date').setAttribute('min', this.htmlDate_(this.minDate_));
+      $('date').setAttribute('max', this.htmlDate_(this.maxDate_));
+
+      this.updateTime_();
+
+      // Show the timezone select if we have a timezone ID.
+      var currentTimezoneId = loadTimeData.getValue('currentTimezoneId');
+      if (currentTimezoneId !== '') {
+        this.setTimezone_(currentTimezoneId);
+        $('timezone-select').addEventListener(
+            'change', this.onTimezoneChange_.bind(this), false);
+        $('timezone').hidden = false;
+      }
+
+      this.sizeToFit_();
+
+      $('time').addEventListener('blur', this.onTimeBlur_.bind(this), false);
+      $('date').addEventListener('blur', this.onTimeBlur_.bind(this), false);
+
+      $('done').addEventListener('click', this.onSubmit_.bind(this), false);
+    },
+
+    /**
+     * Sets the current timezone.
+     * @param {string} timezoneId The timezone ID to select.

[Dan Beam, 2014/04/23 17:51:45]

@private

[michaelpg, 2014/04/24 01:32:23]

Done.

+     */
+    setTimezone_: function(timezoneId) {
+      $('timezone-select').value = timezoneId;
+      this.updateTime_();
+    },
+
+    /**
+     * Updates the date/time controls to the current local time.
+     * Called initially, then called again once a minute.
+     * @private
+     */
+    updateTime_: function() {
+      var now = new Date();
+
+      // Only update time controls if neither is focused.
+      if (document.activeElement.id != 'date' &&

[Dan Beam, 2014/04/23 17:51:45]

can |document.activeElement| ever be null?

[michaelpg, 2014/04/24 01:32:23]

According to the w3c, if there is no focused element it returns the body
element, which is non-null because we have document.body. In my testing it has
acted accordingly. 

+          document.activeElement.id != 'time') {
+        $('time').value = this.htmlTime_(now);
+        this.prevValues_['time'] = $('time').value;
+
+        $('date').value = this.htmlDate_(now);
+        this.prevValues_['date'] = $('date').value;

[Dan Beam, 2014/04/23 17:51:45]

this.prevValues_.time = $('time').value = this.htmlTime_(now);
this.prevValues_.date = $('date').value = this.htmlDate_(now);

[michaelpg, 2014/04/24 01:32:23]

Done.

+      }
+
+      window.clearTimeout(this.timeTimeout_);
+
+      // Start timer to update these inputs every minute.
+      var secondsRemaining = 60 - now.getSeconds();  // 0 <= getSeconds() < 60
+      this.timeTimeout_ = window.setTimeout(this.updateTime_.bind(this),
+                                            secondsRemaining * 1000);

[Dan Beam, 2014/04/23 17:51:45]

use setInterval instead

[michaelpg, 2014/04/24 01:32:23]

no. I want this to be called right when the seconds are 0+epsilon so that when
the minute changes in the status area clock, the minute changes here at the same
time. This function can be called at arbitrary times (e.g. when the timezone
changes) whereas I'd like to ensure the callback happens every minute on the
dot. Thus the secondsRemaining calculation.

+    },
+
+    /**
+     * Attempts to set the system time to the time given by the controls.
+     * @private
+     */
+    applyTime_: function() {
+      var date = $('date').valueAsDate;

[Dan Beam, 2014/04/23 17:51:45]

these valueAs*() getters should probably be functions instead if they're
non-trivial

[michaelpg, 2014/04/24 01:32:23]

it's a build-it attribute of the date/time elements

+      date.setMilliseconds(date.getMilliseconds() + $('time').valueAsNumber);
+
+      // Add timezone offset to get real time.
+      date.setMinutes(date.getMinutes() + date.getTimezoneOffset());
+
+      var seconds = Math.floor(date.valueOf() / 1000);

[Dan Beam, 2014/04/23 17:51:45]

i don't think you need .valueOf() here, just dividing should be enough of a
contextual clue

[michaelpg, 2014/04/24 01:32:23]

Done.

+      chrome.send('setTimeInSeconds', [seconds]);
+    },
+
+    /**
+     * Handles events for the date/time controls.

[Dan Beam, 2014/04/23 17:51:45]

@param {Event} e The blur event.

[michaelpg, 2014/04/24 01:32:23]

Done.

+     * @private
+     */
+    onTimeBlur_: function(e) {
+      if (e.type != 'blur')
+        return;

[Dan Beam, 2014/04/23 17:51:45]

I'm confused, why we would ever hit this?

[michaelpg, 2014/04/24 01:32:23]

We wouldn't, this code is vestigial.

[Dan Beam, 2014/04/24 21:32:23]

don't write vestigal code ;)

+      if (e.target.validity.valid) {
+        // Make this the new fallback time in case of future invalid input.
+        this.prevValues_[e.target.id] = e.target.value;
+        this.applyTime_();
+      } else {
+        // Restore previous value.
+        e.target.value = this.prevValues_[e.target.id];
+      }
+    },
+
+    /**
+     * Handles events for the timezone control.

[Dan Beam, 2014/04/23 17:51:45]

needs a @param

[michaelpg, 2014/04/24 01:32:23]

Done.

+     * @private
+     */
+    onTimezoneChange_: function(e) {
+      chrome.send('setTimezone', [e.target.value]);

[Dan Beam, 2014/04/23 17:51:45]

nit: e.currentTarget

[michaelpg, 2014/04/24 01:32:23]

Not sure I follow. is that to make sure we get the value of the select, not the
option element?

[Dan Beam, 2014/04/24 21:32:23]

this is to make your code impervious to event delegation

[michaelpg, 2014/04/24 22:42:47]

Done.

+    },
+
+    /**
+     * Handles events for the submit button.

[Dan Beam, 2014/04/23 17:51:45]

needs a @param

[michaelpg, 2014/04/24 01:32:23]

Done.

+     * @private
+     */
+    onSubmit_: function(e) {
+      chrome.send('DialogClose');

[Dan Beam, 2014/04/23 17:51:45]

s/DialogClose/dialogClose/

[michaelpg, 2014/04/24 01:32:23]

I know, but no.

https://code.google.com/p/chromium/codesearch#search/&q=%5C%22dialogclose%5C%...

[Dan Beam, 2014/04/24 21:32:23]

rename it

[michaelpg, 2014/04/24 22:42:47]

OK. Since that'll require changing 10 more files and grabbing more owners, do
you mind if I submit the change as a separate CL?

[Dan Beam, 2014/04/25 00:05:48]

https://chromiumcodereview.appspot.com/251623003/

+    },
+
+    /**
+     * Resizes the window if necessary to show the entire contents.
+     * @private
+     */
+    sizeToFit_: function() {
+      var newWidth = window.outerWidth;
+      var newHeight = window.outerHeight;
+
+      // Because of l10n, we should check if the window can fit the
+      // timezone select.
+      var timezoneWidth = $('timezone-select').offsetWidth;
+      var labelWidth = $('timezone-label').offsetWidth;
+
+      var minWidth = 2 * BODY_PADDING_PX + labelWidth +
+                     LABEL_PADDING_PX + timezoneWidth;
+      if (window.innerWidth < minWidth) {
+        // Apply the width to the body now so the next calculation is correct.
+        document.body.style.width = (minWidth - 2 * BODY_PADDING_PX) + 'px';
+        newWidth = minWidth;
+      }
+
+      // After resizing horizontally, make sure the page will fit vertically.
+      var minHeight = document.body.scrollHeight;
+      if (window.innerHeight < minHeight) {
+        newHeight = minHeight +
+                    window.outerHeight - window.innerHeight;  // title bar
+      }
+
+      if (newWidth != window.outerWidth || newHeight != window.outerHeight) {
+        // Move before resize for a less jumpy transition.
+        window.moveTo(window.screen.width / 2 - newWidth / 2,
+                      window.screen.height / 2 - newHeight / 2);
+        window.resizeTo(newWidth, newHeight);
+      }
+    },
+
+    /**
+     * Builds a time string as "HH:MM".
+     * @param {Date} date The time to represent.

[Dan Beam, 2014/04/23 17:51:45]

@return {string} A date suitable for representing in HTML.

(or something like that)

[michaelpg, 2014/04/24 01:32:23]

Done.

+     * @private
+     */
+    htmlTime_: function(date) {
+      // Get the current time and subtract the timezone offset, so the
+      // JSON string is in local time.
+      var localDate = new Date(date);
+      localDate.setMinutes(date.getMinutes() - date.getTimezoneOffset());
+      return localDate.toJSON().slice(11, 16);

[Dan Beam, 2014/04/23 17:51:45]

s/toJSON/toISOString/

[michaelpg, 2014/04/24 01:32:23]

Done.

+    },
+
+    /**
+     * Builds a full-date string as defined in RFC 3339.
+     * @param {Date} date The date to represent.

[Dan Beam, 2014/04/23 17:51:45]

needs @return

[michaelpg, 2014/04/24 01:32:23]

Done.

+     * @private
+     */
+    htmlDate_: function(date) {

[Dan Beam, 2014/04/23 17:51:45]

seems like htmlDate_ and htmlTime_ share some code and are almost always called
together -- maybe .toHtml() => {date: <date_string>, time: <time_string>} ?

[michaelpg, 2014/04/24 01:32:23]

Done.

+      // Get the current time and subtract the timezone offset, so the
+      // JSON string is in local time.
+      var localDate = new Date(date);
+      localDate.setMinutes(date.getMinutes() - date.getTimezoneOffset());
+      return localDate.toJSON().slice(0, 10);

[Dan Beam, 2014/04/23 17:51:45]

toISOString

[michaelpg, 2014/04/24 01:32:23]

Done.

+    }
+  };
+
+  // Forward public APIs to private implementations.
+  [
+    'setTimezone',
+  ].forEach(function(name) {
+    TimeSetter[name] = function() {
+      var instance = TimeSetter.getInstance();
+      return instance[name + '_'].apply(instance, arguments);
+    };
+  });

[Dan Beam, 2014/04/23 17:51:45]

meh, wait until there's more than 1 of these, IMO, and for now change L210-218
to just:

  TimeSetter.setTimezone = function(timezoneId) {
    TimeSetter.getInstance().setTimezone_(timezoneId);
  };

[michaelpg, 2014/04/24 01:32:23]

Done.

+
+  return {
+    TimeSetter: TimeSetter
+  };
+});
+
+document.addEventListener(
+    'DOMContentLoaded',
+    setTime.TimeSetter.getInstance().initialize.bind(
+        setTime.TimeSetter.getInstance()));