Save most recent 3 destinations across multiple sessions

chrome/browser/resources/print_preview/data/app_state.js

Index: chrome/browser/resources/print_preview/data/app_state.js
diff --git a/chrome/browser/resources/print_preview/data/app_state.js b/chrome/browser/resources/print_preview/data/app_state.js
index b782f4ea3d1543fc1b8edb23ba6fdb842778570c..2c1801d78d4f552e005dceedb8e017256ad554ea 100644
--- a/chrome/browser/resources/print_preview/data/app_state.js
+++ b/chrome/browser/resources/print_preview/data/app_state.js
@@ -29,18 +29,25 @@ cr.define('print_preview', function() {
   };
 
   /**
+    * Number of recent print destinations to store across browser sessions.
+    * @const {number}
+    */
+  AppState.NUM_DESTINATIONS_ = 3;
+
+
+  /**
    * Enumeration of field names for serialized app state.
    * @enum {string}
    */
   AppState.Field = {
     VERSION: 'version',
-    SELECTED_DESTINATION_ID: 'selectedDestinationId',
-    SELECTED_DESTINATION_ACCOUNT: 'selectedDestinationAccount',
-    SELECTED_DESTINATION_ORIGIN: 'selectedDestinationOrigin',
-    SELECTED_DESTINATION_CAPABILITIES: 'selectedDestinationCapabilities',
-    SELECTED_DESTINATION_NAME: 'selectedDestinationName',
-    SELECTED_DESTINATION_EXTENSION_ID: 'selectedDestinationExtensionId',
-    SELECTED_DESTINATION_EXTENSION_NAME: 'selectedDestinationExtensionName',
+    RECENT_DESTINATION_IDS: 'recentDestinationIds',
+    RECENT_DESTINATION_ACCOUNTS: 'recentDestinationAccounts',
+    RECENT_DESTINATION_ORIGINS: 'recentDestinationOrigins',
+    RECENT_DESTINATION_CAPABILITIES: 'recentDestinationCapabilities',
+    RECENT_DESTINATION_NAMES: 'recentDestinationNames',
+    RECENT_DESTINATION_EXTENSION_IDS: 'recentDestinationExtensionIds',
+    RECENT_DESTINATION_EXTENSION_NAMES: 'recentDestinationExtensionNames',
     IS_GCP_PROMO_DISMISSED: 'isGcpPromoDismissed',
     DPI: 'dpi',
     MEDIA_SIZE: 'mediaSize',
@@ -74,14 +81,29 @@ cr.define('print_preview', function() {
   AppState.NATIVE_FUNCTION_NAME_ = 'saveAppState';
 
   AppState.prototype = {
+
+    /**
+      * Helper function to get the most recent value of one of the destination
+      * fields in the app state.
+      * @param {?print_preview.AppState.Field} fieldName The state field to
+      *     return the value of.
+      * @return {?string|?print_preview.Cdd} The most recent value of the
+      *     destination state field. ?print_preview.Cdd if the state field is
+      *     RECENT_DESTINATION_CAPABILIITIES, ?string otherwise.
+      */
+    getStateValue_: function(fieldName) {
+      return (this.state_[fieldName] && this.state_[fieldName].length > 0) ?
+          this.state_[fieldName][0] : null;
+    },
+
     /** @return {?string} ID of the selected destination. */
     get selectedDestinationId() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_ID];
+      return this.getStateValue_(AppState.Field.RECENT_DESTINATION_IDS);
     },
 
     /** @return {?string} Account the selected destination is registered for. */
     get selectedDestinationAccount() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_ACCOUNT];
+      return this.getStateValue_(AppState.Field.RECENT_DESTINATION_ACCOUNTS);
     },
 
     /**
@@ -89,24 +111,26 @@ cr.define('print_preview', function() {
      *     selected destination.
      */
     get selectedDestinationOrigin() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_ORIGIN];
+      return this.getStateValue_(AppState.Field.RECENT_DESTINATION_ORIGINS);
     },
 
     /** @return {?print_preview.Cdd} CDD of the selected destination. */
     get selectedDestinationCapabilities() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_CAPABILITIES];
+      return this.getStateValue_(
+          AppState.Field.RECENT_DESTINATION_CAPABILITIES);
     },
 
     /** @return {?string} Name of the selected destination. */
     get selectedDestinationName() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_NAME];
+      return this.getStateValue_(AppState.Field.RECENT_DESTINATION_NAMES);
     },
 
     /**
      * @return {?string} Extension ID associated with the selected destination.
      */
     get selectedDestinationExtensionId() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_EXTENSION_ID];
