Simplified print preview printer selection to be more consistent with Chrome

chrome/browser/resources/print_preview/print_preview_cloud.js

Index: chrome/browser/resources/print_preview/print_preview_cloud.js
diff --git a/chrome/browser/resources/print_preview/print_preview_cloud.js b/chrome/browser/resources/print_preview/print_preview_cloud.js
index 943f667c60ee2221ea7d2d6c8722e50480836ba3..d51dd2bf7c0e42b8923756eb488fccccc207756c 100644
--- a/chrome/browser/resources/print_preview/print_preview_cloud.js
+++ b/chrome/browser/resources/print_preview/print_preview_cloud.js
@@ -119,12 +119,12 @@ cr.define('cloudprint', function() {
       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.

     }
   }
 
@@ -306,7 +306,8 @@ cr.define('cloudprint', function() {
                              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);
   }
@@ -350,7 +351,86 @@ cr.define('cloudprint', function() {
     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,