[CUPS] Implement the local CUPS printer setup waiting UI.

chrome/browser/resources/print_preview/search/destination_search.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';
 
   /**
    * Component used for searching for a print destination.
    * This is a modal dialog that allows the user to search and select a
@@ skipping 56 matching lines @@
 
     /**
      * Child overlay used for resolving a provisional destination. The overlay
      * is shown when the user attempts to select a provisional destination.
      * Set only when a destination is being resolved.
      * @private {?print_preview.ProvisionalDestinationResolver}
      */
     this.provisionalDestinationResolver_ = null;
 
     /**
+     * The destination that is currently in configuration.
+     * @private {?print_preview.Destination}
+     */
+    this.destinationInConfiguring_ = null;
+
+    /**
      * Search box used to search through the destination lists.
      * @type {!print_preview.SearchBox}
      * @private
      */
     this.searchBox_ = new print_preview.SearchBox(
         loadTimeData.getString('searchBoxPlaceholder'));
     this.addChild(this.searchBox_);
 
     /**
      * Destination list containing recent destinations.
@@ skipping 129 matching lines @@
       this.tracker.add(
           this.getChildElement('.cloudprint-promo > .close-button'),
           'click',
           this.onCloudprintPromoCloseButtonClick_.bind(this));
       this.tracker.add(
           this.searchBox_,
           print_preview.SearchBox.EventType.SEARCH,
           this.onSearch_.bind(this));
       this.tracker.add(
           this,
+          print_preview.DestinationListItem.EventType.CONFIGURE_REQUEST,
+          this.onDestinationConfigureRequest_.bind(this));
+      this.tracker.add(
+          this,
           print_preview.DestinationListItem.EventType.SELECT,
           this.onDestinationSelect_.bind(this));
       this.tracker.add(
           this,
           print_preview.DestinationListItem.EventType.REGISTER_PROMO_CLICKED,
           function() {
             this.metrics_.record(print_preview.Metrics.DestinationSearchBucket.
                 REGISTER_PROMO_SELECTED);
           }.bind(this));
 
@@ skipping 323 matching lines @@
      * Called when a destination search should be executed. Filters the
      * destination lists with the given query.
      * @param {Event} evt Contains the search query.
      * @private
      */
     onSearch_: function(evt) {
       this.filterLists_(evt.queryRegExp);
     },
 
     /**
+     * Handler for {@code print_preview.DestinationListItem.EventType.
+     * CONFIGURE_REQUEST} event, which is called to check a destination list
+     * item needs to be setup on Chrome OS before being selected. Note we do not
+     * allow configuring more than one destination at the same time.
+     * @param {Event} evt Contains the destination needs to be setup.

[dpapad, 2017/03/30 20:56:23]

!Event

[xdai1, 2017/03/30 23:18:43]

Done.

+     * @private
+     */
+    onDestinationConfigureRequest_: function(evt) {

[dpapad, 2017/03/30 20:56:23]

|evt| is not an established abbreviation. Can you rename to |event| or just |e|?

See https://google.github.io/styleguide/cppguide.html#General_Naming_Rules, from
the C++ styleguide that states "do not abbreviate by deleting letters within a
word".

[xdai1, 2017/03/30 23:18:43]

Sure. Renamed it to event.

+      var destinationItem = evt.destination.isLocal
+          ? this.localList_.getDestinationItem(evt.destination.id)

[dpapad, 2017/03/30 20:56:24]

It is way more common to put the binary operator in the previous line

var destinationItem = evt.destination.isLocal ?
    this.localList_.getDestinationItem(evt.destination.id) :
    this.cloudList_.getDestinationItem(evt.destination.id);

[xdai1, 2017/03/30 23:18:43]

Done.

+          : this.cloudList_.getDestinationItem(evt.destination.id);
+      if (!destinationItem)

[dpapad, 2017/03/30 20:56:24]

When can this happen? Please add a comment to explain, or if it should never
happen, add an assertion instead  of silently returning early.

[xdai1, 2017/03/30 23:18:44]

I don't think it can happen. Modified to an assertion.

+        return;
+
+      // Another printer setup is in process or the printer doesn't need to be
+      // set up. Reject the setup request directly.
+      if (this.destinationInConfiguring_ != null ||
+          evt.destination.origin != print_preview.Destination.Origin.CROS ||
+          evt.destination.capabilities != null) {
+        destinationItem.onConfigureRequestRejected(
+            this.destinationInConfiguring_ != null);
+      } else {
+        destinationItem.onConfigureRequestAccepted();
+        this.handleConfigureDestination_(evt.destination);
+      }
+    },
+
+    /**
+     * Called When a destination needs to be setup.
+     * @param {!print_preview.Destination} destination The destination needs to
+     *     be setup.
+     * @private
+     */
+    handleConfigureDestination_: function(destination) {
+      assert(destination.origin == print_preview.Destination.Origin.CROS,
+             'Only local printer on Chrome OS requires setup.');
+      this.destinationInConfiguring_ = destination;
+      this.destinationStore_.resolveCrosDestination(destination).then(
+        /**
+         * Called when the Promise is fulfilled.
+         * @param {!print_preview.PrinterSetupResponse} response.
+         */
+        function(response) {
+          this.localList_.getDestinationItem(destination.id)
+              .onConfigureResolved(response);
+          this.destinationInConfiguring_ = null;
+        }.bind(this),
+        /**
+         * Called when the Promise is rejected.

[dpapad, 2017/03/30 20:56:24]

Nit: Maybe remove this comment? Does not add much extra information. The
signature of then(a, b) already implies that b is called when the promise is
rejected.

[xdai1, 2017/03/30 23:18:44]

I would prefer to preserve this comment if the above comment is preserved. Or I
can remove both comments here. What do you think?

[dpapad, 2017/03/31 00:37:52]

I would prefer trimming the 1st comment to
/** @param {!print_preview.PrinterSetupResponse} response */

and keeping it since it documents the response's type. The 2nd one as said above
does not add any valuable info I think, but you are welcome to keep it if you
think otherwise.

+         */
+        function() {
+          this.localList_.getDestinationItem(destination.id)
+              .onConfigureResolved({printerId: destination.id, success: false});
+          this.destinationInConfiguring_ = null;
+        }.bind(this));
+    },
+
+    /**
      * Handler for {@code print_preview.DestinationListItem.EventType.SELECT}
-     * event, which is called when a destinationi list item is selected.
+     * event, which is called when a destination list item is selected.
      * @param {Event} evt Contains the selected destination.
      * @private
      */
     onDestinationSelect_: function(evt) {
       this.handleOnDestinationSelect_(evt.destination);
     },
 
     /**
      * Called when a destination is selected. Clears the search and hides the
      * widget. If The destination is provisional, it runs provisional
      * destination resolver first.
      * @param {!print_preview.Destination} destination The selected destination.
      * @private
      */
     handleOnDestinationSelect_: function(destination) {
-      if (destination.origin == print_preview.Destination.Origin.CROS &&
-          !destination.capabilities) {
-        // local printers on CrOS require setup.
-        assert(!this.printerConfigurer_);
-        this.printerConfigurer_ = new print_preview.CrosDestinationResolver(
-            this.destinationStore_, destination);
-        this.addChild(this.printerConfigurer_);
-        this.printerConfigurer_.run(this.getElement()).
-            then(
-                /**
-                  * @param {!print_preview.PrinterSetupResponse} result
-                  *    An object containing the printerId and capabilities.
-                  */
-                function(result) {
-                  assert(result.printerId == destination.id);
-                  destination.capabilities = result.capabilities;
-                  this.handleOnDestinationSelect_(destination);
-                }.bind(this),
-                function() {
-                  console.warn(
-                      'Failed to setup destination: ' + destination.id);
-                }).
-            then(function() {
-              this.removeChild(this.printerConfigurer_);
-              this.printerConfigurer_ = null;
-            }.bind(this));
-        return;
-      }
-
       if (destination.isProvisional) {
         assert(!this.provisionalDestinationResolver_,
                'Provisional destination resolver already exists.');
         this.provisionalDestinationResolver_ =
             print_preview.ProvisionalDestinationResolver.create(
                 this.destinationStore_, destination);
         assert(!!this.provisionalDestinationResolver_,
                'Unable to create provisional destination resolver');
 
         var lastFocusedElement = document.activeElement;
@@ skipping 168 matching lines @@
     onWindowResize_: function() {
       this.reflowLists_();
     }
   };
 
   // Export
   return {
     DestinationSearch: DestinationSearch
   };
 });