Revert "Hide saved printers when empty"

This reverts commit 6268a44d.

Reason for revert: Test fails on ChromeOS, see: https://crbug.com/999072

Original change's description:
> 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}

TBR=khorimoto@chromium.org,zentaro@chromium.org,baileyberro@chromium.org,jimmyxgong@chromium.org

Change-Id: Iee2c84d5e9aacd691bb695229a2c26d8ffff1b2a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 993819, 999072
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1776039Reviewed-by: Oskar Sundbom <ossu@chromium.org>
Commit-Queue: Oskar Sundbom <ossu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691547}

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

@@ -103,7 +103,6 @@ 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,7 +19,6 @@
 <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>
@@ -122,9 +121,6 @@
         </div>
       </template>
 
-      <template is="dom-if"
-          if="[[doesAccountHaveSavedPrinters_(savedPrinters_)]]"
-          id="savedPrintersSection" restamp>
       <div class="settings-box first">
         <div class="start">
           <span>$i18n{savedPrintersTitle}</span>
@@ -133,10 +129,8 @@
 
       <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,7 +14,6 @@ Polymer({
 
   behaviors: [
       CrNetworkListenerBehavior,
-      settings.RouteObserverBehavior,
       WebUIListenerBehavior,
   ],
 
@@ -43,15 +42,6 @@ Polymer({
       reflectToAttribute: true,
     },
 
-    /**
-     * @type {!Array<!PrinterListEntry>}
-     * @private
-     */
-    savedPrinters_: {
-      type: Array,
-      value: () => [],
-    },
-
     /** @private */
     showCupsEditPrinterDialog_: Boolean,
 
@@ -103,6 +93,9 @@ Polymer({
     if (this.enableUpdatedUi_) {
       return;
     }
+
+    this.addWebUIListener(
+        'on-printers-changed', this.printersChanged_.bind(this));
   },
 
   /** @override */
@@ -110,21 +103,6 @@ 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>}
@@ -152,13 +130,21 @@ Polymer({
     const printerName = event.detail.printerName;
     switch (event.detail.resultCode) {
       case PrinterSetupResult.SUCCESS:
+        if (this.enableUpdatedUi_) {
+          this.$$('#savedPrinters').updateSavedPrintersList();
+        } else {
           this.updateCupsPrintersList_();
+        }
         this.addPrinterResultText_ =
             loadTimeData.getStringF('printerAddedSuccessfulMessage',
                                     printerName);
         break;
       case PrinterSetupResult.EDIT_SUCCESS:
+        if (this.enableUpdatedUi_) {
+          this.$$('#savedPrinters').updateSavedPrintersList();
+        } else {
           this.updateCupsPrintersList_();
+        }
         this.addPrinterResultText_ =
             loadTimeData.getStringF('printerEditedSuccessfulMessage',
                                     printerName);
@@ -190,22 +176,15 @@ Polymer({
   updateCupsPrintersList_: function() {
     settings.CupsPrintersBrowserProxyImpl.getInstance()
         .getCupsPrintersList()
-        .then(this.onPrintersChanged_.bind(this));
+        .then(this.printersChanged_.bind(this));
   },
 
   /**
    * @param {!CupsPrintersList} cupsPrintersList
    * @private
    */
-  onPrintersChanged_: function(cupsPrintersList) {
-    if (this.enableUpdatedUi_) {
-      this.savedPrinters_ = cupsPrintersList.printerList.map(
-          printer => /** @type {!PrinterListEntry} */({
-              printerInfo: printer,
-              printerType: PrinterType.SAVED}));
-    } else {
+  printersChanged_: function(cupsPrintersList) {
     this.printers = cupsPrintersList.printerList;
-    }
   },
 
   /** @private */
@@ -256,13 +235,5 @@ 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,13 +14,17 @@ Polymer({
   ],
 
   properties: {
-    /** @type {!Array<!PrinterListEntry>} */
-    savedPrinters: {
+    /**
+     * @type {!Array<!PrinterListEntry>}
+     * @private
+     */
+    savedPrinters_: {
       type: Array,
+      value: () => [],
     },
 
     /**
-     * Search term for filtering |savedPrinters|.
+     * Search term for filtering |savedPrinters_|.
      * @type {string}
      */
     searchTerm: {
@@ -56,6 +60,35 @@ 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
@@ -63,10 +96,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);
@@ -82,6 +115,7 @@ 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,52 +130,34 @@ function verifyPrintersList(entryList, printerList) {
 }
 
 /**
- * Removes a saved printer located at |index|.
+ * Removes a saved printer located at |index| and then verify that saved
+ * printers list is updated accordingly.
  * @param {!TestCupsPrintersBrowserProxy} cupsPrintersBrowserProxy
- * @param {!HTMLElement} savedPrintersElement
+ * @param {!HTMLElement} savedPrinters
+ * @param {!HTMLElement} entryList
+ * @param {!Array<!CupsPrinterInfo>} printerList
  * @param {number} index
  * @return {!Promise}
  */
-function removePrinter(cupsPrintersBrowserProxy, savedPrintersElement, index) {
-  let printerList = cupsPrintersBrowserProxy.printerList.printerList;
-  let savedPrinterEntries = getPrinterEntries(savedPrintersElement);
-
-  clickThreeDotMenu(savedPrinterEntries[index]);
-  savedPrintersElement.$$('#removeButton').click();
+function removeAndVerifyPrinters(
+    cupsPrintersBrowserProxy, savedPrinters, entryList, printerList, index) {
+  clickThreeDotMenu(entryList[index]);
+  savedPrinters.$$('#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
@@ -255,8 +237,6 @@ 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.
@@ -265,6 +245,9 @@ suite('CupsSavedPrintersTests', function() {
     assertTrue(!!page);
 
     Polymer.dom.flush();
+
+    savedPrintersElement = page.$$('settings-cups-saved-printers');
+    assertTrue(!!savedPrintersElement);
   });
 
   teardown(function() {
@@ -277,15 +260,15 @@ suite('CupsSavedPrintersTests', function() {
   });
 
   test('SavedPrintersSuccessfullyPopulates', function() {
-    // Wait for saved printers to populate.
-    return test_util.flushTasks().then(() => {
-      savedPrintersElement = page.$$('settings-cups-saved-printers');
-      assertTrue(!!savedPrintersElement);
-
     // List component contained by CupsSavedPrinters.
     const savedPrintersList =
         savedPrintersElement.$$('settings-cups-printers-entry-list');
 
+    return cupsPrintersBrowserProxy.whenCalled('getCupsPrintersList')
+        .then(function() {
+          // Wait for saved printers to populate.
+          Polymer.dom.flush();
+
           const printerListEntries = getPrinterEntries(savedPrintersElement);
 
           verifyPrintersList(printerListEntries, printerList);
@@ -295,45 +278,31 @@ suite('CupsSavedPrintersTests', function() {
   test('SuccessfullyRemoveMultipleSavedPrinters', function() {
     let savedPrinterEntries = [];
 
+    return cupsPrintersBrowserProxy.whenCalled('getCupsPrintersList')
+        .then(function() {
           // 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);
+          Polymer.dom.flush();
 
-          savedPrintersList =
-              savedPrintersElement.$$('settings-cups-printers-entry-list');
           savedPrinterEntries = getPrinterEntries(savedPrintersElement);
 
-          verifyPrintersList(savedPrinterEntries, printerList);
-
-          assertTrue(!!page.$$('#savedPrinters'));
-
-          return removeAllPrinters(
-              cupsPrintersBrowserProxy, savedPrintersElement);
+          return removeAndVerifyPrinters(
+              cupsPrintersBrowserProxy, savedPrintersElement,
+              savedPrinterEntries, printerList, 1 /** index */);
         })
-        .then(() => {
-          assertFalse(!!page.$$('#savedPrinters'));
+        .then(function() {
+          return removeAndVerifyPrinters(
+              cupsPrintersBrowserProxy, savedPrintersElement,
+              savedPrinterEntries, printerList, 0 /** index */);
+        })
+        .then(function() {
+          return removeAndVerifyPrinters(
+              cupsPrintersBrowserProxy, savedPrintersElement,
+              savedPrinterEntries, printerList, 1 /** index */);
+        })
+        .then(function() {
+          return removeAndVerifyPrinters(
+              cupsPrintersBrowserProxy, savedPrintersElement,
+              savedPrinterEntries, printerList, 0 /** index */);
         });
   });
 
@@ -343,11 +312,10 @@ suite('CupsSavedPrintersTests', function() {
     let editDialog = null;
     let savedPrinterEntries = null;
 
+    return cupsPrintersBrowserProxy.whenCalled('getCupsPrintersList')
+        .then(function() {
           // Wait for saved printers to populate.
-    return test_util.flushTasks()
-        .then(() => {
-          savedPrintersElement = page.$$('settings-cups-saved-printers');
-          assertTrue(!!savedPrintersElement);
+          Polymer.dom.flush();
 
           savedPrinterEntries = getPrinterEntries(savedPrintersElement);
 
@@ -371,7 +339,7 @@ suite('CupsSavedPrintersTests', function() {
 
           return cupsPrintersBrowserProxy.whenCalled('updateCupsPrinter');
         })
-        .then(() => {
+        .then(function() {
           assertEquals(expectedName, editDialog.activePrinter.printerName);
 
           // Mimic changes to |cupsPrintersBrowserProxy.printerList|.
@@ -388,11 +356,10 @@ suite('CupsSavedPrintersTests', function() {
     let savedPrinterEntries = null;
     let editDialog = null;
 
+    return cupsPrintersBrowserProxy.whenCalled('getCupsPrintersList')
+        .then(function() {
           // Wait for saved printers to populate.
-    return test_util.flushTasks()
-        .then(() => {
-          savedPrintersElement = page.$$('settings-cups-saved-printers');
-          assertTrue(!!savedPrintersElement);
+          Polymer.dom.flush();
 
           savedPrinterEntries = getPrinterEntries(savedPrintersElement);
 
@@ -420,7 +387,7 @@ suite('CupsSavedPrintersTests', function() {
 
           return cupsPrintersBrowserProxy.whenCalled('reconfigureCupsPrinter');
         })
-        .then(() => {
+        .then(function() {
           assertEquals(expectedName, editDialog.activePrinter.printerName);
           assertEquals(
               expectedAddress, editDialog.activePrinter.printerAddress);
@@ -491,8 +458,6 @@ 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.