Save most recent 3 destinations across multiple sessions

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 cr.define('print_preview', function() {
   'use strict';
 
   /**
    * Object used to get and persist the print preview application state.
    * @constructor
    */
   function AppState() {
     /**
      * Internal representation of application state.
      * @type {Object}
      * @private
      */
     this.state_ = {};
     this.state_[AppState.Field.VERSION] = AppState.VERSION_;
     this.state_[AppState.Field.IS_GCP_PROMO_DISMISSED] = true;
-
-    /**
+    this.numDestinations_ = 3;

[dpapad, 2016/09/17 01:53:02]

Let's add type annotation to this.

@private {number}

Also, if this is meant to be a constant, you could move it a few lines below and
change it as follows:

/**
 * Explanatory comment.
 * @const {number}
 */
AppState.NUM_DESTINATIONS = 3;

[rbpotter, 2016/09/19 21:16:19]

Done.

+     /**
      * Whether the app state has been initialized. The app state will ignore all
      * writes until it has been initialized.
      * @type {boolean}
      * @private
      */
     this.isInitialized_ = false;
   };
 
   /**
    * 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',
     MARGINS_TYPE: 'marginsType',
     CUSTOM_MARGINS: 'customMargins',
     IS_COLOR_ENABLED: 'isColorEnabled',
     IS_DUPLEX_ENABLED: 'isDuplexEnabled',
     IS_HEADER_FOOTER_ENABLED: 'isHeaderFooterEnabled',
     IS_LANDSCAPE_ENABLED: 'isLandscapeEnabled',
     IS_COLLATE_ENABLED: 'isCollateEnabled',
@@ skipping 13 matching lines @@
 
   /**
    * Name of C++ layer function to persist app state.
    * @type {string}
    * @const
    * @private
    */
   AppState.NATIVE_FUNCTION_NAME_ = 'saveAppState';
 
   AppState.prototype = {
     /** @return {?string} ID of the selected destination. */

[dpapad, 2016/09/17 01:53:02]

The type annotation here seems no longer accurate. It seems that this function
now always returns a string, never null (same below).

[rbpotter, 2016/09/19 21:16:19]

Done.

     get selectedDestinationId() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_ID];
+      if (this.state_[AppState.Field.RECENT_DESTINATION_IDS] &&
+          this.state_[AppState.Field.RECENT_DESTINATION_IDS].length > 0)
+        return this.state_[AppState.Field.RECENT_DESTINATION_IDS][0];
+      else
+        return '';
     },
 
     /** @return {?string} Account the selected destination is registered for. */
     get selectedDestinationAccount() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_ACCOUNT];
+      if (this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS] &&

[dpapad, 2016/09/17 01:53:02]

This logic is repeated 7 times. Can you package it in a helper function?
Something like

getStateValue_: function(name) {
  return this.state_[name] && this.state_[name].length > 0 ?
this.state_[name][0] : '';
},

get selectedDestinationAccount() {
  return this.getStateValue_(AppState.Field.RECENT_DESTINATION_ACCOUNTS);
}

...

[rbpotter, 2016/09/19 21:16:18]

Done.

+          this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS].length > 0)
+        return this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS][0];
+      else
+        return '';
     },
 
     /**
      * @return {?print_preview.Destination.Origin<string>} Origin of the
      *     selected destination.
      */
     get selectedDestinationOrigin() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_ORIGIN];
+      if (this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS] &&
+          this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS].length > 0)
+        return this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS][0];
+      else
+        return '';
     },
 
     /** @return {?print_preview.Cdd} CDD of the selected destination. */
     get selectedDestinationCapabilities() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_CAPABILITIES];
+      if (this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES] &&
+          this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES].length >
+          0)
+        return this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES][0];
+      else
+        return null;
     },
 
     /** @return {?string} Name of the selected destination. */
     get selectedDestinationName() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_NAME];
+      if (this.state_[AppState.Field.RECENT_DESTINATION_NAMES] &&
+          this.state_[AppState.Field.RECENT_DESTINATION_NAMES].length > 0)
+        return this.state_[AppState.Field.RECENT_DESTINATION_NAMES][0];
+      else
+        return '';
     },
 
     /**
      * @return {?string} Extension ID associated with the selected destination.
      */
     get selectedDestinationExtensionId() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_EXTENSION_ID];
