Simplified print preview printer selection to be more consistent with Chrome

chrome/browser/resources/print_preview/print_preview_cloud.js

 // Copyright (c) 2011 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('cloudprint', function() {
 
   // The URL to use to access the cloud print servers.
   // Set by a call to setBaseURL.
   var cloudPrintBaseURL = '';
 
@@ skipping 101 matching lines @@
   /**
    * Parse the response from the fetchPrinters call.
    * @param {function} callback Function to be called to process response.
    * @param {XMLHttpRequest} xhr The object used to make the request.
    */
   function fetchPrintersResponse(callback, xhr) {
     if (xhr.status == 200) {
       var searchResult = JSON.parse(xhr.responseText);
       if (searchResult['success']) {
         var printerList = searchResult['printers'];
-        callback.call(this, printerList);
+         addCloudPrinters(printerList, callback);

[dpapad, 2011/09/22 18:11:57]

Fix indentation.

[Albert Bodenhamer, 2011/09/22 23:23:39]

Done.

       } else {
-        callback.call(this, null);
+        addCloudPrinters(null, callback);
       }
     } else {
-      callback.call(this, null);
+      addCloudPrinters(null, callback);

[dpapad, 2011/09/22 18:11:57]

Could you restructure the if blocks as follows?
if (xhr.status == 200) {
  var searchResult = JSON.parse(xhr.responseText);
  if (searchResult['success']) {
    var printerList = searchResult['printers'];
    addCloudPrinters(printerList, callback);
    return;
  }
}
addCloudPrinters(null, callback);

[Albert Bodenhamer, 2011/09/22 23:23:39]

Done.

     }
   }
 
   /**
    * Retrieve the list of printers available via cloud print.
    * @param {function} callback Function to be called to process response.
    */
   function fetchPrinters(callback, all) {
    var query = 'q=^recent';
    if (all) {
@@ skipping 161 matching lines @@
    * @param {Object} cloud_print_data Data to be stored in cloudPrintOptions.
    * @param {function} add_callback The callback to be called to add the new
    *     printer to the print preview UI.
    * @param {function} update_caps_callback The callback to be called to update
    *     capabilities on the new printer.
    */
   function setDefaultPrinter(printer_name,
                              cloud_print_data,
                              add_callback,
                              update_caps_callback) {
-    var printer = add_callback([JSON.parse(cloud_print_data)]);
+    var printer = addCloudPrinters([JSON.parse(cloud_print_data)],
+                                   add_callback);
     if (printer)
       update_caps_callback(printer);
   }
 
   /**
    * Returns the data necessary to serialize a cloud print printer.
    * @param {Object} printer The printer object to get data for.
    * @return {string} A JSON string that can be used to recreate the
    *     cloud print portion of the printer object, or and empty string if
    *     there is no data to save.
@@ skipping 23 matching lines @@
    *     identify it.
    */
   function setCloudPrint(printer, name, id) {
     if (!printer.cloudPrintOptions) {
       printer.cloudPrintOptions = new Object;
     }
     printer.cloudPrintOptions.name = name;
     printer.cloudPrintOptions.id = id;
   }
 
+  /**
+   * Test if a particular cloud printer has already been added to the
+   * printer dropdown.
+   * @param {string} id A unique value to track this printer.
+   * @return {boolean} True if this id has previously been passed to

[dpapad, 2011/09/22 18:11:57]

Nit: True if |id| has ...

[Albert Bodenhamer, 2011/09/22 23:23:39]

Done.

+   *     trackCloudPrinterAdded.
+   */
+  function cloudPrinterAlreadyAdded(id) {
+    return (addedCloudPrinters[id]);

[dpapad, 2011/09/22 18:11:57]

Nit: No need for parenthesis.

[Albert Bodenhamer, 2011/09/22 23:23:39]

Done.

+  }
+
+  /**
+   * Record that a cloud printer will added to the printer dropdown.
+   * @param {string} id A unique value to track this printer.
+   * @return {boolean} False if adding this printer would exceed
+   *     |maxCloudPrinters|.
+   */
+  function trackCloudPrinterAdded(id) {
+    if (Object.keys(addedCloudPrinters).length < maxCloudPrinters) {
+      addedCloudPrinters[id] = true;
+      return true;
+    } else {
+      return false;
+    }
+  }
+
+  /**
+   * Add cloud printers to the list drop down.
+   * Called from the cloudprint object on receipt of printer information from
+   * the cloud print server.
+   * @param {Array} printers Array of printer info objects.
+   * @return {Object} The currently selected printer.
+   */
+  function addCloudPrinters(printers, addDestinationListOptionAtPosition) {
+    var isFirstPass = false;
+    var printerList = $('printer-list');
+
+    if (firstCloudPrintOptionPos == lastCloudPrintOptionPos) {
+      isFirstPass = true;
+      // Remove empty entry added by setDefaultPrinter.
+      if (printerList[0] && printerList[0].textContent == '')
+        printerList.remove(0);
+    }
+    if (printers != null) {
+      for (var i = 0; i < printers.length; i++) {
+        if (!cloudPrinterAlreadyAdded(printers[i]['id'])) {
+          if (!trackCloudPrinterAdded(printers[i]['id'])) {
+            break;
+          }
+          var option = addDestinationListOptionAtPosition(
+              lastCloudPrintOptionPos++,
+              printers[i]['name'],
+              printers[i]['id'],
+              printers[i]['name'] == defaultOrLastUsedPrinterName,
+              false,
+              false);
+          cloudprint.setCloudPrint(option,
+                                   printers[i]['name'],
+                                   printers[i]['id']);
+        }
+      }
+    } else {
+      if (!cloudPrinterAlreadyAdded(SIGN_IN)) {
+        addDestinationListOptionAtPosition(lastCloudPrintOptionPos++,
+                                           localStrings.getString('signIn'),
+                                           SIGN_IN,
+                                           false,
+                                           false,
+                                           false);
+        trackCloudPrinterAdded(SIGN_IN);
+      }
+    }
+    var selectedPrinter = printerList.selectedIndex;
+    if (selectedPrinter < 0)
+      return null;
+    return printerList.options[selectedPrinter];
+  }
+
   return {
+    addCloudPrinters: addCloudPrinters,
     colorIsDefault: colorIsDefault,
     fetchPrinters: fetchPrinters,
     getBaseURL: getBaseURL,
     getData: getData,
     getPrintTicketJSON: getPrintTicketJSON,
     isCloudPrint: isCloudPrint,
     printToCloud: printToCloud,
     setBaseURL: setBaseURL,
     setCloudPrint: setCloudPrint,
     setColor: setColor,
     setDefaultPrinter: setDefaultPrinter,
     supportsColor: supportsColor,
     updatePrinterCaps: updatePrinterCaps
   };
 });