Fix all remaining print preview closure compiler errors

chrome/browser/resources/print_preview/search/destination_list_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.
 
 cr.define('print_preview', function() {
   'use strict';
 
   /**
    * Component that renders a destination item in a destination list.
    * @param {!cr.EventTarget} eventTarget Event target to dispatch selection
@@ skipping 20 matching lines @@
      * @private
      */
     this.destination_ = destination;
 
     /**
      * Active filter query text.
      * @type {RegExp}
      * @private
      */
     this.query_ = query;
-  };
+  }
 
   /**
    * Event types dispatched by the destination list item.
    * @enum {string}
    */
   DestinationListItem.EventType = {
     // Dispatched to check the printer needs to be configured before activation.
     CONFIGURE_REQUEST: 'print_preview.DestinationListItem.CONFIGURE_REQUEST',
     // Dispatched when the list item is activated.
     SELECT: 'print_preview.DestinationListItem.SELECT',
@@ skipping 16 matching lines @@
       print_preview.Component.prototype.enterDocument.call(this);
       this.tracker.add(this.getElement(), 'click', this.onActivate_.bind(this));
       this.tracker.add(
           this.getElement(), 'keydown', this.onKeyDown_.bind(this));
       this.tracker.add(
           this.getChildElement('.register-promo-button'),
           'click',
           this.onRegisterPromoClicked_.bind(this));
     },
 
-    /** @return {!print_preiew.Destination} */
+    /** @return {!print_preview.Destination} */
     get destination() {
       return this.destination_;
     },
 
     /**
      * Updates the list item UI state.
      * @param {!print_preview.Destination} destination Destination data object
      *     to render.
      * @param {RegExp} query Active filter query.
      */
@@ skipping 145 matching lines @@
           span.className = 'destination-list-item-query-highlight';
           span.textContent = sections[i];
           parent.appendChild(span);
         }
       }
     },
 
     /**
      * Shows/Hides the configuring in progress message and starts/stops its
      * animation accordingly.
-     * @param {bool} show If the message and animation should be shown.
+     * @param {boolean} show If the message and animation should be shown.
      * @private
      */
     updateConfiguringMessage_: function(show) {
       setIsVisible(
           this.getChildElement('.configuring-in-progress-text'), show);
       this.getChildElement('.configuring-text-jumping-dots')
           .classList.toggle('jumping-dots', show);
     },
 
     /**
@@ skipping 25 matching lines @@
               print_preview.DestinationConnectionStatus.UNREGISTERED) {
         var selectEvt = new Event(DestinationListItem.EventType.SELECT);
         selectEvt.destination = this.destination_;
         this.eventTarget_.dispatchEvent(selectEvt);
       }
     },
 
     /**
      * Called when the key is pressed on the destination item. Dispatches a
      * SELECT event when Enter is pressed.
-     * @param {KeyboardEvent} e Keyboard event to process.
+     * @param {!KeyboardEvent} e Keyboard event to process.
      * @private
      */
     onKeyDown_: function(e) {
       if (!hasKeyModifiers(e)) {
         if (e.keyCode == 13) {
           var activeElementTag = document.activeElement ?
               document.activeElement.tagName.toUpperCase() : '';
           if (activeElementTag == 'LI') {
             e.stopPropagation();
             e.preventDefault();
@@ skipping 11 matching lines @@
       var promoClickedEvent = new Event(
           DestinationListItem.EventType.REGISTER_PROMO_CLICKED);
       promoClickedEvent.destination = this.destination_;
       this.eventTarget_.dispatchEvent(promoClickedEvent);
     },
 
     /**
      * Handles click and 'Enter' key down events for the extension icon element.
      * It opens extensions page with the extension associated with the
      * destination highlighted.
-     * @param {MouseEvent|KeyboardEvent} e The event to handle.
+     * @param {Event} e The event to handle.
      * @private
      */
     onExtensionIconClicked_: function(e) {
       e.stopPropagation();
       window.open('chrome://extensions?id=' + this.destination_.extensionId);
     },
 
     /**
      * Handles key down event for the extensin icon element. Keys different than
      * 'Enter' are ignored.
-     * @param {KeyboardEvent} e The event to handle.
+     * @param {Event} e The event to handle.
      * @private
      */
     onExtensionIconKeyDown_: function(e) {
-      if (hasKeyModifiers(e))
+      if (e && hasKeyModifiers(e))

[Dan Beam, 2017/05/10 00:00:14]

same nit regarding !Event vs e && (just to type filter out null)

[rbpotter, 2017/05/10 01:56:23]

Does not work here as this gets assigned to extensionIconEl.keydown,
which requires (function ((Event|null)): ?|null).

Changed to an assert instead, let me know if it should be something
different.

[dpapad, 2017/05/10 02:54:12]

Nit(optional): It seems that onkeydown,onclick are erroneously annotated with
Event instead of !Event, see [1]. Perhaps changing the code to use
addEventListener('click', ...) instead would allow you to use @param{!Event}
here and remove the assertion.

[1]
https://github.com/google/closure-compiler/blob/02ee72c0f8248ffe6be9f9efd591f...
[2]
https://github.com/google/closure-compiler/blob/8ac08c03cd695b84f8a79ac3a1338...

[dpapad, 2017/05/10 02:59:20]

Actually you should probably not follow my previous suggestion, because calling
addEventListener() and using onclick, onkeydown are not equivalent here.

addEventListener() adds a new listener every time it is called, whereas onclick
= ..., replaces the existing listener. The enclosing code [1] seems to be
executed multiple times, so you don't want to add multiple listeners every time
is called.

[1]
https://cs.chromium.org/chromium/src/chrome/browser/resources/print_preview/s...

[Dan Beam, 2017/05/10 03:18:35]

this works for me 
http://closure-compiler.appspot.com/home#code%3D%252F%252F%2520%253D%253DClos...

[rbpotter, 2017/05/10 03:42:10]

Done. Should I change it to !KeyboardEvent if we are going to cast the
function anyway? (was KeyboardEvent before this CL)

         return;
       if (e.keyCode != 13 /* Enter */)
         return;
-      this.onExtensionIconClicked_(event);
+      this.onExtensionIconClicked_(e);
     }
   };
 
   // Export
   return {
     DestinationListItem: DestinationListItem
   };
 });