Print Preview: Change getPrinters to cr.sendWithPromise

chrome/browser/resources/print_preview/data/destination_store.js

Index: chrome/browser/resources/print_preview/data/destination_store.js
diff --git a/chrome/browser/resources/print_preview/data/destination_store.js b/chrome/browser/resources/print_preview/data/destination_store.js
index 116b1c64ee0f5fcb3fff3e44a513e9c1ef7009b6..a480c4f87cb453c1b0f0f1fbe4d191beb5b31666 100644
--- a/chrome/browser/resources/print_preview/data/destination_store.js
+++ b/chrome/browser/resources/print_preview/data/destination_store.js
@@ -1078,7 +1078,14 @@ cr.define('print_preview', function() {
     startLoadLocalDestinations: function() {
       if (!this.hasLoadedAllLocalDestinations_) {
         this.hasLoadedAllLocalDestinations_ = true;
-        this.nativeLayer_.startGetLocalDestinations();
+        this.nativeLayer_.getPrinters().then(

[dpapad, 2017/06/01 19:01:53]

Isn't this equivalent to

this.nativeLayer_.getPrinters().then(this.onLocalDestinationsSet_.bind(this));

[rbpotter, 2017/06/01 22:31:12]

Done.

+            /**
+             * @param {Array<print_preview.LocalDestinationInfo>}
+             *     destinationInfos A list of the local destinations retrieved.
+             */
+            function(destinationInfos) {
+              this.onLocalDestinationsSet_(destinationInfos);
+            }.bind(this));
         this.isLocalDestinationSearchInProgress_ = true;

[dpapad, 2017/06/01 19:01:53]

Not for this CL, mostly brainstorming:

I am seeing a few similar booleans in this file

this.isLocalDestinationSearchInProgress_
this.isPrivetDestinationSearchInProgress_
this.isExtensionDestinationSearchInProgress_

and also similar methods like

startLoadLocalDestinations
startLoadPrivetDestinations
startLoadExtensionDestinations

I think these are all remnants of the previous design where the request and the
response where not directly associated to each other via a Promise. I am hoping
that we can eventually improve this design, but at this point I don't know
exactly how. In theory, every  function that spawns an async operation, should
return a Promise. For example

instead of startLoadLocalDestinations it could (maybe?) be

/** @return !Promise<...> Fires when local destinations have been loaded */
loadLocalDestinations: function(...) {...}

and similar for all other such methods in this file.

[rbpotter, 2017/06/01 22:31:12]

Acknowledged.

         cr.dispatchSimpleEvent(
             this, DestinationStore.EventType.DESTINATION_SEARCH_STARTED);
@@ -1362,10 +1369,6 @@ cr.define('print_preview', function() {
       var nativeLayerEventTarget = this.nativeLayer_.getEventTarget();
       this.tracker_.add(
           nativeLayerEventTarget,
-          print_preview.NativeLayer.EventType.LOCAL_DESTINATIONS_SET,
-          this.onLocalDestinationsSet_.bind(this));
-      this.tracker_.add(
-          nativeLayerEventTarget,
           print_preview.NativeLayer.EventType.CAPABILITIES_SET,
           this.onLocalDestinationCapabilitiesSet_.bind(this));
       this.tracker_.add(
@@ -1437,11 +1440,12 @@ cr.define('print_preview', function() {
 
     /**
      * Called when the local destinations have been got from the native layer.
-     * @param {Event} event Contains the local destinations.
+     * @param {Array<print_preview.LocalDestinationInfo>} destinationInfos A

[dpapad, 2017/06/01 19:01:53]

Should this be !Array<!print_preview.LocalDestinationInfo> ?

[rbpotter, 2017/06/01 22:31:12]

Done.

+     *     list of the local destinations retrieved.
      * @private
      */
-    onLocalDestinationsSet_: function(event) {
-      var localDestinations = event.destinationInfos.map(function(destInfo) {
+    onLocalDestinationsSet_: function(destinationInfos) {
+      var localDestinations = destinationInfos.map(function(destInfo) {
         return print_preview.LocalDestinationParser.parse(destInfo);
       });
       this.insertDestinations_(localDestinations);