Print Preview: Fix data/ errors

chrome/browser/resources/print_preview/data/ticket_items/ticket_item.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.
 /**
  * @typedef {Object|number|boolean|string}
  */
 print_preview.ValueType;
 
 cr.define('print_preview.ticket_items', function() {
   'use strict';
 
   /**
    * An object that represents a user modifiable item in a print ticket. Each
    * ticket item has a value which can be set by the user. Ticket items can also
    * be unavailable for modifying if the print destination doesn't support it or
    * if other ticket item constraints are not met.
    * @param {?print_preview.AppState} appState Application state model to update
    *     when ticket items update.
-   * @param {?print_preview.AppState.Field} field Field of the app state to
+   * @param {?print_preview.AppStateField} field Field of the app state to
    *     update when ticket item is updated.
    * @param {?print_preview.DestinationStore} destinationStore Used listen for
    *     changes in the currently selected destination's capabilities. Since
    *     this is a common dependency of ticket items, it's handled in the base
    *     class.
    * @param {?print_preview.DocumentInfo=} opt_documentInfo Used to listen for
    *     changes in the document. Since this is a common dependency of ticket
    *     items, it's handled in the base class.
    * @constructor
    * @extends {cr.EventTarget}
    */
   function TicketItem(appState, field, destinationStore, opt_documentInfo) {
     cr.EventTarget.call(this);
 
     /**
      * Application state model to update when ticket items update.
      * @type {print_preview.AppState}
      * @private
      */
     this.appState_ = appState || null;
 
     /**
      * Field of the app state to update when ticket item is updated.
-     * @type {?print_preview.AppState.Field}
+     * @type {?print_preview.AppStateField}
      * @private
      */
     this.field_ = field || null;
 
     /**
      * Used listen for changes in the currently selected destination's
      * capabilities.
      * @type {print_preview.DestinationStore}
      * @private
      */
@@ skipping 14 matching lines @@
     this.value_ = null;
 
     /**
      * Keeps track of event listeners for the ticket item.
      * @type {!EventTracker}
      * @private
      */
     this.tracker_ = new EventTracker();
 
     this.addEventHandlers_();
-  };
+  }
 
   /**
    * Event types dispatched by this class.
    * @enum {string}
    */
   TicketItem.EventType = {
     CHANGE: 'print_preview.ticket_items.TicketItem.CHANGE'
   };
 
   TicketItem.prototype = {
@@ skipping 51 matching lines @@
     },
 
     /**
      * @param {?} value Value to set as the value of the ticket item.
      */
     updateValue: function(value) {
       // Use comparison with capabilities for event.
       var sendUpdateEvent = !this.isValueEqual(value);
       // Don't lose requested value if capability is not available.
       this.updateValueInternal(value);
-      if (this.appState_) {
+      if (this.appState_ && this.field_) {

[dpapad, 2017/05/05 17:32:53]

Is this 100% equivalent? What if |this.field_| was legitimately the value of 0,
or the empty string ''? Are those not valid values for |this.field_|?

[rbpotter, 2017/05/05 18:25:46]

Done.

         this.appState_.persistField(this.field_, value);
       }
       if (sendUpdateEvent)
         cr.dispatchSimpleEvent(this, TicketItem.EventType.CHANGE);
     },
 
     /**
      * @return {?} Default value of the ticket item if no value was set by
      *     the user.
      * @protected

[dpapad, 2017/05/05 17:32:52]

Maybe add @abstract here and below (not necessarily in this CL)? See
https://github.com/google/closure-compiler/wiki/%40abstract-classes-and-methods.

[rbpotter, 2017/05/05 18:25:46]

Done.

      */
     getDefaultValueInternal: function() {
       throw Error('Abstract method not overridden');
     },
 
     /**
      * @return {?} Default value of the ticket item if the capability is
      *     not available.
      * @protected
      */
@@ skipping 65 matching lines @@
             this.dispatchChangeEventInternal.bind(this));
       }
     },
   };
 
   // Export
   return {
     TicketItem: TicketItem
   };
 });