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

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

Index: chrome/browser/resources/settings/search_page/search_page.js
diff --git a/chrome/browser/resources/settings/search_page/search_page.js b/chrome/browser/resources/settings/search_page/search_page.js
index 0f8b3cbad16c70ce713db4da33a02b5941fbb2eb..0746aad1e017745e4f068cd4df987a296c9e232f 100644
--- a/chrome/browser/resources/settings/search_page/search_page.js
+++ b/chrome/browser/resources/settings/search_page/search_page.js
@@ -6,13 +6,6 @@
  * @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
  */
@@ -28,58 +21,39 @@ Polymer({
       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);
+  /** @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));
   },
 
-
-  /**
-   * 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;
+  /** @private */
+  onManageSearchEnginesTap_: function() {
+    this.$.pages.setSubpageChain(['search-engines']);
   },
 
   /** @private */
-  onSearchEnginesTap_: function() {
-    this.$.pages.setSubpageChain(['search-engines']);
+  onDefaultEngineChanged_: function() {
+    this.browserProxy_.setDefaultSearchEngine(this.$.searchEnginesMenu.value);
   },
 });