Print Preview: Increase persisted destinations

Increase the number of persisted destinations to 10 on Chrome OS (for
use with PluginVM) and 5 on other platforms.

Only display/prefetch at most 3 non-PDF/drive destinations.

Bug: 1015662
Test: xvfb-run ./browser_tests --gtest_filter=PrintPreviewDestinationSettingsTest.*
Test: autoninja webui_closure_compile

Change-Id: Id310f4d10504e92c333844672c268f30db3a3476
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2344286Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Commit-Queue: Pranav Batra <batrapranav@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808220}

chrome/browser/resources/print_preview/print_preview.js

@@ -30,7 +30,7 @@ export {Error, State} from './data/state.js';
 export {BackgroundGraphicsModeRestriction, CapabilitiesResponse, LocalDestinationInfo, NativeInitialSettings, NativeLayer, NativeLayerImpl, PrinterSetupResponse, ProvisionalDestinationInfo} from './native_layer.js';
 export {getSelectDropdownBackground} from './print_preview_utils.js';
 export {DEFAULT_MAX_COPIES} from './ui/copies_settings.js';
-export {DestinationState} from './ui/destination_settings.js';
+export {DestinationState, NUM_PERSISTED_DESTINATIONS} from './ui/destination_settings.js';
 export {PDFPlugin, PluginProxy, PluginProxyImpl} from './ui/plugin_proxy.js';
 export {PreviewAreaState} from './ui/preview_area.js';
 export {SelectBehavior} from './ui/select_behavior.js';

chrome/browser/resources/print_preview/ui/destination_settings.js

@@ -45,7 +45,16 @@ export const DestinationState = {
 };
 
 /** @type {number} Number of recent destinations to save. */