+      return this.getStateValue_(
+          AppState.Field.RECENT_DESTINATION_EXTENSION_IDS);
     },
 
     /**
@@ -114,7 +138,52 @@ cr.define('print_preview', function() {
      *     destination.
      */
     get selectedDestinationExtensionName() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_EXTENSION_NAME];
+      return this.getStateValue_(
+          AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES);
+    },
+
+   /** @return {?Array<string>} IDs of the recent destinations. */

[dpapad, 2016/09/21 00:34:30]

Nit: Indentation off by 1.

[rbpotter, 2016/09/21 01:43:07]

Done.

+    get recentDestinationIds() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_IDS];
+    },
+
+    /**
+     * @return {?Array<string>} Accounts the recent destinations are registered
+     *     for.
+     */
+    get recentDestinationAccounts() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS];
+    },
+
+    /** @return {?Array<string>} Origins of the recent destinations. */
+    get recentDestinationOrigins() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS];
+    },
+
+    /** @return {?Array<print_preview.Cdd>} CDDs of the recent destinations. */
+    get recentDestinationCapabilities() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES];
+    },
+
+    /** @return {?Array<string>} Names of the recent destinations. */
+    get recentDestinationNames() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_NAMES];
+    },
+
+    /**
+     * @return {?Array<string>} Extension IDs associated with the recent
+     * destinations.
+     */
+    get recentDestinationExtensionIds() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS];
+    },
+
+    /**
+     * @return {?Array<string>} Extension names associated with the recent
+     *     destinations.
+     */
+    get recentDestinationExtensionNames() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES];
     },
 
     /** @return {boolean} Whether the GCP promotion has been dismissed. */
