Hide saved printers when empty

- The header for saved printers will be hidden when the saved printers
  list is empty.
- Adds a browsertest to reflect this change.

Bug: 993819
Test: browser tests, end to end manual
Change-Id: I4bbae8f389161a7c3252df09d39380c896413fa2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1767488
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@{#691391}

chrome/browser/resources/settings/printing_page/BUILD.gn

@@ -103,6 +103,7 @@ if (is_chromeos) {
       ":cups_nearby_printers",
       ":cups_printers_browser_proxy",
       ":cups_saved_printers",
+      "..:route",
       "//ui/webui/resources/cr_components/chromeos/network:mojo_interface_provider",
       "//ui/webui/resources/cr_elements/chromeos/network:cr_network_listener_behavior",
       "//ui/webui/resources/cr_elements/cr_toast:cr_toast",

chrome/browser/resources/settings/printing_page/cups_printers.html

@@ -19,6 +19,7 @@
 <link rel="import" href="cups_printers_list.html">
 <link rel="import" href="cups_saved_printers.html">
 <link rel="import" href="cups_nearby_printers.html">
+<link rel="import" href="../route.html">
 
 <dom-module id="settings-cups-printers">
   <template>
@@ -121,16 +122,21 @@
         </div>
       </template>
 
-      <div class="settings-box first">
-        <div class="start">
-          <span>$i18n{savedPrintersTitle}</span>
+      <template is="dom-if"
+          if="[[doesAccountHaveSavedPrinters_(savedPrinters_)]]"
+          id="savedPrintersSection" restamp>
+        <div class="settings-box first">
+          <div class="start">
+            <span>$i18n{savedPrintersTitle}</span>
+          </div>
         </div>
-      </div>
 
-      <settings-cups-saved-printers id="savedPrinters"
-          active-printer="{{activePrinter}}"
-          search-term="[[searchTerm]]">
-      </settings-cups-saved-printers>
+        <settings-cups-saved-printers id="savedPrinters"
+            active-printer="{{activePrinter}}"
+            saved-printers="[[savedPrinters_]]"
+            search-term="[[searchTerm]]">
+        </settings-cups-saved-printers>
+      </template>
 
       <div class="padded first" id="nearbyPrinters">
         <div>$i18n{nearbyPrintersListTitle}</div>

chrome/browser/resources/settings/printing_page/cups_printers.js

@@ -14,6 +14,7 @@ Polymer({
 
   behaviors: [
       CrNetworkListenerBehavior,
+      settings.RouteObserverBehavior,
       WebUIListenerBehavior,
   ],
 
@@ -42,6 +43,15 @@ Polymer({
       reflectToAttribute: true,
     },
 
+    /**
+     * @type {!Array<!PrinterListEntry>}
+     * @private
+     */
+    savedPrinters_: {
+      type: Array,
+      value: () => [],
+    },
+
     /** @private */
     showCupsEditPrinterDialog_: Boolean,
 
@@ -93,9 +103,6 @@ Polymer({
     if (this.enableUpdatedUi_) {
       return;
     }
-
-    this.addWebUIListener(
-        'on-printers-changed', this.printersChanged_.bind(this));
   },
 
   /** @override */
@@ -103,6 +110,21 @@ Polymer({
     this.updateCupsPrintersList_();
   },
 
+
+  /**
+   * settings.RouteObserverBehavior
+   * @param {!settings.Route} route
+   * @protected
+   */
+  currentRouteChanged: function(route) {
+    if (route != settings.routes.CUPS_PRINTERS) {
+      cr.removeWebUIListener('on-printers-changed');
+      return;
+    }
+    cr.addWebUIListener(
+        'on-printers-changed', this.onPrintersChanged_.bind(this));
+  },
+
   /**
    * CrosNetworkConfigObserver impl
    * @param {!Array<chromeos.networkConfig.mojom.NetworkStateProperties>}
@@ -130,21 +152,13 @@ Polymer({
     const printerName = event.detail.printerName;
     switch (event.detail.resultCode) {
       case PrinterSetupResult.SUCCESS:
-        if (this.enableUpdatedUi_) {
-          this.$$('#savedPrinters').updateSavedPrintersList();
-        } else {
-          this.updateCupsPrintersList_();
-        }
+        this.updateCupsPrintersList_();
         this.addPrinterResultText_ =
             loadTimeData.getStringF('printerAddedSuccessfulMessage',
                                     printerName);
         break;
       case PrinterSetupResult.EDIT_SUCCESS:
-        if (this.enableUpdatedUi_) {
-          this.$$('#savedPrinters').updateSavedPrintersList();
-        } else {
-          this.updateCupsPrintersList_();
-        }
+        this.updateCupsPrintersList_();
         this.addPrinterResultText_ =
             loadTimeData.getStringF('printerEditedSuccessfulMessage',
                                     printerName);
@@ -176,15 +190,22 @@ Polymer({
   updateCupsPrintersList_: function() {
     settings.CupsPrintersBrowserProxyImpl.getInstance()
         .getCupsPrintersList()
-        .then(this.printersChanged_.bind(this));
+        .then(this.onPrintersChanged_.bind(this));
   },
 
   /**
    * @param {!CupsPrintersList} cupsPrintersList
    * @private
    */
-  printersChanged_: function(cupsPrintersList) {
-    this.printers = cupsPrintersList.printerList;
+  onPrintersChanged_: function(cupsPrintersList) {
+    if (this.enableUpdatedUi_) {
+      this.savedPrinters_ = cupsPrintersList.printerList.map(
+          printer => /** @type {!PrinterListEntry} */({
+              printerInfo: printer,
+              printerType: PrinterType.SAVED}));
+    } else {
+      this.printers = cupsPrintersList.printerList;
+    }
   },
 
   /** @private */
@@ -235,5 +256,13 @@ Polymer({
   addPrinterButtonActive_: function(
       connectedToNetwork, userNativePrintersAllowed) {
     return connectedToNetwork && userNativePrintersAllowed;
+  },
+
+  /**
+   * @return {boolean} Whether |savedPrinters_| is empty.
+   * @private
+   */
+  doesAccountHaveSavedPrinters_: function() {
+    return !!this.savedPrinters_.length;
   }
 });

chrome/browser/resources/settings/printing_page/cups_saved_printers.html

@@ -19,7 +19,7 @@
     </cr-action-menu>
 
     <style include="settings-shared"></style>
-    <settings-cups-printers-entry-list printers="[[savedPrinters_]]"
+    <settings-cups-printers-entry-list printers="[[savedPrinters]]"
         search-term="[[searchTerm]]">
     </settings-cups-printers-entry-list>
   </template>

chrome/browser/resources/settings/printing_page/cups_saved_printers.js

@@ -14,17 +14,13 @@ Polymer({
   ],
 
   properties: {
-    /**
-     * @type {!Array<!PrinterListEntry>}
-     * @private
-     */
-    savedPrinters_: {
+    /** @type {!Array<!PrinterListEntry>} */
+    savedPrinters: {
       type: Array,
-      value: () => [],
     },
 
     /**
-     * Search term for filtering |savedPrinters_|.
+     * Search term for filtering |savedPrinters|.
      * @type {string}
      */
     searchTerm: {
@@ -60,35 +56,6 @@ Polymer({
     this.browserProxy_ = settings.CupsPrintersBrowserProxyImpl.getInstance();
   },
 
-  /** @override */
-  ready: function() {
-    this.addWebUIListener(
-        'on-printers-changed', this.printersChanged_.bind(this));
-    this.updateSavedPrintersList();
-  },
-
-  /** Public function to update the printer list. */
-  updateSavedPrintersList: function() {
-    settings.CupsPrintersBrowserProxyImpl.getInstance()
-        .getCupsPrintersList()
-        .then(this.printersChanged_.bind(this));
-  },
-
-  /**
-   * @param {!CupsPrintersList} cupsPrintersList
-   * @private
-   */
-  printersChanged_: function(cupsPrintersList) {
-    if (!cupsPrintersList) {
-      return;
-    }
-
-    this.savedPrinters_ = cupsPrintersList.printerList.map(
-        printer => /** @type {!PrinterListEntry} */({
-            printerInfo: printer,
-            printerType: PrinterType.SAVED}));
-  },
-
   /**
    * @param {!CustomEvent<{target: !HTMLElement, item: !PrinterListEntry}>} e
    * @private
@@ -96,10 +63,10 @@ Polymer({
   onOpenActionMenu_: function(e) {
     const item = /** @type {!PrinterListEntry} */(e.detail.item);
     this.activePrinterListEntryIndex_ =
-        this.savedPrinters_.findIndex(
+        this.savedPrinters.findIndex(
             printer => printer.printerInfo == item.printerInfo);
     this.activePrinter =
-        this.get(['savedPrinters_', this.activePrinterListEntryIndex_])
+        this.get(['savedPrinters', this.activePrinterListEntryIndex_])
         .printerInfo;
 
     const target = /** @type {!HTMLElement} */ (e.detail.target);
@@ -115,7 +82,6 @@ Polymer({
 
   /** @private */
   onRemoveTap_: function() {
-    this.splice('savedPrinters_', this.activePrinterListEntryIndex_, 1);
     this.browserProxy_.removeCupsPrinter(
         this.activePrinter.printerId, this.activePrinter.printerName);
     this.activePrinter = null;

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

@@ -130,34 +130,52 @@ function verifyPrintersList(entryList, printerList) {
 }
 
 /**
- * Removes a saved printer located at |index| and then verify that saved
- * printers list is updated accordingly.
+ * Removes a saved printer located at |index|.
  * @param {!TestCupsPrintersBrowserProxy} cupsPrintersBrowserProxy
- * @param {!HTMLElement} savedPrinters
- * @param {!HTMLElement} entryList
- * @param {!Array<!CupsPrinterInfo>} printerList
+ * @param {!HTMLElement} savedPrintersElement
  * @param {number} index
  * @return {!Promise}
  */
-function removeAndVerifyPrinters(
-    cupsPrintersBrowserProxy, savedPrinters, entryList, printerList, index) {
-  clickThreeDotMenu(entryList[index]);
-  savedPrinters.$$('#removeButton').click();
+function removePrinter(cupsPrintersBrowserProxy, savedPrintersElement, index) {
+  let printerList = cupsPrintersBrowserProxy.printerList.printerList;
+  let savedPrinterEntries = getPrinterEntries(savedPrintersElement);
+
+  clickThreeDotMenu(savedPrinterEntries[index]);
+  savedPrintersElement.$$('#removeButton').click();
 
   return cupsPrintersBrowserProxy.whenCalled('removeCupsPrinter')
       .then(function() {
         // Simulate removing the printer from |cupsPrintersBrowserProxy|.
         printerList.splice(index, 1);
 
+        // Simuluate saved printer changes.
+        cr.webUIListenerCallback(
+            'on-printers-changed', cupsPrintersBrowserProxy.printerList);
         Polymer.dom.flush();
-        return cupsPrintersBrowserProxy.whenCalled('getCupsPrintersList');
-      })
-      .then(function() {
-        entryList = getPrinterEntries(savedPrinters);
-        verifyPrintersList(entryList, printerList);
       });
 }
 
+/**
+ * Removes all saved printers through recursion.
+ * @param {!TestCupsPrintersBrowserProxy} cupsPrintersBrowserProxy
+ * @param {!HTMLElement} savedPrintersElement
+ * @return {!Promise}
+ */
+function removeAllPrinters(cupsPrintersBrowserProxy, savedPrintersElement) {
+  let printerList = cupsPrintersBrowserProxy.printerList.printerList;
+  let savedPrinterEntries = getPrinterEntries(savedPrintersElement);
+
+  if (!printerList.length) {
+    return Promise.resolve();
+  }
+
+  return removePrinter(
+             cupsPrintersBrowserProxy, savedPrintersElement, 0 /* index */)
+      .then(test_util.flushTasks)
+      .then(removeAllPrinters.bind(
+          this, cupsPrintersBrowserProxy, savedPrintersElement));
+}
+
 /**
  * @param {string} printerName
  * @param {string} printerAddress
@@ -237,6 +255,8 @@ suite('CupsSavedPrintersTests', function() {
     api_.enableNetworkType('WiFi');
 
     PolymerTest.clearBody();
+    settings.navigateTo(settings.routes.CUPS_PRINTERS);
+
     page = document.createElement('settings-cups-printers');
     // Enable feature flag to show the new saved printers list.
     // TODO(jimmyxgong): Remove this line when the feature flag is removed.
@@ -245,9 +265,6 @@ suite('CupsSavedPrintersTests', function() {
     assertTrue(!!page);
 
     Polymer.dom.flush();
-
-    savedPrintersElement = page.$$('settings-cups-saved-printers');
-    assertTrue(!!savedPrintersElement);
   });
 
   teardown(function() {
@@ -260,49 +277,63 @@ suite('CupsSavedPrintersTests', function() {
   });
 
   test('SavedPrintersSuccessfullyPopulates', function() {
-    // List component contained by CupsSavedPrinters.
-    const savedPrintersList =
-        savedPrintersElement.$$('settings-cups-printers-entry-list');
+    // Wait for saved printers to populate.
+    return test_util.flushTasks().then(() => {
+      savedPrintersElement = page.$$('settings-cups-saved-printers');
+      assertTrue(!!savedPrintersElement);
 
-    return cupsPrintersBrowserProxy.whenCalled('getCupsPrintersList')
-        .then(function() {
-          // Wait for saved printers to populate.
-          Polymer.dom.flush();
+      // List component contained by CupsSavedPrinters.
+      const savedPrintersList =
+          savedPrintersElement.$$('settings-cups-printers-entry-list');
 
-          const printerListEntries = getPrinterEntries(savedPrintersElement);
+      const printerListEntries = getPrinterEntries(savedPrintersElement);
 
-          verifyPrintersList(printerListEntries, printerList);
-        });
+      verifyPrintersList(printerListEntries, printerList);
+    });
   });
 
   test('SuccessfullyRemoveMultipleSavedPrinters', function() {
     let savedPrinterEntries = [];
 
-    return cupsPrintersBrowserProxy.whenCalled('getCupsPrintersList')
-        .then(function() {
-          // Wait for saved printers to populate.
-          Polymer.dom.flush();
+    // Wait for saved printers to populate.
+    return test_util.flushTasks()
+        .then(() => {
+          savedPrintersElement = page.$$('settings-cups-saved-printers');
+          assertTrue(!!savedPrintersElement);
 
+          return removeAllPrinters(
+              cupsPrintersBrowserProxy, savedPrintersElement);
+        })
+        .then(() => {
+          let entryList = getPrinterEntries(savedPrintersElement);
+          verifyPrintersList(entryList, printerList);
+        });
+  });
+
+  test('HideSavedPrintersWhenEmpty', function() {
+    // List component contained by CupsSavedPrinters.
+    let savedPrintersList = [];
+    let savedPrinterEntries = [];
+
+    // Wait for saved printers to populate.
+    return test_util.flushTasks()
+        .then(() => {
+          savedPrintersElement = page.$$('settings-cups-saved-printers');
+          assertTrue(!!savedPrintersElement);
+
+          savedPrintersList =
+              savedPrintersElement.$$('settings-cups-printers-entry-list');
           savedPrinterEntries = getPrinterEntries(savedPrintersElement);
 
-          return removeAndVerifyPrinters(
-              cupsPrintersBrowserProxy, savedPrintersElement,
-              savedPrinterEntries, printerList, 1 /** index */);
-        })
-        .then(function() {
-          return removeAndVerifyPrinters(
-              cupsPrintersBrowserProxy, savedPrintersElement,
-              savedPrinterEntries, printerList, 0 /** index */);
-        })
-        .then(function() {
-          return removeAndVerifyPrinters(
-              cupsPrintersBrowserProxy, savedPrintersElement,
-              savedPrinterEntries, printerList, 1 /** index */);
+          verifyPrintersList(savedPrinterEntries, printerList);
+
+          assertTrue(!!page.$$('#savedPrinters'));
+
+          return removeAllPrinters(
+              cupsPrintersBrowserProxy, savedPrintersElement);
         })
-        .then(function() {
-          return removeAndVerifyPrinters(
-              cupsPrintersBrowserProxy, savedPrintersElement,
-              savedPrinterEntries, printerList, 0 /** index */);
+        .then(() => {
+          assertFalse(!!page.$$('#savedPrinters'));
         });
   });
 
@@ -312,10 +343,11 @@ suite('CupsSavedPrintersTests', function() {
     let editDialog = null;
     let savedPrinterEntries = null;
 
-    return cupsPrintersBrowserProxy.whenCalled('getCupsPrintersList')
-        .then(function() {
-          // Wait for saved printers to populate.
-          Polymer.dom.flush();
+    // Wait for saved printers to populate.
+    return test_util.flushTasks()
+        .then(() => {
+          savedPrintersElement = page.$$('settings-cups-saved-printers');
+          assertTrue(!!savedPrintersElement);
 
           savedPrinterEntries = getPrinterEntries(savedPrintersElement);
 
@@ -339,7 +371,7 @@ suite('CupsSavedPrintersTests', function() {
 
           return cupsPrintersBrowserProxy.whenCalled('updateCupsPrinter');
         })
-        .then(function() {
+        .then(() => {
           assertEquals(expectedName, editDialog.activePrinter.printerName);
 
           // Mimic changes to |cupsPrintersBrowserProxy.printerList|.
@@ -356,10 +388,11 @@ suite('CupsSavedPrintersTests', function() {
     let savedPrinterEntries = null;
     let editDialog = null;
 
-    return cupsPrintersBrowserProxy.whenCalled('getCupsPrintersList')
-        .then(function() {
-          // Wait for saved printers to populate.
-          Polymer.dom.flush();
+    // Wait for saved printers to populate.
+    return test_util.flushTasks()
+        .then(() => {
+          savedPrintersElement = page.$$('settings-cups-saved-printers');
+          assertTrue(!!savedPrintersElement);
 
           savedPrinterEntries = getPrinterEntries(savedPrintersElement);
 
@@ -387,7 +420,7 @@ suite('CupsSavedPrintersTests', function() {
 
           return cupsPrintersBrowserProxy.whenCalled('reconfigureCupsPrinter');
         })
-        .then(function() {
+        .then(() => {
           assertEquals(expectedName, editDialog.activePrinter.printerName);
           assertEquals(
               expectedAddress, editDialog.activePrinter.printerAddress);
@@ -458,6 +491,8 @@ suite('CupsNearbyPrintersTests', function() {
     setNetworksForTest(api_, activeNetworks_);
 
     PolymerTest.clearBody();
+    settings.navigateTo(settings.routes.CUPS_PRINTERS);
+
     page = document.createElement('settings-cups-printers');
     // Enable feature flag to show the new saved printers list.
     // TODO(jimmyxgong): Remove this line when the feature flag is removed.