MD Settings: Search page, remove last usage of chrome.searchPrivate API.

chrome/browser/resources/settings/search_page/search_page.js

 // Copyright 2015 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.
 
 /**
  * @fileoverview
  * 'settings-search-page' is the settings page containing search settings.
  *
- * Example:
- *
- *    <iron-animated-pages>
- *      <settings-search-page prefs="{{prefs}}"></settings-search-page>
- *      ... other pages ...
- *    </iron-animated-pages>
- *
  * @group Chrome Settings Elements
  * @element settings-search-page
  */
 Polymer({
   is: 'settings-search-page',
 
   properties: {
     /**
      * The current active route.
      */
     currentRoute: {
       type: Object,
       notify: true,
     },
 
-    /**
-     * List of default search engines available.

[Dan Beam, 2016/02/23 02:41:22]

this removes "default" as a bit of documentation.  is that important?

[dpapad, 2016/02/23 18:35:39]

Restored.

-     * @type {?Array<!SearchEngine>}
-     */
-    searchEngines: {
+    /** @private {!Array<!SearchEngine>} */
+    searchEngines_: {
       type: Array,
-      value: function() { return []; },
+      value: function() { return []; }
     },
+
+    /** @private {!settings.SearchEnginesBrowserProxy} */
+    browserProxy_: Object,
   },
 
   /** @override */
   created: function() {
-    chrome.searchEnginesPrivate.onSearchEnginesChanged.addListener(
-        this.updateSearchEngines_.bind(this));
-    chrome.searchEnginesPrivate.getSearchEngines(
-        this.updateSearchEngines_.bind(this));
+    this.browserProxy_ = settings.SearchEnginesBrowserProxyImpl.getInstance();
   },
 
-  /**
-   * Persists the new default search engine back to Chrome. Called when the
-   * user selects a new default in the search engines dropdown.
-   * @private
-   */
-  defaultEngineGuidChanged_: function() {
-    chrome.searchEnginesPrivate.setSelectedSearchEngine(
-        this.$.searchEnginesMenu.value);
-  },
-
-
-  /**
-   * Updates the list of default search engines based on the given |engines|.
-   * @param {!Array<!SearchEngine>} engines All the search engines.
-   * @private
-   */
-  updateSearchEngines_: function(engines) {
-    var defaultEngines = [];
-
-    engines.forEach(function(engine) {
-      if (engine.type ==
-          chrome.searchEnginesPrivate.SearchEngineType.DEFAULT) {
-        defaultEngines.push(engine);
-        if (engine.isSelected) {
-          this.$.searchEnginesMenu.value = engine.guid;
-        }
-      }
-    }, this);
-
-    this.searchEngines = defaultEngines;
+  /** @override */
+  ready: function() {
+    var updateSearchEngines = function(searchEngines) {
+      this.set('searchEngines', searchEngines.defaults);
+    };

[Dan Beam, 2016/02/23 02:41:22]

bind() only once here

[dpapad, 2016/02/23 18:35:39]

Done.

+    this.browserProxy_.getSearchEnginesList().then(
+        updateSearchEngines.bind(this));
+    cr.addWebUIListener(

[michaelpg, 2016/02/20 04:31:16]

Since the BrowserProxy handles push (setDefaultSearchEngine) and pull
(getSearchEnginesList), would it make sense to also have it handle events
(onSearchEnginesChanged) instead of going down to the cr layer here in the UI?

[dpapad, 2016/02/22 18:57:53]

Are you suggesting to wrap the cr.addWebUIListener() call with a new method on
the proxy that delegates to cr.addWebUIListener, or to not use
cr.addWebUIListener at all?


If you are suggesting the latter, I think it would make things more complicated.
Briefly giving a description of the pattern we have ended up, which seems to
work well enough (both for prod and testing).

1) The browser proxy intentionally has no state, it simply wraps every single
chrome.send() call with a type annotated version. It has no reference back to
any Polymer element that is using the proxy.
2) The proxy does not handle any events, and does not expose and global method
that is called from C++.
3) Individual Polymer elements are responsible for calling cr.addWebUIListener
(and removeWebUIListener if needed).

The above 3 characteristics allow the following usage during tests.
1) A test version of the proxy is used to intercept JS -> C++ calls, and to
provide mock responses.
2) C++ event callback codepaths can be triggered during the test by directly
calling cr.webUIListenerCallback.

+        'search-engines-changed', updateSearchEngines.bind(this));
   },
 
   /** @private */
-  onSearchEnginesTap_: function() {
+  onManageSearchEnginesTap_: function() {
     this.$.pages.setSubpageChain(['search-engines']);
   },
+
+  /** @private */
+  onDefaultEngineChanged_: function() {
+    this.browserProxy_.setDefaultSearchEngine(this.$.searchEnginesMenu.value);
+  },
 });