Print Preview: Change getPrinters to cr.sendWithPromise

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

 // Copyright (c) 2012 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('print_preview', function() {
   'use strict';
 
   /**
    * A data store that stores destinations and dispatches events when the data
    * store changes.
@@ skipping 1060 matching lines @@
           }
         }
       }
       this.selectPdfDestination_();
     },
 
     /** Initiates loading of local print destinations. */
     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);
       }
     },
 
     /** Initiates loading of privet print destinations. */
     startLoadPrivetDestinations: function() {
       if (!this.hasLoadedAllPrivetDestinations_) {
         if (this.privetDestinationSearchInProgress_)
           clearTimeout(this.privetSearchTimeout_);
@@ skipping 262 matching lines @@
     },
 
     /**
      * Binds handlers to events.
      * @private
      */
     addEventListeners_: 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(
           nativeLayerEventTarget,
           print_preview.NativeLayer.EventType.GET_CAPABILITIES_FAIL,
           this.onGetCapabilitiesFail_.bind(this));
       this.tracker_.add(
           nativeLayerEventTarget,
           print_preview.NativeLayer.EventType.DESTINATIONS_RELOAD,
           this.onDestinationsReload_.bind(this));
@@ skipping 51 matching lines @@
       this.hasLoadedAllExtensionDestinations_ = false;
 
       clearTimeout(this.autoSelectTimeout_);
       this.autoSelectTimeout_ = setTimeout(
           this.selectDefaultDestination_.bind(this),
           DestinationStore.AUTO_SELECT_TIMEOUT_);
     },
 
     /**
      * 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);
       this.isLocalDestinationSearchInProgress_ = false;
       cr.dispatchSimpleEvent(
           this, DestinationStore.EventType.DESTINATION_SEARCH_DONE);
     },
 
     /**
      * Called when the native layer retrieves the capabilities for the selected
@@ skipping 242 matching lines @@
       return this.getDestinationKey_(
           destination.origin, destination.id, destination.account);
     }
   };
 
   // Export
   return {
     DestinationStore: DestinationStore
   };
 });