-const NUM_PERSISTED_DESTINATIONS = 3;
+export let NUM_PERSISTED_DESTINATIONS = 5;
+// <if expr="chromeos">
+NUM_PERSISTED_DESTINATIONS = 10;
+// </if>
+
+/**
+ * @type {number} Number of unpinned recent destinations to display.
+ * Pinned destinations include "Save as PDF" and "Save to Google Drive".
+ */
+const NUM_UNPINNED_DESTINATIONS = 3;
 
 Polymer({
   is: 'print-preview-destination-settings',
@@ -227,14 +236,22 @@ Polymer({
     this.destinationStore_.startLoadCookieDestination(
         Destination.GooglePromotedId.DOCS);
     this.updateDriveDestination_();
-    const recentDestinations = this.getSettingValue('recentDestinations');
-    recentDestinations.forEach(destination => {
+    const recentDestinations = /** @type {!Array<!RecentDestination>} */ (
+        this.getSettingValue('recentDestinations'));
+    let numDestinationsChecked = 0;
+    for (const destination of recentDestinations) {
+      if (!this.destinationIsDriveOrPdf_(destination)) {
+        numDestinationsChecked++;
+      }
       if (destination.origin === DestinationOrigin.COOKIES &&
           (destination.account === this.activeUser_ ||
            destination.account === '')) {
         this.destinationStore_.startLoadCookieDestination(destination.id);
       }
-    });
+      if (numDestinationsChecked === NUM_UNPINNED_DESTINATIONS) {
+        break;
+      }
+    }
 
     // Re-filter the dropdown destinations for the new account.
     if (!this.isDialogOpen_) {
@@ -303,11 +320,40 @@ Polymer({
     }
     this.pdfPrinterDisabled_ = pdfPrinterDisabled;
     this.$.userManager.initUserAccounts(userAccounts, syncAvailable);
+    let recentDestinations =
+        /** @type {!Array<!RecentDestination>} */ (
+            this.getSettingValue('recentDestinations'));
+    recentDestinations = recentDestinations.slice(
+        0, this.getRecentDestinationsDisplayCount_(recentDestinations));
     this.destinationStore_.init(
         this.pdfPrinterDisabled_, defaultPrinter,
-        serializedDefaultDestinationRulesStr,
-        /** @type {!Array<RecentDestination>} */
-        (this.getSettingValue('recentDestinations')));
+        serializedDefaultDestinationRulesStr, recentDestinations);
+  },
+
+  /**
+   * @param {!Array<!RecentDestination>} recentDestinations recent destinations.
+   * @return {number} Number of recent destinations to display.
+   * @private
+   */
+  getRecentDestinationsDisplayCount_(recentDestinations) {
+    let numDestinationsToDisplay = NUM_UNPINNED_DESTINATIONS;
+    for (let i = 0; i < recentDestinations.length; i++) {
+      // Once all NUM_UNPINNED_DESTINATIONS unpinned destinations have been
+      // found plus an extra unpinned destination, return the total number of
+      // destinations found excluding the last extra unpinned destination.
+      //
+      // The extra unpinned destination ensures that pinned destinations
+      // located directly after the last unpinned destination are included
+      // in the display count.
+      if (i > numDestinationsToDisplay) {
+        return numDestinationsToDisplay;
+      }
+      // If a destination is pinned, increment numDestinationsToDisplay.
+      if (this.destinationIsDriveOrPdf_(recentDestinations[i])) {
+        numDestinationsToDisplay++;
+      }
+    }
+    return Math.min(recentDestinations.length, numDestinationsToDisplay);
   },
 
   /** @private */
@@ -401,16 +447,31 @@ Polymer({
     }
 
     // Determine if this destination is already in the recent destinations,
-    // and where in the array it is located.
+    // where in the array it is located, and whether or not it is visible.
     const newDestination = makeRecentDestination(assert(this.destination));
     const recentDestinations =
         /** @type {!Array<!RecentDestination>} */ (
             this.getSettingValue('recentDestinations'));
-    let indexFound = recentDestinations.findIndex(function(recent) {
-      return (
-          newDestination.id === recent.id &&
-          newDestination.origin === recent.origin);
-    });
+    let indexFound = -1;
+    // Note: isVisible should be only be used if the destination is unpinned.
+    // Although pinned destinations are always visible, isVisible may not
+    // necessarily be set to true in this case.
+    let isVisible = false;
+    let numUnpinnedChecked = 0;
+    for (let index = 0; index < recentDestinations.length; index++) {
+      const recent = recentDestinations[index];
+      if (recent.id === newDestination.id &&
+          recent.origin === newDestination.origin) {
+        indexFound = index;
+        // If we haven't seen the maximum unpinned destinations already, this
+        // destination is visible in the dropdown.
+        isVisible = numUnpinnedChecked < NUM_UNPINNED_DESTINATIONS;
+        break;
+      }
+      if (!this.destinationIsDriveOrPdf_(recent)) {
+        numUnpinnedChecked++;
+      }
+    }
 
     // No change
     if (indexFound === 0 &&
@@ -424,13 +485,18 @@ Polymer({
     if (isNew && recentDestinations.length === NUM_PERSISTED_DESTINATIONS) {
       indexFound = NUM_PERSISTED_DESTINATIONS - 1;
     }
+
     if (indexFound !== -1) {
       this.setSettingSplice('recentDestinations', indexFound, 1, null);
     }
 
     // Add the most recent destination
     this.setSettingSplice('recentDestinations', 0, 0, newDestination);
-    if (!this.destinationIsDriveOrPdf_(newDestination) && isNew) {
+
+    // The dropdown needs to be updated if a new printer or one not currently
+    // visible in the dropdown has been added.
+    if (!this.destinationIsDriveOrPdf_(newDestination) &&
+        (isNew || !isVisible)) {
       this.updateDropdownDestinations_();
     }
   },
@@ -439,16 +505,23 @@ Polymer({
   updateDropdownDestinations_() {
     const recentDestinations = /** @type {!Array<!RecentDestination>} */ (
         this.getSettingValue('recentDestinations'));
-
     const updatedDestinations = [];
-    recentDestinations.forEach(recent => {
+    let numDestinationsChecked = 0;
+    for (const recent of recentDestinations) {
+      if (this.destinationIsDriveOrPdf_(recent)) {
+        continue;
+      }
+      numDestinationsChecked++;
       const key = createRecentDestinationKey(recent);
       const destination = this.destinationStore_.getDestinationByKey(key);
-      if (destination && !this.destinationIsDriveOrPdf_(recent) &&
+      if (destination &&
           (!destination.account || destination.account === this.activeUser_)) {
         updatedDestinations.push(destination);
       }
-    });
+      if (numDestinationsChecked === NUM_UNPINNED_DESTINATIONS) {
+        break;
+      }
+    }
 
     this.displayedDestinations_ = updatedDestinations;
     this.updateDriveDestination_();

chrome/test/data/webui/print_preview/destination_settings_test.js

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import {CloudPrintInterface, CloudPrintInterfaceEventType, CloudPrintInterfaceImpl, Destination, DestinationConnectionStatus, DestinationErrorType, DestinationOrigin, DestinationState, DestinationStore, DestinationType, Error, LocalDestinationInfo, makeRecentDestination, NativeLayer, NativeLayerImpl, RecentDestination, State} from 'chrome://print/print_preview.js';
+import {CloudPrintInterface, CloudPrintInterfaceEventType, CloudPrintInterfaceImpl, Destination, DestinationConnectionStatus, DestinationErrorType, DestinationOrigin, DestinationState, DestinationStore, DestinationType, Error, LocalDestinationInfo, makeRecentDestination, NativeLayer, NativeLayerImpl, NUM_PERSISTED_DESTINATIONS, RecentDestination, State} from 'chrome://print/print_preview.js';
 import {assert} from 'chrome://resources/js/assert.m.js';
 import {isChromeOS, webUIListenerCallback} from 'chrome://resources/js/cr.m.js';
 import {flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
@@ -57,6 +57,9 @@ suite(destination_settings_test.suiteName, function() {
   /** @type {!Array<!Destination>} */
   let destinations = [];
 
+  /** @type {!Array<!Destination>} */
+  const extraDestinations = [];
+
   /** @type {!Array<string>} */
   let initialAccounts = [];
 
@@ -80,6 +83,15 @@ suite(destination_settings_test.suiteName, function() {
     NativeLayerImpl.instance_ = nativeLayer;
     localDestinations = [];
     destinations = getDestinations(localDestinations);
+    // Add some extra destinations.
+    for (let i = 0; i < NUM_PERSISTED_DESTINATIONS; i++) {
+      const id = `e${i}`;
+      const name = `n${i}`;
+      localDestinations.push({deviceName: id, printerName: name});
+      extraDestinations.push(new Destination(
+          id, DestinationType.LOCAL, getLocalOrigin(), name,
+          DestinationConnectionStatus.ONLINE));
+    }
     nativeLayer.setLocalDestinations(localDestinations);
     cloudPrintInterface = new CloudPrintInterfaceStub();
     CloudPrintInterfaceImpl.instance_ = cloudPrintInterface;
@@ -256,11 +268,11 @@ suite(destination_settings_test.suiteName, function() {
       });
 
   // Tests that the dropdown contains the appropriate destinations when there
-  // are 3 recent destinations.
+  // are 5 recent destinations.
   test(
       assert(destination_settings_test.TestNames.RecentDestinations),
       function() {
-        recentDestinations = destinations.slice(0, 3).map(
+        recentDestinations = destinations.slice(0, 5).map(
             destination => makeRecentDestination(destination));
 
         const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(3);
@@ -303,7 +315,7 @@ suite(destination_settings_test.suiteName, function() {
   test(
       assert(destination_settings_test.TestNames.RecentDestinationsMissing),
       function() {
-        recentDestinations = destinations.slice(0, 3).map(
+        recentDestinations = destinations.slice(0, 5).map(
             destination => makeRecentDestination(destination));
         const missing = localDestinations.splice(1, 1)[0];
         nativeLayer.setLocalDestinations(localDestinations);
@@ -350,11 +362,11 @@ suite(destination_settings_test.suiteName, function() {
   // Tests that the dropdown contains the appropriate destinations when Save
   // as PDF is one of the recent destinations.
   test(assert(destination_settings_test.TestNames.SaveAsPdfRecent), function() {
-    recentDestinations = destinations.slice(0, 3).map(
+    recentDestinations = destinations.slice(0, 5).map(
         destination => makeRecentDestination(destination));
     recentDestinations.splice(
         1, 1, makeRecentDestination(getSaveAsPdfDestination()));
-    const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(2);
+    const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(3);
     initialize();
 
     return whenCapabilitiesDone
@@ -369,6 +381,7 @@ suite(destination_settings_test.suiteName, function() {
           assertDropdownItems([
             makeLocalDestinationKey('ID1'),
             makeLocalDestinationKey('ID3'),
+            makeLocalDestinationKey('ID4'),
             'Save as PDF/local/',
           ]);
 
@@ -379,6 +392,7 @@ suite(destination_settings_test.suiteName, function() {
           assertDropdownItems([
             makeLocalDestinationKey('ID1'),
             makeLocalDestinationKey('ID3'),
+            makeLocalDestinationKey('ID4'),
             'Save as PDF/local/',
             '__google__docs/cookies/foo@chromium.org',
           ]);
@@ -390,12 +404,12 @@ suite(destination_settings_test.suiteName, function() {
   test(
       assert(destination_settings_test.TestNames.GoogleDriveRecent),
       function() {
-        recentDestinations = destinations.slice(0, 3).map(
+        recentDestinations = destinations.slice(0, 5).map(
             destination => makeRecentDestination(destination));
         recentDestinations.splice(
             1, 1,
             makeRecentDestination(getGoogleDriveDestination(defaultUser)));
-        const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(2);
+        const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(3);
         initialize();
 
         return whenCapabilitiesDone
@@ -414,6 +428,7 @@ suite(destination_settings_test.suiteName, function() {
               assertDropdownItems([
                 makeLocalDestinationKey('ID1'),
                 makeLocalDestinationKey('ID3'),
+                makeLocalDestinationKey('ID4'),
                 'Save as PDF/local/',
               ]);
 
@@ -424,6 +439,7 @@ suite(destination_settings_test.suiteName, function() {
               assertDropdownItems([
                 makeLocalDestinationKey('ID1'),
                 makeLocalDestinationKey('ID3'),
+                makeLocalDestinationKey('ID4'),
                 'Save as PDF/local/',
                 '__google__docs/cookies/foo@chromium.org',
               ]);
@@ -436,7 +452,7 @@ suite(destination_settings_test.suiteName, function() {
   test(
       assert(destination_settings_test.TestNames.GoogleDriveAutoselect),
       function() {
-        recentDestinations = destinations.slice(0, 3).map(
+        recentDestinations = destinations.slice(0, 5).map(
             destination => makeRecentDestination(destination));
         recentDestinations.splice(
             0, 1,
@@ -463,6 +479,7 @@ suite(destination_settings_test.suiteName, function() {
               assertDropdownItems([
                 makeLocalDestinationKey('ID2'),
                 makeLocalDestinationKey('ID3'),
+                makeLocalDestinationKey('ID4'),
                 'Save as PDF/local/',
                 '__google__docs/cookies/foo@chromium.org',
               ]);
@@ -473,11 +490,11 @@ suite(destination_settings_test.suiteName, function() {
   // DESTINATION_SELECT event firing, with Save as PDF set as the current
   // destination.
   test(assert(destination_settings_test.TestNames.SelectSaveAsPdf), function() {
-    recentDestinations = destinations.slice(0, 3).map(
+    recentDestinations = destinations.slice(0, 5).map(
         destination => makeRecentDestination(destination));
     recentDestinations.splice(
         1, 1, makeRecentDestination(getSaveAsPdfDestination()));
-    const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(2);
+    const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(3);
     initialize();
 
     const dropdown = destinationSettings.$$('#destinationSelect');
@@ -494,6 +511,7 @@ suite(destination_settings_test.suiteName, function() {
           assertDropdownItems([
             makeLocalDestinationKey('ID1'),
             makeLocalDestinationKey('ID3'),
+            makeLocalDestinationKey('ID4'),
             'Save as PDF/local/',
           ]);
           // Most recent destination is selected by default.
@@ -521,12 +539,12 @@ suite(destination_settings_test.suiteName, function() {
   test(
       assert(destination_settings_test.TestNames.SelectGoogleDrive),
       function() {
-        recentDestinations = destinations.slice(0, 3).map(
+        recentDestinations = destinations.slice(0, 5).map(
             destination => makeRecentDestination(destination));
         recentDestinations.splice(
             1, 1,
             makeRecentDestination(getGoogleDriveDestination(defaultUser)));
-        const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(2);
+        const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(3);
         initialize();
         const dropdown = destinationSettings.$$('#destinationSelect');
 
@@ -541,6 +559,7 @@ suite(destination_settings_test.suiteName, function() {
               assertDropdownItems([
                 makeLocalDestinationKey('ID1'),
                 makeLocalDestinationKey('ID3'),
+                makeLocalDestinationKey('ID4'),
                 'Save as PDF/local/',
               ]);
               assertFalse(dropdown.disabled);
@@ -552,6 +571,7 @@ suite(destination_settings_test.suiteName, function() {
               assertDropdownItems([
                 makeLocalDestinationKey('ID1'),
                 makeLocalDestinationKey('ID3'),
+                makeLocalDestinationKey('ID4'),
                 'Save as PDF/local/',
                 '__google__docs/cookies/foo@chromium.org',
               ]);
@@ -581,7 +601,7 @@ suite(destination_settings_test.suiteName, function() {
   test(
       assert(destination_settings_test.TestNames.SelectRecentDestination),
       function() {
-        recentDestinations = destinations.slice(0, 3).map(
+        recentDestinations = destinations.slice(0, 5).map(
             destination => makeRecentDestination(destination));
         const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(3);
         initialize();
@@ -618,7 +638,7 @@ suite(destination_settings_test.suiteName, function() {
 
   // Tests that selecting the 'see more' option opens the dialog.
   test(assert(destination_settings_test.TestNames.OpenDialog), function() {
-    recentDestinations = destinations.slice(0, 3).map(
+    recentDestinations = destinations.slice(0, 5).map(
         destination => makeRecentDestination(destination));
     const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(3);
     initialize();
@@ -791,7 +811,7 @@ suite(destination_settings_test.suiteName, function() {
               assertEquals(
                   0, nativeLayer.getCallCount('getPrinterCapabilities'));
 
-              // Select a third destination
+              // Select a third destination.
               selectDestination(destinations[1]);
               return nativeLayer.whenCalled('getPrinterCapabilities');
             })
@@ -799,16 +819,24 @@ suite(destination_settings_test.suiteName, function() {
               assertRecentDestinations(['ID2', 'Save as PDF', 'ID1']);
               assertEquals(
                   1, nativeLayer.getCallCount('getPrinterCapabilities'));
-
-              // Select a fourth destination. List does not grow.
               nativeLayer.resetResolver('getPrinterCapabilities');
-              selectDestination(destinations[2]);
-              return nativeLayer.whenCalled('getPrinterCapabilities');
+              // Fill recent destinations up to the cap, then add a couple
+              // more destinations. Make sure the length of the list does not
+              // exceed NUM_PERSISTED_DESTINATIONS.
+              const whenCapabilitiesDone =
+                  nativeLayer.waitForMultipleCapabilities(
+                      NUM_PERSISTED_DESTINATIONS);
+              for (const destination of extraDestinations) {
+                selectDestination(destination);
+              }
+              return whenCapabilitiesDone;
             })
             .then(() => {
-              assertRecentDestinations(['ID3', 'ID2', 'Save as PDF']);
+              assertRecentDestinations(
+                  extraDestinations.map(dest => dest.id).reverse());
               assertEquals(
-                  1, nativeLayer.getCallCount('getPrinterCapabilities'));
+                  NUM_PERSISTED_DESTINATIONS,
+                  nativeLayer.getCallCount('getPrinterCapabilities'));
             });
       });
 
@@ -817,9 +845,9 @@ suite(destination_settings_test.suiteName, function() {
   test(
       assert(destination_settings_test.TestNames.ResetDestinationOnSignOut),
       function() {
-        recentDestinations = destinations.slice(0, 3).map(
+        recentDestinations = destinations.slice(0, 5).map(
             destination => makeRecentDestination(destination));
-        const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(2);
+        const whenCapabilitiesDone = nativeLayer.waitForMultipleCapabilities(3);
         const driveDestination = getGoogleDriveDestination(defaultUser);
         recentDestinations.splice(
             0, 1, makeRecentDestination(driveDestination));
@@ -840,6 +868,7 @@ suite(destination_settings_test.suiteName, function() {
               assertDropdownItems([
                 makeLocalDestinationKey('ID2'),
                 makeLocalDestinationKey('ID3'),
+                makeLocalDestinationKey('ID4'),
                 'Save as PDF/local/',
                 '__google__docs/cookies/foo@chromium.org',
               ]);
@@ -854,6 +883,7 @@ suite(destination_settings_test.suiteName, function() {
               assertDropdownItems([
                 makeLocalDestinationKey('ID2'),
                 makeLocalDestinationKey('ID3'),
+                makeLocalDestinationKey('ID4'),
                 'Save as PDF/local/',
               ]);