+      if (this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS] &&
+          this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS].length >
+          0)
+        return this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS][0];
+      else
+        return '';
     },
 
     /**
      * @return {?string} Extension name associated with the selected
      *     destination.
      */
     get selectedDestinationExtensionName() {
-      return this.state_[AppState.Field.SELECTED_DESTINATION_EXTENSION_NAME];
+      if (this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES] &&
+          this.state_[
+          AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES].length > 0)
+        return this.state_[
+            AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES][0];
+      else
+        return '';
+    },
+
+   /** @return {?array of strings} IDs of the recent destinations. */

[dpapad, 2016/09/17 01:53:02]

@return {?Array<string>} IDs of the recent destinations.
(here and elsewhere)

[rbpotter, 2016/09/19 21:16:19]

Done.

+    get recentDestinationIds() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_IDS];
+    },
+
+    /**
+     * @return {?array of strings} Accounts the recent destinations are
+     *     registered for.
+     */
+    get recentDestinationAccounts() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS];
+    },
+
+    /**
+     * @return {?array of print_preview.Destination.Origin<string>} Origins of
+     *     the recent destinations.
+     */
+    get recentDestinationOrigins() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS];
+    },
+
+    /**
+     * @return {?array of print_preview.Cdds} CDDs of the recent
+     * destinations.
+     */
+    get recentDestinationCapabilities() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES];
+    },
+
+    /** @return {?array of strings} Names of the recent destinations. */
+    get recentDestinationNames() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_NAMES];
+    },
+
+    /**
+     * @return {?array of strings} Extension IDs associated with the recent
+     * destinations.
+     */
+    get recentDestinationExtensionIds() {
+      return this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS];
+    },
+
+    /**
+     * @return {?array of strings} 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. */
     get isGcpPromoDismissed() {
       return this.state_[AppState.Field.IS_GCP_PROMO_DISMISSED];
     },
 
     /**
      * @param {!print_preview.AppState.Field} field App state field to check if
      *     set.
@@ skipping 30 matching lines @@
             this.state_ = state;
           }
         } catch(e) {
           console.error('Unable to parse state: ' + e);
           // Proceed with default state.
         }
       } else {
         // Set some state defaults.
         this.state_[AppState.Field.IS_GCP_PROMO_DISMISSED] = false;
       }
+
+      // Deal with empty or single element cases.
+      if (!this.state_[AppState.Field.RECENT_DESTINATION_IDS])

[dpapad, 2016/09/17 01:53:02]

Is there a way to package redundant logic into a helper?

[rbpotter, 2016/09/19 21:16:18]

Done.

+        this.state_[AppState.Field.RECENT_DESTINATION_IDS] = [];
+      else if (this.state_[
+          AppState.Field.RECENT_DESTINATION_IDS].constructor != Array) {

[dpapad, 2016/09/17 01:53:02]

instanceof is more suited for this type of checks.

var a = [];
if (a instanceof Array) {...}

[rbpotter, 2016/09/19 21:16:19]

Done.

+        var tmp = this.state_[AppState.Field.RECENT_DESTINATION_IDS];
+        this.state_[AppState.Field.RECENT_DESTINATION_IDS][0] = tmp;
+      }
+
+      if (!this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS])
+        this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS] = [];
+      else if (this.state_[
+          AppState.Field.RECENT_DESTINATION_ACCOUNTS].constructor != Array) {
+        var tmp = this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS];
+        this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS][0] = tmp;
+      }
+
+      if (!this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS])
+        this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS] = [];
+      else if (this.state_[
+          AppState.Field.RECENT_DESTINATION_ORIGINS].constructor != Array) {
+        var tmp = this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS];
+        this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS][0] = tmp;
+      }
+
+      if (!this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES])
+        this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES] = [];
+      else if (this.state_[
+          AppState.Field.RECENT_DESTINATION_CAPABILITIES].constructor !=
+          Array) {
+        var tmp = this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES];
+        this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES][0] = tmp;
+      }
+
+      if (!this.state_[AppState.Field.RECENT_DESTINATION_NAMES])
+        this.state_[AppState.Field.RECENT_DESTINATION_NAMES] = [];
+      else if (this.state_[
+          AppState.Field.RECENT_DESTINATION_NAMES].constructor != Array) {
+        var tmp = this.state_[AppState.Field.RECENT_DESTINATION_NAMES];
+        this.state_[AppState.Field.RECENT_DESTINATION_NAMES][0] = tmp;
+      }
+
+      if (!this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS])
+        this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS] = [];
+      else if (this.state_[
+          AppState.Field.RECENT_DESTINATION_EXTENSION_IDS].constructor !=
+          Array) {
+        var tmp = this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS];
+        this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS][0] = tmp;
+      }
+
+      if (!this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES])
+        this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES] = [];
+      else if (this.state_[
+          AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES].constructor !=
+          Array) {
+        var tmp = this.state_[
+            AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES];
+        this.state_[
+            AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES][0] = tmp;
+      }
+
+
     },
 
     /**
      * Sets to initialized state. Now object will accept persist requests.
      */
     setInitialized: function() {
       this.isInitialized_ = true;
     },
 
     /**
@@ skipping 12 matching lines @@
       this.persist_();
     },
 
     /**
      * Persists the selected destination.
      * @param {!print_preview.Destination} dest Destination to persist.
      */
     persistSelectedDestination: function(dest) {
       if (!this.isInitialized_)
         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;
+
+      for (var i = this.numDestinations_ - 1; i > 0; i--) {

[dpapad, 2016/09/17 01:53:02]

Can you explain a bit what this logic does? Perhaps there is an easier way to do
it using the Array#shift or splice() methods?

[rbpotter, 2016/09/19 21:16:19]

Acknowledged - see new code in the next patchset. Not sure if it is
simpler/easier or not, let me know what you think.

+        var shiftDestination = true;
+        for (var j = i - 1; j >= 0; j--) {
+          if (!this.state_[AppState.Field.RECENT_DESTINATION_IDS] ||
+              dest.id == this.state_[AppState.Field.RECENT_DESTINATION_IDS][j])
+            shiftDestination = false;
+        }
+        if (shiftDestination) {
+          this.state_[AppState.Field.RECENT_DESTINATION_IDS][i] =
+              this.state_[AppState.Field.RECENT_DESTINATION_IDS][i - 1];
+          this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS][i] =
+              this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS][i - 1];
+          this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS][i] =
+              this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS][i - 1];
+          this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES][i] =
+              this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES][
+              i - 1];
+          this.state_[AppState.Field.RECENT_DESTINATION_NAMES][i] =
+              this.state_[AppState.Field.RECENT_DESTINATION_NAMES][i - 1];
+          this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS][i] =
+              this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS][
+              i - 1];
+          this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES][i] =
+              this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES][
+              i - 1];
+        }
+      }
+      this.state_[AppState.Field.RECENT_DESTINATION_IDS][0] = dest.id;
+      this.state_[AppState.Field.RECENT_DESTINATION_ACCOUNTS][0] =
+          dest.account || '';
+      this.state_[AppState.Field.RECENT_DESTINATION_ORIGINS][0] =
+          dest.origin;
+      this.state_[AppState.Field.RECENT_DESTINATION_CAPABILITIES][0] =
+          dest.capabilities || '';
+      this.state_[AppState.Field.RECENT_DESTINATION_NAMES][0] =
+          dest.name || '';
+      this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_IDS][0] =
+          dest.extensionId || '';
+      this.state_[AppState.Field.RECENT_DESTINATION_EXTENSION_NAMES][0] =
+          dest.extensionName || '';
       this.persist_();
     },
 
     /**
      * Persists whether the GCP promotion has been dismissed.
      * @param {boolean} isGcpPromoDismissed Whether the GCP promotion has been
      *     dismissed.
      */
     persistIsGcpPromoDismissed: function(isGcpPromoDismissed) {
       if (!this.isInitialized_)
         return;
       this.state_[AppState.Field.IS_GCP_PROMO_DISMISSED] = isGcpPromoDismissed;
       this.persist_();
     },
 
     /**
      * Calls into the native layer to persist the application state.
      * @private
      */
     persist_: function() {
       chrome.send(AppState.NATIVE_FUNCTION_NAME_,
                   [JSON.stringify(this.state_)]);
     }
   };
 
   return {
     AppState: AppState
   };
 });