CrOS Settings: Leverage browser proxy pattern in date_time_page.js

 - Place stray calls to chrome.send() and cr.sendWithPromise() in
   c/b/r/settings/chromeos/date_time_page/ behind a browser proxy.
 - Update date_time_page_tests.js to leverage TestBrowserProxy.
 - Remove any calls to the obsolete registerMessageCallback() test
   helper, as well as any setTimeout() calls in
   date_time_page_tests.js.

This cleanup also revealed a completely defunct test case
"set date and time button", where no code in the test was actually
executed (also fixed).

The motivation behind this is to remove usages of
registerMessageCallback() test helper in order to eventually fully
remove it.

Bug: 1148403,1147940
Change-Id: I786cb2485483c7477bd6fd746cd3f1a054be4d3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2532674Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826923}

chrome/browser/resources/settings/chromeos/date_time_page/BUILD.gn

@@ -18,6 +18,7 @@ js_type_check("closure_compile") {
 js_library("date_time_page") {
   deps = [
     ":date_time_types",
+    ":timezone_browser_proxy",
     ":timezone_selector",
     "..:deep_linking_behavior",
     "..:os_route",
@@ -40,6 +41,7 @@ js_library("timezone_browser_proxy") {
 
 js_library("timezone_selector") {
   deps = [
+    ":timezone_browser_proxy",
     "../../controls:settings_dropdown_menu",
     "../../prefs:prefs_behavior",
     "//ui/webui/resources/js:cr",
@@ -104,6 +106,7 @@ js_library("timezone_browser_proxy.m") {
 js_library("timezone_selector.m") {
   sources = [ "$root_gen_dir/chrome/browser/resources/settings/chromeos/date_time_page/timezone_selector.m.js" ]
   deps = [
+    ":timezone_browser_proxy.m",
     "../../controls:settings_dropdown_menu.m",
     "../../prefs:prefs_behavior.m",
     "../../prefs:prefs_types.m",
@@ -146,7 +149,7 @@ polymer_modulizer("date_time_page") {
   html_type = "dom-module"
   migrated_imports = os_settings_migrated_imports
   namespace_rewrites = os_settings_namespace_rewrites
-  auto_imports = os_settings_auto_imports
+  auto_imports = os_settings_auto_imports + [ "chrome/browser/resources/settings/chromeos/date_time_page/timezone_browser_proxy.html|TimeZoneBrowserProxyImpl,TimeZoneBrowserProxy" ]
 }
 
 polymer_modulizer("timezone_selector") {
@@ -155,7 +158,7 @@ polymer_modulizer("timezone_selector") {
   html_type = "dom-module"
   migrated_imports = os_settings_migrated_imports
   namespace_rewrites = os_settings_namespace_rewrites
-  auto_imports = os_settings_auto_imports
+  auto_imports = os_settings_auto_imports + [ "chrome/browser/resources/settings/chromeos/date_time_page/timezone_browser_proxy.html|TimeZoneBrowserProxyImpl,TimeZoneBrowserProxy" ]
 }
 
 polymer_modulizer("timezone_subpage") {
@@ -163,8 +166,7 @@ polymer_modulizer("timezone_subpage") {
   html_file = "timezone_subpage.html"
   html_type = "dom-module"
   migrated_imports = os_settings_migrated_imports
-  namespace_rewrites = os_settings_namespace_rewrites +
-                       [ "settings.TimeZoneBrowserProxy|TimeZoneBrowserProxy" ]
+  namespace_rewrites = os_settings_namespace_rewrites
   auto_imports = os_settings_auto_imports + [
                    "chrome/browser/resources/settings/chromeos/date_time_page/date_time_types.html|TimeZoneAutoDetectMethod",
                    "chrome/browser/resources/settings/chromeos/date_time_page/timezone_browser_proxy.html|TimeZoneBrowserProxyImpl,TimeZoneBrowserProxy",

chrome/browser/resources/settings/chromeos/date_time_page/date_time_page.html

@@ -15,6 +15,7 @@
 <link rel="import" href="../../settings_page/settings_subpage.html">
 <link rel="import" href="../../settings_shared_css.html">
 <link rel="import" href="date_time_types.html">
+<link rel="import" href="timezone_browser_proxy.html">
 <link rel="import" href="timezone_selector.html">
 <link rel="import" href="timezone_subpage.html">
 

chrome/browser/resources/settings/chromeos/date_time_page/date_time_page.js

@@ -85,11 +85,19 @@ Polymer({
     },
   },
 
+  /** @private {?settings.TimeZoneBrowserProxy} */
+  browserProxy_: null,
+
+  /** @override */
+  created() {
+    this.browserProxy_ = settings.TimeZoneBrowserProxyImpl.getInstance();
+  },
+
   /** @override */
   attached() {
     this.addWebUIListener(
         'can-set-date-time-changed', this.onCanSetDateTimeChanged_.bind(this));
-    chrome.send('dateTimePageReady');
+    this.browserProxy_.dateTimePageReady();
   },
 
   /**
@@ -115,7 +123,7 @@ Polymer({
 
   /** @private */
   onSetDateTimeTap_() {
-    chrome.send('showSetDateTimeUI');
+    this.browserProxy_.showSetDateTimeUI();
   },
 
   /**

chrome/browser/resources/settings/chromeos/date_time_page/timezone_browser_proxy.js

@@ -3,7 +3,7 @@
 // found in the LICENSE file.
 
 // clang-format off
-// #import {addSingletonGetter} from 'chrome://resources/js/cr.m.js';
+// #import {addSingletonGetter, sendWithPromise} from 'chrome://resources/js/cr.m.js';
 // clang-format on
 
 /** @fileoverview A helper object used by the time zone subpage page. */
@@ -12,6 +12,15 @@ cr.define('settings', function() {
   /* #export */ class TimeZoneBrowserProxy {
     /** Notifies C++ code to show parent access code verification view. */
     showParentAccessForTimeZone() {}
+
+    /** Notifies C++ code that the date_time page is ready. */
+    dateTimePageReady() {}
+
+    /** Notifies C++ code to show the chrome://set-time standalone dialog. */
+    showSetDateTimeUI() {}
+
+    /** @return {!Promise<!Array<!Array<string>>>} */
+    getTimeZones() {}
   }
 
   /** @implements {settings.TimeZoneBrowserProxy} */
@@ -20,6 +29,21 @@ cr.define('settings', function() {
     showParentAccessForTimeZone() {
       chrome.send('handleShowParentAccessForTimeZone');
     }
+
+    /** @override */
+    dateTimePageReady() {
+      chrome.send('dateTimePageReady');
+    }
+
+    /** @override */
+    showSetDateTimeUI() {
+      chrome.send('showSetDateTimeUI');
+    }
+
+    /** @override */
+    getTimeZones() {
+      return cr.sendWithPromise('getTimeZones');
+    }
   }
 
   cr.addSingletonGetter(TimeZoneBrowserProxyImpl);

chrome/browser/resources/settings/chromeos/date_time_page/timezone_selector.html

@@ -6,6 +6,7 @@
 <link rel="import" href="../../prefs/prefs_behavior.html">
 <link rel="import" href="../../prefs/prefs_types.html">
 <link rel="import" href="../../settings_shared_css.html">
+<link rel="import" href="timezone_browser_proxy.html">
 
 <dom-module id="timezone-selector">
   <template>

chrome/browser/resources/settings/chromeos/date_time_page/timezone_selector.js

@@ -108,8 +108,9 @@ Polymer({
     // Setting several preferences at once will trigger several
     // |maybeGetTimeZoneList_| calls, which we don't want.
     this.getTimeZonesRequestSent_ = true;
-    cr.sendWithPromise('getTimeZones')
-        .then((timezones) => {
+    settings.TimeZoneBrowserProxyImpl.getInstance()
+        .getTimeZones()
+        .then(timezones => {
           this.setTimeZoneList_(timezones);
         })
         .finally(() => {

chrome/browser/resources/settings/chromeos/os_settings.gni

@@ -73,7 +73,7 @@ os_settings_namespace_rewrites = settings_namespace_rewrites +
                                    "settings.routes|routes",
                                    "settings.SmartLockSignInEnabledState|SmartLockSignInEnabledState",
                                    "settings.TimeZoneAutoDetectMethod|TimeZoneAutoDetectMethod",
-                                   "settings.TimeZoneBrowserProxyImpl|TimeZoneBrowserProxyImpl",
+                                   "settings.TimeZoneBrowserProxy|TimeZoneBrowserProxy",
                                    "settings.ValidateKerberosConfigResult|ValidateKerberosConfigResult",
                                    "settings.WallpaperBrowserProxy|WallpaperBrowserProxy",
                                    "smb_shares.SmbBrowserProxy|SmbBrowserProxy",

chrome/test/data/webui/settings/chromeos/date_time_page_tests.js

@@ -11,18 +11,48 @@
 // #import {TestBrowserProxy} from 'chrome://test/test_browser_proxy.m.js';
 // #import {TimeZoneAutoDetectMethod, TimeZoneBrowserProxyImpl} from 'chrome://os-settings/chromeos/lazy_load.js';
 // #import {getDeepActiveElement} from 'chrome://resources/js/util.m.js';
-// #import {waitAfterNextRender} from 'chrome://test/test_util.m.js';
+// #import {waitAfterNextRender, flushTasks} from 'chrome://test/test_util.m.js';
 // clang-format on
 
 /** @implements {settings.TimeZoneBrowserProxy} */
 class TestTimeZoneBrowserProxy extends TestBrowserProxy {
   constructor() {
-    super(['showParentAccessForTimeZone']);
+    super([
+      'dateTimePageReady',
+      'getTimeZones',
+      'showParentAccessForTimeZone',
+      'showSetDateTimeUI',
+    ]);
+
+    /** @private {!Array<!Array<string>>} */
+    this.fakeTimeZones_ = [];
   }
+
+  /** @override */
+  dateTimePageReady() {
+    this.methodCalled('dateTimePageReady');
+  }
+
+  /** @override */
+  getTimeZones() {
+    this.methodCalled('getTimeZones');
+    return Promise.resolve(this.fakeTimeZones_);
+  }
+
+  /** @param {!Array<!Array<string>>} timeZones */
+  setTimeZones(timeZones) {
+    this.fakeTimeZones_ = timeZones;
+  }
+
   /** @override */
   showParentAccessForTimeZone() {
     this.methodCalled('showParentAccessForTimeZone');
   }
+
+  /** @override */
+  showSetDateTimeUI() {
+    this.methodCalled('showSetDateTimeUI');
+  }
 }
 
 function getFakePrefs() {
@@ -210,10 +240,6 @@ const fakeTimeZones = [
 suite('settings-date-time-page', function() {
   let dateTime;
 
-  // Track whether handler functions have been called.
-  let dateTimePageReadyCalled;
-  let getTimeZonesCalled;
-
   /** @type {?TestTimeZoneBrowserProxy} */
   let testBrowserProxy = null;
 
@@ -223,34 +249,12 @@ suite('settings-date-time-page', function() {
     settings.TimeZoneBrowserProxyImpl.instance_ = testBrowserProxy;
     PolymerTest.clearBody();
     CrSettingsPrefs.resetForTesting();
-
-    dateTimePageReadyCalled = false;
-    getTimeZonesCalled = false;
-
-    registerMessageCallback('dateTimePageReady', null, function() {
-      assertFalse(dateTimePageReadyCalled);
-      dateTimePageReadyCalled = true;
-    });
-
-    registerMessageCallback('getTimeZones', null, function(callback) {
-      assertFalse(getTimeZonesCalled);
-      getTimeZonesCalled = true;
-      cr.webUIResponse(callback, true, fakeTimeZones);
-    });
   });
 
   teardown(function() {
     settings.Router.getInstance().resetRouteForTesting();
   });
 
-  function checkDateTimePageReadyCalled() {
-    if (dateTime.prefs.cros.flags.fine_grained_time_zone_detection_enabled
-            .value) {
-      return;
-    }
-    assertTrue(dateTimePageReadyCalled);
-  }
-
   function getTimeZoneSelector(id) {
     return dateTime.$$('timezone-selector').$$(id);
   }
@@ -287,14 +291,14 @@ suite('settings-date-time-page', function() {
         updatePrefsWithPolicy(dateTime.prefs, managed, valueFromPolicy);
   }
 
-  test('auto-detect on', function(done) {
+  test('auto-detect on', async function() {
+    testBrowserProxy.setTimeZones(fakeTimeZones);
     const prefs = getFakePrefs();
     dateTime = initializeDateTime(prefs, false);
+    Polymer.dom.flush();
     const resolveMethodDropdown = dateTime.$$('#timeZoneResolveMethodDropdown');
 
-    setTimeout(function() {
-      checkDateTimePageReadyCalled();
-      assertFalse(getTimeZonesCalled);
+    assertEquals(0, testBrowserProxy.getCallCount('getTimeZones'));
 
     verifyAutoDetectSetting(true, false);
     assertFalse(resolveMethodDropdown.disabled);
@@ -302,47 +306,35 @@ suite('settings-date-time-page', function() {
 
     clickDisableAutoDetect(dateTime);
     Polymer.dom.flush();
-    });
 
-    setTimeout(function() {
     verifyAutoDetectSetting(false, false);
     assertTrue(resolveMethodDropdown.disabled);
-      assertTrue(getTimeZonesCalled);
+    await testBrowserProxy.whenCalled('getTimeZones');
 
     verifyTimeZonesPopulated(true);
-      done();
-    });
   });
 
-  test('auto-detect off', function(done) {
+  test('auto-detect off', async function() {
+    testBrowserProxy.setTimeZones(fakeTimeZones);
     dateTime = initializeDateTime(getFakePrefs(), false);
     const resolveMethodDropdown = dateTime.$$('#timeZoneResolveMethodDropdown');
 
-    setTimeout(function() {
     dateTime.set(
-          'prefs.generated.resolve_timezone_by_geolocation_on_off.value',
-          false);
+        'prefs.generated.resolve_timezone_by_geolocation_on_off.value', false);
     dateTime.set(
         'prefs.generated.resolve_timezone_by_geolocation_method_short.value',
         settings.TimeZoneAutoDetectMethod.DISABLED);
-    });
 
-    setTimeout(function() {
-      checkDateTimePageReadyCalled();
-      assertTrue(getTimeZonesCalled);
+    await testBrowserProxy.whenCalled('getTimeZones');
 
     verifyAutoDetectSetting(false, false);
     assertTrue(resolveMethodDropdown.disabled);
     verifyTimeZonesPopulated(true);
 
     clickEnableAutoDetect(dateTime);
-    });
 
-    setTimeout(function() {
     verifyAutoDetectSetting(true);
     assertFalse(resolveMethodDropdown.disabled);
-      done();
-    });
   });
 
   test('Deep link to auto set time zone on main page', async () => {
@@ -367,14 +359,14 @@ suite('settings-date-time-page', function() {
         'Auto set time zone toggle should be focused for settingId=1001.');
   });
 
-  test('auto-detect forced on', function(done) {
+  test('auto-detect forced on', async function() {
+    testBrowserProxy.setTimeZones(fakeTimeZones);
     const prefs = getFakePrefs();
     dateTime = initializeDateTime(prefs, true, true);
+    Polymer.dom.flush();
     const resolveMethodDropdown = dateTime.$$('#timeZoneResolveMethodDropdown');
 
-    setTimeout(function() {
-      checkDateTimePageReadyCalled();
-      assertFalse(getTimeZonesCalled);
+    assertEquals(0, testBrowserProxy.getCallCount('getTimeZones'));
 
     verifyAutoDetectSetting(true, true);
     assertFalse(resolveMethodDropdown.disabled);
@@ -382,34 +374,28 @@ suite('settings-date-time-page', function() {
 
     // Cannot disable auto-detect.
     clickDisableAutoDetect(dateTime);
-    });
 
-    setTimeout(function() {
     verifyAutoDetectSetting(true, true);
     assertFalse(resolveMethodDropdown.disabled);
-      assertFalse(getTimeZonesCalled);
+    assertEquals(0, testBrowserProxy.getCallCount('getTimeZones'));
 
     // Update the policy: force auto-detect off.
     updatePolicy(dateTime, true, false);
-    });
-    setTimeout(function() {
+
     verifyAutoDetectSetting(false, true);
     assertTrue(resolveMethodDropdown.disabled);
 
-      assertTrue(getTimeZonesCalled);
+    await testBrowserProxy.whenCalled('getTimeZones');
     verifyTimeZonesPopulated(true);
-      done();
-    });
   });
 
-  test('auto-detect forced off', function(done) {
+  test('auto-detect forced off', async function() {
+    testBrowserProxy.setTimeZones(fakeTimeZones);
     const prefs = getFakePrefs();
     dateTime = initializeDateTime(prefs, true, false);
     const resolveMethodDropdown = dateTime.$$('#timeZoneResolveMethodDropdown');
 
-    setTimeout(function() {
-      checkDateTimePageReadyCalled();
-      assertTrue(getTimeZonesCalled);
+    await testBrowserProxy.whenCalled('getTimeZones');
 
     verifyAutoDetectSetting(false, true);
     assertTrue(resolveMethodDropdown.disabled);
@@ -417,19 +403,14 @@ suite('settings-date-time-page', function() {
 
     // Remove the policy so user's preference takes effect.
     updatePolicy(dateTime, false, false);
-    });
 
-    setTimeout(function() {
     verifyAutoDetectSetting(true, false);
     assertFalse(resolveMethodDropdown.disabled);
 
     // User can disable auto-detect.
     clickDisableAutoDetect(dateTime);
-    });
-    setTimeout(function() {
+
     verifyAutoDetectSetting(false, false);
-      done();
-    });
   });
 
   test('auto-detect on supervised account', async () => {
@@ -514,32 +495,27 @@ suite('settings-date-time-page', function() {
     assertFalse(timezoneSelector.disabled);
   });
 
-  test('set date and time button', function() {
-    dateTime = initializeDateTime(getFakePrefs(), false);
-
-    let showSetDateTimeUICalled = false;
-    registerMessageCallback('showSetDateTimeUI', null, function() {
-      assertFalse(showSetDateTimeUICalled);
-      showSetDateTimeUICalled = true;
-    });
+  test('set date and time button', async function() {
+    const prefs = getFakePrefs();
+    // Set fine grained time zone off so that toggle appears on this page.
+    prefs.cros.flags.fine_grained_time_zone_detection_enabled.value = false;
+    dateTime = initializeDateTime(prefs, false);
 
-    setTimeout(function() {
     const setDateTimeButton = dateTime.$$('#setDateTime');
     assertEquals(0, setDateTimeButton.offsetHeight);
 
     // Make the date and time editable.
     cr.webUIListenerCallback('can-set-date-time-changed', true);
+    await test_util.flushTasks();
     assertGT(setDateTimeButton.offsetHeight, 0);
 
-      assertFalse(showSetDateTimeUICalled);
+    assertEquals(0, testBrowserProxy.getCallCount('showSetDateTimeUI'));
     setDateTimeButton.click();
-    });
-    setTimeout(function() {
-      assertTrue(showSetDateTimeUICalled);
+
+    assertEquals(1, testBrowserProxy.getCallCount('showSetDateTimeUI'));
 
     // Make the date and time not editable.
     cr.webUIListenerCallback('can-set-date-time-changed', false);
     assertEquals(setDateTimeButton.offsetHeight, 0);
   });
-  });
 });