@@ -145,6 +214,28 @@ cr.define('print_preview', function() {
     },
 
     /**
+     * Helper function to set up the recent destination fields in case they are
+     * empty or only contain one value.
+     * @param {print_preview.AppState.Field} stateFieldName The state field to
+     *     be set up.
+     */
+    setUpState_: function(stateFieldName) {
+      if (!this.state_[AppState.Field.RECENT_DESTINATION_IDS] ||
+          this.state_[AppState.Field.RECENT_DESTINATION_IDS].length == 0) {

[dpapad, 2016/09/21 00:34:30]

Should these comparisons use stateFieldName instead of RECENT_DESTINATION_IDS?
It is not clear if this is intentional.

[rbpotter, 2016/09/21 01:43:07]

This is intentional. The other states can have empty strings or, in the case of
capabilities, nullptr as values. If there is only one destination and it happens
to have one of these values this could lead to the check succeeding incorrectly.
Ids is a required value, so the check will only succeed if there are really no
destinations stored.

+        // Ids must be a nonempty string or an Array instance. If it is null or
+        // an empty array, no destinations were saved.
+        this.state_[stateFieldName] = [];
+      } else if (!(this.state_[stateFieldName] instanceof Array)) {
+        var tmp = this.state_[stateFieldName];
+        this.state_[stateFieldName] = [tmp];
+      } else if (this.state_[stateFieldName].length >
+          AppState.NUM_DESTINATIONS_) {
+        this.state_[stateFieldName].splice(AppState.NUM_DESTINATIONS_,

[dpapad, 2016/09/21 00:34:30]

Could you simply truncate the array by setting the length, as follows?

this.state_[stateFieldName].length = AppState.NUM_DESTINATIONS_;

This is a valid way to truncate an array (see
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...).

[rbpotter, 2016/09/21 01:43:07]

Done.

+            this.state_[stateFieldName].length - AppState.NUM_DESTINATIONS_);
+      }
+    },
+
+    /**
      * Initializes the app state from a serialized string returned by the native
      * layer.
      * @param {?string} serializedAppStateStr Serialized string representation
@@ -164,7 +255,16 @@ cr.define('print_preview', function() {
       } else {
         // Set some state defaults.
         this.state_[AppState.Field.IS_GCP_PROMO_DISMISSED] = false;
+        this.state_[AppState.Field.RECENT_DESTINATION_IDS] = [];
       }
+
+      this.setUpState_(AppState.Field.RECENT_DESTINATION_IDS);
+      this.setUpState_(AppState.Field.RECENT_DESTINATION_ACCOUNTS);
+      this.setUpState_(AppState.Field.RECENT_DESTINATION_ORIGINS);
+      this.setUpState_(AppState.Field.RECENT_DESTINATION_CAPABILITIES);
+      this.setUpState_(AppState.Field.RECENT_DESTINATION_NAMES);
+      this.setUpState_(AppState.Field.RECENT_DESTINATION_EXTENSION_IDS);
+      this.setUpState_(AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES);
     },
 
     /**
@@ -191,22 +291,68 @@ cr.define('print_preview', function() {
     },
 
     /**
+     * Shifts the desired recent destination field's values as needed and places
+     * the most recent value, destVal, in position 0 in the array.
+     * @param {number} indexFound the index where the destination already exists
+     *     in the array, or -1 if it is not in the array.
+     * @param {!print_preview.AppState.Field} fieldName the field array to
+     *     adjust
+     * @param {string or print_preview.Cdd} the value that should be added as

[dpapad, 2016/09/21 00:34:30]

{string|printPreview.Cdd} destVal The value that ...

[rbpotter, 2016/09/21 01:43:07]

Done.

+     *     the most recent value in the array.
+     */
+    shiftStateField_: function(indexFound, fieldName, destVal) {
+      if (indexFound == -1 &&
+          this.state_[fieldName].length == AppState.NUM_DESTINATIONS_)
+        indexFound = AppState.NUM_DESTINATIONS_ - 1;
+      if (indexFound != -1)
+        this.state_[fieldName].splice(indexFound, 1);
+      this.state_[fieldName].splice(0, 0, destVal);
+    },
+
+    /**
      * Persists the selected destination.
      * @param {!print_preview.Destination} dest Destination to persist.
      */
     persistSelectedDestination: function(dest) {
-      if (!this.isInitialized_)
+      if (!this.isInitialized_ || !dest)

[dpapad, 2016/09/21 00:34:30]

Can dest be null/undefined? If so line 314 should depict that by specifying
{?print_preview.Destination}

[rbpotter, 2016/09/21 01:43:07]

Done - No, it shouldn't be. This was accidentally left over from testing. 

         return;
-      this.state_[AppState.Field.SELECTED_DESTINATION_ID] = dest.id;
-      this.state_[AppState.Field.SELECTED_DESTINATION_ACCOUNT] = dest.account;
-      this.state_[AppState.Field.SELECTED_DESTINATION_ORIGIN] = dest.origin;
-      this.state_[AppState.Field.SELECTED_DESTINATION_CAPABILITIES] =
-          dest.capabilities;
-      this.state_[AppState.Field.SELECTED_DESTINATION_NAME] = dest.displayName;
-      this.state_[AppState.Field.SELECTED_DESTINATION_EXTENSION_ID] =
-          dest.extensionId;
-      this.state_[AppState.Field.SELECTED_DESTINATION_EXTENSION_NAME] =
-          dest.extensionName;
+
+      // Determine if this destination is already in the recent destinations,
+      // and where in the array it is located.
+      var idIndexFound = this.state_[
+          AppState.Field.RECENT_DESTINATION_IDS].indexOf(dest.id);
+      var originIndexFound = this.state_[
+          AppState.Field.RECENT_DESTINATION_ORIGINS].indexOf(dest.origin);
+
+      if (idIndexFound != originIndexFound ||
+          idIndexFound >= AppState.NUM_DESTINATIONS_)
+        idIndexFound = -1;
+
+      if (idIndexFound == 0) {
+        this.persist_();
+        return;
+      }
+
+      // Shift all the destination state fields so that the are always ordered
+      // from most recent (entry 0) to least recent.
+      this.shiftStateField_(idIndexFound,
+          AppState.Field.RECENT_DESTINATION_IDS, dest.id);
+      this.shiftStateField_(idIndexFound,
+          AppState.Field.RECENT_DESTINATION_ACCOUNTS, dest.account || '');
+      this.shiftStateField_(idIndexFound,
+          AppState.Field.RECENT_DESTINATION_ORIGINS, dest.origin);
+      this.shiftStateField_(idIndexFound,
+          AppState.Field.RECENT_DESTINATION_CAPABILITIES,
+          dest.capabilities);
+      this.shiftStateField_(idIndexFound,
+          AppState.Field.RECENT_DESTINATION_NAMES, dest.name || '');
+      this.shiftStateField_(idIndexFound,
+          AppState.Field.RECENT_DESTINATION_EXTENSION_IDS,
+          dest.extension_id || '');
+      this.shiftStateField_(idIndexFound,
+          AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES,
+          dest.extension_name || '');
+
       this.persist_();
     },