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

chrome/browser/resources/options/browser_options.js

 // Copyright (c) 2012 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('options', function() {
   var OptionsPage = options.OptionsPage;
   var ArrayDataModel = cr.ui.ArrayDataModel;
   var RepeatingButton = cr.ui.RepeatingButton;
   var HotwordSearchSettingIndicator = options.HotwordSearchSettingIndicator;
 
@@ skipping 277 matching lines @@
           chrome.send('coreOptionsUserMetricsAction', ['Import_ShowDlg']);
         };
 
         if ($('themes-native-button')) {
           $('themes-native-button').onclick = function(event) {
             chrome.send('themesSetNative');
           };
         }
       }
 
+      // Date and time section (CrOS only).
+      if (cr.isChromeOS && $('set-time-button'))

[Dan Beam, 2014/04/24 21:54:33]

i still think this makes more sense to be based on whether the node exists.

at this layer, the code only cares that the node exists so it can wire up a
click handler (and not cause a null pointer exception by calling
$('set-time-button').doSomething()).

i also don't see a useful case where we'd include this node in the HTML (but
hidden, maybe) but then not use it... this just slows down the page and confuses
other engineers, I'd assume.

hence, why I'd ifdef out the code in HTML and use if ($('set-time-button')) here

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

Yep. The HTML is already ifdef'd out, so there's no reason to use cr.isChromeOS.
Done.

+        $('set-time-button').onclick = this.handleSetTime_.bind(this);
+
       // Default browser section.
       if (!cr.isChromeOS) {
         if (!loadTimeData.getBoolean('showSetDefault')) {
           $('set-default-browser-section').hidden = true;
         }
         $('set-as-default-browser').onclick = function(event) {
           chrome.send('becomeDefaultBrowser');
         };
 
         $('auto-launch').onclick = this.handleAutoLaunchChanged_;
@@ skipping 1412 matching lines @@
     /**
      * Shows (or not) the "User" section of the settings page based on whether
      * any of the sub-sections are present (or not).
      * @private
      */
     maybeShowUserSection_: function() {
       $('sync-users-section').hidden =
           $('profiles-section').hidden &&
           $('sync-section').hidden &&
           $('profiles-supervised-dashboard-tip').hidden;
+    },
+
+    /**
+     * Updates the date and time section with time sync information.
+     * @param {boolean} canSetTime Whether the system time can be set.
+     * @private
+     */
+    toggleCanSetTime_: function(canSetTime) {

[Dan Beam, 2014/04/24 21:54:33]

if this were a toggle function, it'd look something like this:

  $('time-synced-explanation').hidden = !$('time-synced-explanation').hidden;
  $('set-time').hidden = !$('set-time').hidden;

why not use set*?

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

Done.

+      // If the time has been network-synced, it cannot be set manually.
+      $('time-synced-explanation').hidden = canSetTime;
+      $('set-time').hidden = !canSetTime;
+    },
+
+    /**
+     * Handle the 'set date and time' button click.
+     * @private
+     */
+    handleSetTime_: function() {
+      chrome.send('showSetTime');
     }

[Dan Beam, 2014/04/24 21:54:33]

nit: },

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

Done.

+
   };
 
   //Forward public APIs to private implementations.
   [
     'addBluetoothDevice',
     'enableCertificateButton',
     'enableFactoryResetSection',
     'getCurrentProfile',
     'getStartStopSyncButton',
     'hideBluetoothSettings',
@@ skipping 17 matching lines @@
     'setupProxySettingsSection',
     'showBluetoothSettings',
     'showCreateProfileError',
     'showCreateProfileSuccess',
     'showCreateProfileWarning',
     'showHotwordSection',
     'showManagedUserImportError',
     'showManagedUserImportSuccess',
     'showMouseControls',
     'showTouchpadControls',
+    'toggleCanSetTime',
     'toggleHomepageControlled',
     'toggleSearchEngineControlled',
     'toggleStartupPagesControlled',
     'updateAccountPicture',
     'updateAutoLaunchState',
     'updateDefaultBrowserState',
     'updateEasyUnlock',
     'updateManagesSupervisedUsers',
     'updateSearchEngines',
     'updateStartupPages',
@@ skipping 14 matching lines @@
     BrowserOptions.getLoggedInUsername = function() {
       return BrowserOptions.getInstance().username_;
     };
   }
 
   // Export
   return {
     BrowserOptions: BrowserOptions
   };
 });