Add print server entry to CupsPrintersEntryManager

- Adds a new printer entry type: PRINTSERVER
- Print server printers are treated as nearby printers
- Adds browser test to reflect this change.

Bug: 1015628
Test: browser test, End to end manual
Change-Id: I3a21f3af8be274fe47c7e57d2e4a349ea35b974c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1993754
Commit-Queue: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732902}

chrome/browser/resources/settings/chromeos/os_printing_page/cups_add_printer_dialog.js

@@ -495,7 +495,7 @@ Polymer({
    */
   onPrintServerAddedSucceeded_: function(printers) {
     this.inProgress_ = false;
-    this.fire('show-cups-print-server-toast', {printers: printers});
+    this.fire('add-print-server-and-show-toast', {printers: printers});
     this.$$('add-printer-dialog').close();
   },
 

chrome/browser/resources/settings/chromeos/os_printing_page/cups_nearby_printers.js

@@ -60,6 +60,7 @@ Polymer({
 
   listeners: {
     'add-automatic-printer': 'onAddAutomaticPrinter_',
+    'add-print-server-printer': 'onAddPrintServerPrinter_',
     'query-discovered-printer': 'onQueryDiscoveredPrinter_',
   },
 
@@ -104,6 +105,22 @@ Polymer({
             this.onAddNearbyPrinterFailed_.bind(this));
   },
 
+  /**
+   * @param {!CustomEvent<{item: !PrinterListEntry}>} e
+   * @private
+   */
+  onAddPrintServerPrinter_(e) {
+    const item = e.detail.item;
+    this.setActivePrinter_(item);
+
+    settings.CupsPrintersBrowserProxyImpl.getInstance()
+        .addCupsPrinter(item.printerInfo)
+        .then(
+            this.onAddNearbyPrintersSucceeded_.bind(
+                this, item.printerInfo.printerName),
+            this.onAddNearbyPrinterFailed_.bind(this));
+  },
+
   /**
    * @param {!CustomEvent<{item: !PrinterListEntry}>} e
    * @private

chrome/browser/resources/settings/chromeos/os_printing_page/cups_printer_types.js

@@ -20,6 +20,7 @@ let PrinterListEntry;
  */
 const PrinterType = {
   SAVED: 0,
-  AUTOMATIC: 1,
-  DISCOVERED: 2,
+  PRINTSERVER: 1,
+  AUTOMATIC: 2,
+  DISCOVERED: 3,
 };
\ No newline at end of file

chrome/browser/resources/settings/chromeos/os_printing_page/cups_printers.js

@@ -84,7 +84,7 @@ Polymer({
   listeners: {
     'edit-cups-printer-details': 'onShowCupsEditPrinterDialog_',
     'show-cups-printer-toast': 'openResultToast_',
-    'show-cups-print-server-toast': 'openPrintServerResultToast_',
+    'add-print-server-and-show-toast': 'addPrintServerAndShowResultToast_',
     'open-manufacturer-model-dialog-for-specified-printer':
         'openManufacturerModelDialogForSpecifiedPrinter_',
   },
@@ -190,7 +190,8 @@ Polymer({
    * }>} event
    * @private
    */
-  openPrintServerResultToast_: function(event) {
+  addPrintServerAndShowResultToast_: function(event) {
+    this.entryManager_.addPrintServerPrinters(event.detail.printers);
     const length = event.detail.printers.printerList.length;
     if (length === 0) {
       this.addPrintServerResultText_ =

chrome/browser/resources/settings/chromeos/os_printing_page/cups_printers_entry.html

@@ -37,7 +37,15 @@
       </template>
       <template is="dom-if"
           if="[[isAutomaticPrinter_(printerEntry.printerType)]]">
-        <cr-button id="savePrinterButton" on-click="onAddAutomaticPrinterTap_"
+        <cr-button class="save-printer-button"
+            on-click="onAddAutomaticPrinterTap_"
+            aria-label$="[[getSaveButtonAria_()]]">
+          $i18n{savePrinter}
+        </cr-button>
+      </template>
+      <template is="dom-if"
+          if="[[isPrintServerPrinter_(printerEntry.printerType)]]">
+        <cr-button class="save-printer-button" on-click="onAddServerPrinterTap_"
             aria-label$="[[getSaveButtonAria_()]]">
           $i18n{savePrinter}
         </cr-button>

chrome/browser/resources/settings/chromeos/os_printing_page/cups_printers_entry.js

@@ -33,14 +33,21 @@ Polymer({
     });
   },
 
+  /** @private */
   onAddDiscoveredPrinterTap_(e) {
     this.fire('query-discovered-printer', {item: this.printerEntry});
   },
 
+  /** @private */
   onAddAutomaticPrinterTap_() {
     this.fire('add-automatic-printer', {item: this.printerEntry});
   },
 
+  /** @private */
+  onAddServerPrinterTap_: function() {
+    this.fire('add-print-server-printer', {item: this.printerEntry});
+  },
+
   /**
    * @return {boolean}
    * @private
@@ -65,6 +72,14 @@ Polymer({
     return this.printerEntry.printerType == PrinterType.AUTOMATIC;
   },
 
+  /**
+   * @return {boolean}
+   * @private
+   */
+  isPrintServerPrinter_() {
+    return this.printerEntry.printerType == PrinterType.PRINTSERVER;
+  },
+
   getSaveButtonAria_() {
     return loadTimeData.getStringF(
         'savePrinterAria', this.printerEntry.printerInfo.printerName);

chrome/browser/resources/settings/chromeos/os_printing_page/cups_printers_entry_list_behavior.js

@@ -81,6 +81,13 @@ const CupsPrintersEntryListBehavior = {
    * @private
    */
   onNearbyPrintersChanged_(printerList) {
+    // |printerList| consists of automatic and discovered printers that have
+    // not been saved and are available. Add all unsaved print server printers
+    // to |printerList|.
+    this.entryManager_.printServerPrinters = settings.printing.findDifference(
+        this.entryManager_.printServerPrinters, this.savedPrinters);
+    printerList = printerList.concat(this.entryManager_.printServerPrinters);
+
     this.updateList(
         'nearbyPrinters', printer => printer.printerInfo.printerId,
         printerList);

chrome/browser/resources/settings/chromeos/os_printing_page/cups_printers_entry_manager.js

@@ -23,8 +23,9 @@ let PrintersListCallback;
 
 cr.define('settings.printing', function() {
   /**
-   * Class for managing printer entries. Holds both Saved and Nearby printers
-   * and notifies observers of any applicable changes to either printer lists.
+   * Class for managing printer entries. Holds both Saved, Nearby, Print Server
+   * printers and notifies observers of any applicable changes to either printer
+   * lists.
    */
   class CupsPrintersEntryManager {
     constructor() {
@@ -37,6 +38,9 @@ cr.define('settings.printing', function() {
       /** @private {!Array<PrintersListWithDeltasCallback>} */
       this.onSavedPrintersChangedListeners_ = [];
 
+      /** @type {!Array<!PrinterListEntry>} */
+      this.printServerPrinters = [];
+
       /** @type {!Array<PrintersListCallback>} */
       this.onNearbyPrintersChangedListeners_ = [];
     }
@@ -52,6 +56,7 @@ cr.define('settings.printing', function() {
 
     removeWebUIListeners() {
       cr.removeWebUIListener('on-nearby-printers-changed');
+      cr.removeWebUIListener('on-print-server-added');
     }
 
     /** @return {!Array<!PrinterListEntry>} */
@@ -142,6 +147,29 @@ cr.define('settings.printing', function() {
       this.notifyOnNearbyPrintersChangedListeners_();
     }
 
+    /**
+     * Adds the found print server printers to |printServerPrinters|.
+     * |foundPrinters| consist of print server printers that have not been saved
+     * and will appear in the nearby printers list.
+     * @param {!CupsPrintersList} foundPrinters
+     */
+    addPrintServerPrinters(foundPrinters) {
+      // Get only new printers from |foundPrinters|. We ignore previously
+      // found printers.
+      const newPrinters = foundPrinters.printerList.filter(p1 => {
+        return !this.printServerPrinters.some(
+            p2 => p2.printerInfo.printerId == p1.printerId);
+      });
+
+      for (const printer of newPrinters) {
+        this.printServerPrinters.push(
+            {printerInfo: printer, printerType: PrinterType.PRINTSERVER});
+      }
+
+      // All printers from print servers are treated as nearby printers.
+      this.notifyOnNearbyPrintersChangedListeners_();
+    }
+
     /**
      * Non-empty/null fields indicate the applicable change to be notified.
      * @param {!Array<!PrinterListEntry>} savedPrinters

chrome/test/data/webui/settings/chromeos/cups_printer_landing_page_tests.js

@@ -20,7 +20,7 @@ function clickThreeDotMenu(printerEntry) {
  */
 function clickAddAutomaticButton(printerEntry) {
   // Click on add button on an item entry.
-  const addButton = printerEntry.$$('#savePrinterButton');
+  const addButton = printerEntry.$$('.save-printer-button');
   assertTrue(!!addButton);
   addButton.click();
   Polymer.dom.flush();
@@ -1163,7 +1163,7 @@ suite('CupsNearbyPrintersTests', function() {
           nearbyPrinterEntries =
               cups_printer_test_util.getPrinterEntries(nearbyPrintersElement);
           assertEquals(1, nearbyPrinterEntries.length);
-          assertTrue(!!nearbyPrinterEntries[0].$$('#savePrinterButton'));
+          assertTrue(!!nearbyPrinterEntries[0].$$('.save-printer-button'));
 
           // Add an automatic printer and assert that that the toast
           // notification is shown.

chrome/test/data/webui/settings/chromeos/cups_printer_page_tests.js

@@ -1255,6 +1255,9 @@ suite('PrintServerTests', function() {
   let page = null;
   let dialog = null;
 
+  /** @type {?settings.printing.CupsPrintersEntryManager} */
+  let entryManager = null;
+
   /** @type {?settings.TestCupsPrintersBrowserProxy} */
   let cupsPrintersBrowserProxy = null;
 
@@ -1268,6 +1271,11 @@ suite('PrintServerTests', function() {
 
   setup(function() {
     const mojom = chromeos.networkConfig.mojom;
+    entryManager = settings.printing.CupsPrintersEntryManager.getInstance();
+    setEntryManagerPrinters(
+        /*savedPrinters=*/[], /*automaticPrinters=*/[],
+        /*discoveredPrinters=*/[], /*printServerPrinters=*/[]);
+
     cupsPrintersBrowserProxy =
         new printerBrowserProxy.TestCupsPrintersBrowserProxy;
 
@@ -1300,6 +1308,20 @@ suite('PrintServerTests', function() {
     page = null;
   });
 
+  /**
+   * @param {!Array<!PrinterListEntry>} savedPrinters
+   * @param {!Array<!CupsPrinterInfo>} automaticPrinters
+   * @param {!Array<!CupsPrinterInfo>} discoveredPrinters
+   * @param {!Array<!PrinterListEntry>} printerServerPrinters
+   */
+  function setEntryManagerPrinters(
+      savedPrinters, automaticPrinters, discoveredPrinters,
+      printerServerPrinters) {
+    entryManager.setSavedPrintersList(savedPrinters);
+    entryManager.setNearbyPrintersList(automaticPrinters, discoveredPrinters);
+    entryManager.printServerPrinters = printerServerPrinters;
+  }
+
   /**
    * @param {!HTMLElement} page
    * @return {?HTMLElement} Returns the print server dialog if it is available.
@@ -1366,34 +1388,140 @@ suite('PrintServerTests', function() {
     assertEquals(loadTimeData.getString(expectedError), dialogError.errorText);
   }
 
+  /**
+   * @param {string} expectedMessage
+   * @param {number} numPrinters
+   * @private
+   */
+  function verifyToastMessage(expectedMessage, numPrinters) {
+    // We always display the total number of printers found from a print
+    // server.
+    const toast = page.$$('#printServerErrorToast');
+    assertTrue(toast.open);
+    assertEquals(
+        loadTimeData.getStringF(expectedMessage, numPrinters),
+        toast.textContent.trim());
+  }
+
   test('AddPrintServerIsSuccessful', function() {
     // Initialize the return result from adding a print server.
     cupsPrintersBrowserProxy.printServerPrinters =
-        /** @type{} CupsPrintServerPrintersInfo*/ ({
+        /** @type{!CupsPrintServerPrintersInfo} */ ({
           printerList: [
             cups_printer_test_util.createCupsPrinterInfo(
-                'nameA', 'addressA', 'idA'),
+                'nameA', 'serverAddress', 'idA'),
             cups_printer_test_util.createCupsPrinterInfo(
-                'nameB', 'addressB', 'idB')
+                'nameB', 'serverAddress', 'idB')
           ]
         });
-    return addPrintServer('serverAddressA', PrintServerResult.NO_ERRORS)
+    return addPrintServer('serverAddress', PrintServerResult.NO_ERRORS)
         .then(() => {
           Polymer.dom.flush();
-          const toast = page.$$('#printServerErrorToast');
-          assertTrue(toast.open);
-          assertEquals(
-              loadTimeData.getStringF(
-                  'printServerFoundManyPrinters',
-                  /*expectedPrintersLength=*/ 2),
-              toast.textContent.trim());
+          verifyToastMessage(
+              'printServerFoundManyPrinters', /*numPrinters=*/ 2);
+          assertEquals(2, entryManager.printServerPrinters.length);
         });
   });
 
+  test('HandleDuplicateQueries', function() {
+    // Initialize the return result from adding a print server.
+    cupsPrintersBrowserProxy.printServerPrinters =
+        /** @type{!CupsPrintServerPrintersInfo} */ ({
+          printerList: [
+            cups_printer_test_util.createCupsPrinterInfo(
+                'nameA', 'serverAddress', 'idA'),
+            cups_printer_test_util.createCupsPrinterInfo(
+                'nameB', 'serverAddress', 'idB')
+          ]
+        });
+
+    return test_util.flushTasks()
+        .then(() => {
+          // Simulate that a print server was queried previously.
+          setEntryManagerPrinters(
+              /*savedPrinters=*/[], /*nearbyPrinters=*/[],
+              /*discoveredPrinters=*/[], [
+                cups_printer_test_util.createPrinterListEntry(
+                    'nameA', 'serverAddress', 'idA', PrinterType.PRINTSERVER),
+                cups_printer_test_util.createPrinterListEntry(
+                    'nameB', 'serverAddress', 'idB', PrinterType.PRINTSERVER)
+              ]);
+          Polymer.dom.flush();
+          assertEquals(2, entryManager.printServerPrinters.length);
+
+          // This will attempt to add duplicate print server printers.
+          // Matching printerId's are considered duplicates.
+          return addPrintServer('serverAddress', PrintServerResult.NO_ERRORS);
+        })
+        .then(() => {
+          Polymer.dom.flush();
+
+          verifyToastMessage(
+              'printServerFoundManyPrinters', /*numPrinters=*/ 2);
+          // Assert that adding the same print server results in no new printers
+          // added to the entry manager.
+          assertEquals(2, entryManager.printServerPrinters.length);
+          const nearbyPrintersElement =
+              page.$$('settings-cups-nearby-printers');
+          assertEquals(2, nearbyPrintersElement.nearbyPrinters.length);
+        });
+  });
+
+  test('HandleDuplicateSavedPrinters', function() {
+    // Initialize the return result from adding a print server.
+    cupsPrintersBrowserProxy.printServerPrinters =
+        /** @type{} CupsPrintServerPrintersInfo*/ ({
+          printerList: [
+            cups_printer_test_util.createCupsPrinterInfo(
+                'nameA', 'serverAddress', 'idA'),
+            cups_printer_test_util.createCupsPrinterInfo(
+                'nameB', 'serverAddress', 'idB')
+          ]
+        });
+
+    return test_util.flushTasks().then(() => {
+      // Simulate that a print server was queried previously.
+      setEntryManagerPrinters(
+          /*savedPrinters=*/[], /*nearbyPrinters=*/[],
+          /*discoveredPrinters=*/[], [
+            cups_printer_test_util.createPrinterListEntry(
+                'nameA', 'serverAddress', 'idA', PrinterType.PRINTSERVER),
+            cups_printer_test_util.createPrinterListEntry(
+                'nameB', 'serverAddress', 'idB', PrinterType.PRINTSERVER)
+          ]);
+      Polymer.dom.flush();
+      assertEquals(2, entryManager.printServerPrinters.length);
+
+      // Simulate adding a saved printer.
+      entryManager.setSavedPrintersList(
+          [cups_printer_test_util.createPrinterListEntry(
+              'nameA', 'serverAddress', 'idA', PrinterType.SAVED)]);
+      Polymer.dom.flush();
+
+      // Simulate the underlying model changes. Nearby printers are also
+      // updated after changes to saved printers.
+      cr.webUIListenerCallback(
+          'on-nearby-printers-changed', /*automaticPrinter=*/[],
+          /*discoveredPrinters=*/[]);
+      Polymer.dom.flush();
+
+      // Verify that we now only have 1 printer in print server printers
+      // list.
+      assertEquals(1, entryManager.printServerPrinters.length);
+      const nearbyPrintersElement = page.$$('settings-cups-nearby-printers');
+      assertEquals(1, nearbyPrintersElement.nearbyPrinters.length);
+      // Verify we correctly removed the duplicate printer, 'idA', since
+      // it exists in the saved printer list. Expect only 'idB' in
+      // the print server printers list.
+      assertEquals(
+          'idB', entryManager.printServerPrinters[0].printerInfo.printerId);
+    });
+  });
+
   test('AddPrintServerAddressError', function() {
     cupsPrintersBrowserProxy.printServerPrinters =
         /** @type{} CupsPrintServerPrintersInfo*/ ({printerList: []});
-    return addPrintServer('serverAddressA', PrintServerResult.INCORRECT_URL)
+    return addPrintServer('serverAddress', PrintServerResult.INCORRECT_URL)
         .then(() => {
           Polymer.dom.flush();
           const printServerDialog = getPrintServerDialog(page);
@@ -1407,7 +1535,7 @@ suite('PrintServerTests', function() {
   test('AddPrintServerConnectionError', function() {
     cupsPrintersBrowserProxy.printServerPrinters =
         /** @type{} CupsPrintServerPrintersInfo*/ ({printerList: []});
-    return addPrintServer('serverAddressA', PrintServerResult.CONNECTION_ERROR)
+    return addPrintServer('serverAddress', PrintServerResult.CONNECTION_ERROR)
         .then(() => {
           Polymer.dom.flush();
           verifyErrorMessage('printServerConnectionError');
@@ -1418,7 +1546,7 @@ suite('PrintServerTests', function() {
     cupsPrintersBrowserProxy.printServerPrinters =
         /** @type{} CupsPrintServerPrintersInfo*/ ({printerList: []});
     return addPrintServer(
-               'serverAddressA', PrintServerResult.CANNOT_PARSE_IPP_RESPONSE)
+               'serverAddress', PrintServerResult.CANNOT_PARSE_IPP_RESPONSE)
         .then(() => {
           Polymer.dom.flush();
           verifyErrorMessage('printServerConfigurationErrorMessage');
@@ -1428,7 +1556,7 @@ suite('PrintServerTests', function() {
   test('AddPrintServerReachableServerButHttpResponseError', function() {
     cupsPrintersBrowserProxy.printServerPrinters =
         /** @type{} CupsPrintServerPrintersInfo*/ ({printerList: []});
-    return addPrintServer('serverAddressA', PrintServerResult.HTTP_ERROR)
+    return addPrintServer('serverAddress', PrintServerResult.HTTP_ERROR)
         .then(() => {
           Polymer.dom.flush();
           verifyErrorMessage('printServerConfigurationErrorMessage');