MD Settings: Use a browser proxy from the languages page. Re-enable test.

chrome/test/data/webui/settings/languages_tests.js

 // Copyright 2016 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('settings-languages', function() {
   /**
+   * @constructor
+   * @implements {settings.LanguagesBrowserProxy}
+   * @extends {settings.TestBrowserProxy}
+   */
+  var TestLanguagesBrowserProxy = function() {
+    var methodNames = [];
+    if (cr.isChromeOS || cr.isWindows)
+      methodNames.push('getProspectiveUILanguage');
+
+    settings.TestBrowserProxy.call(this, methodNames);
+
+    this.languageSettingsPrivate_ = null;

[Dan Beam, 2017/04/19 17:46:52]

this.languageSettingsPrivate_ = new settings.FakeLanguageSettingsPrivate;

[dpapad, 2017/04/19 18:11:30]

See related last comment in this file.

+    this.inputMethodPrivate_ = null;
+  };
+
+  TestLanguagesBrowserProxy.prototype = {
+    __proto__: settings.TestBrowserProxy.prototype,
+
+    /** @override */
+    getLanguageSettingsPrivate: function() {
+      return this.languageSettingsPrivate_;
+    },
+
+    /** @param {!LanguagesSettingsPrivate} private */
+    setLanguageSettingsPrivate: function(private) {
+      this.languageSettingsPrivate_ = private;

[Dan Beam, 2017/04/19 17:46:52]

i would not recommend private as a var name

[dpapad, 2017/04/19 18:11:30]

Done.

+    },
+
+    /** @override */
+    getInputMethodPrivate: function() {
+      return this.inputMethodPrivate_;
+    },
+
+    /** @param {!LanguagesSettingsPrivate} private */
+    setInputMethodPrivate: function(private) {
+      this.inputMethodPrivate_ = private;
+    },
+  };
+
+  if (cr.isChromeOS || cr.isWindows) {
+    /** @override */
+    TestLanguagesBrowserProxy.prototype.getProspectiveUILanguage =
+        function() {
+      this.methodCalled('getProspectiveUILanguage');
+      return Promise.resolve('en-US');
+    };
+  }
+
+  /**
    * Data-binds two Polymer properties using the property-changed events and
    * set/notifyPath API. Useful for testing components which would normally be
    * used together.
    * @param {!HTMLElement} el1
    * @param {!HTMLElement} el2
    * @param {string} property
    */
   function fakeDataBind(el1, el2, property) {
     var forwardChange = function(el, event) {
       if (event.detail.hasOwnProperty('path'))
@@ skipping 50 matching lines @@
      * @param {!Array<string>} expected
      */
     function assertLanguageOrder(expected) {
       assertEquals(expected.length, languageHelper.languages.enabled.length);
       for (var i = 0; i < expected.length; i++) {
         assertEquals(
             expected[i], languageHelper.languages.enabled[i].language.code);
       }
     }
 
+    /** @type {?settings.LanguagesBrowserProxy} */
+    var browserProxy = null;
+
     var languageHelper;
-    var languageSettingsPrivate;
 
     suiteSetup(function() {
       CrSettingsPrefs.deferInitialization = true;
       PolymerTest.clearBody();
     });
 
     setup(function() {
       var settingsPrefs = document.createElement('settings-prefs');
       var settingsPrivate = new settings.FakeSettingsPrivate(getFakePrefs());
       settingsPrefs.initialize(settingsPrivate);
       document.body.appendChild(settingsPrefs);
 
-      languageSettingsPrivate = new settings.FakeLanguageSettingsPrivate();
+      // Setup test browser proxy.
+      browserProxy = new TestLanguagesBrowserProxy();
+      settings.LanguagesBrowserProxyImpl.instance_ = browserProxy;
+
+      // Setup fake languageSettingsPrivate API.
+      var languageSettingsPrivate = new settings.FakeLanguageSettingsPrivate();

[Dan Beam, 2017/04/19 17:46:52]

why not just instantiate these directly in TestLanguageBrowserProxy's
constructor?

[dpapad, 2017/04/19 18:11:30]

The next line calls setSettingsPrefs() which is a method only existing on
FakeLanguageSettingsPrivate, but not on the interface LanguageSettingsPrivate
returned by LanguagesBrowserProxy. So technically I think this is more correct.
In practice it does not matter, because we don't currently type check our tests.

Besides type checking, from a design point of view, a test browser proxy is
meant to be a super thin object that does no work in its constructor. Clients of
the test proxy are supposed to configure the proxy by calling set*() methods on
it, in order to force a certain response.

[Dan Beam, 2017/04/19 18:31:55]

i don't see that as a requirement, and am likely not to do this when it
complicates the code

       languageSettingsPrivate.setSettingsPrefs(settingsPrefs);
-      settings.languageSettingsPrivateApiForTest = languageSettingsPrivate;
+      browserProxy.setLanguageSettingsPrivate(languageSettingsPrivate);
 
-      var getProspectiveUILanguageCalled = false;
-      registerMessageCallback('getProspectiveUILanguage', null,
-          function(callbackId) {
-            assertFalse(getProspectiveUILanguageCalled);
-            getProspectiveUILanguageCalled = true;
-            cr.webUIResponse(callbackId, true, 'en-US');
-          });
+      // Setup fake inputMethodPrivate API.
+      browserProxy.setInputMethodPrivate(new settings.FakeInputMethodPrivate());
 
       languageHelper = document.createElement('settings-languages');
 
       // Prefs would normally be data-bound to settings-languages.
       fakeDataBind(settingsPrefs, languageHelper, 'prefs');
 
       document.body.appendChild(languageHelper);
       return languageHelper.whenReady().then(function() {
-        assertEquals(
-            cr.isChromeOS || cr.isWindows, getProspectiveUILanguageCalled);
+        if (cr.isChromeOS || cr.isWindows)
+          return browserProxy.whenCalled('getProspectiveUILanguage');
       });
     });
 
     test('languages model', function() {
+      var languageSettingsPrivate = browserProxy.getLanguageSettingsPrivate();
       for (var i = 0; i < languageSettingsPrivate.languages.length;
            i++) {
         assertEquals(languageSettingsPrivate.languages[i].code,
                      languageHelper.languages.supported[i].code);
       }
       assertLanguageOrder(['en-US', 'sw']);
       assertEquals('en', languageHelper.languages.translateTarget);
 
       // TODO(michaelpg): Test other aspects of the model.
     });
@@ skipping 72 matching lines @@
         // The US Swahili keyboard should still be enabled, because it supports
         // English which is still enabled.
         assertTrue(languageHelper.languages.inputMethods.enabled.some(
               function(inputMethod) {
                 return inputMethod.id == swUS;
               }));
       });
     }
   });
 });