Typecheck some of ui/webui/resources/js/ with Closure compiler.

Also adds:

  assertNotReached("message");

as a rough equivalent of

  NOTREACHED() << "message";

in Chromium and:

  ASSERT_NOT_REACHED();

in blink.

R=arv@chromium.org
BUG=393873
TEST=gyp --depth . ui/webui/resources/js/compiled_resources.gyp ui/webui/resources/js/chromeos/compiled_resources.gyp && ninja -C out/Default/

Review URL: https://codereview.chromium.org/405743002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287403 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/resources/extensions/extension_error_overlay.js

@@ -77,7 +77,7 @@ cr.define('extensions', function() {
     else if (chrome.developerPrivate)
       chrome.developerPrivate.openDevTools(args);
     else
-      assert(false, 'Cannot call either openDevTools function.');
+      assertNotReached('Cannot call either openDevTools function.');
   };
 
   RuntimeErrorContent.prototype = {
@@ -335,7 +335,7 @@ cr.define('extensions', function() {
         extensions.ExtensionErrorOverlay.requestFileSourceResponse(result);
       });
     } else {
-      assert(false, 'Cannot call either requestFileSource function.');
+      assertNotReached('Cannot call either requestFileSource function.');
     }
   };
 

chrome/browser/resources/ntp4/tile_page.js

@@ -1372,7 +1372,7 @@ cr.define('ntp', function() {
      * @param {number} index The tile index at which the drop occurred.
      */
     addDragData: function(dataTransfer, index) {
-      assert(false);
+      assertNotReached();
     },
 
     /**
@@ -1390,7 +1390,7 @@ cr.define('ntp', function() {
      * @param {Object} dataTransfer The drag event dataTransfer object.
      */
     setDropEffect: function(dataTransfer) {
-      assert(false);
+      assertNotReached();
     },
   };
 

chrome/browser/resources/options/browser_options.js

@@ -1372,8 +1372,7 @@ cr.define('options', function() {
           return profile;
       }
 
-      assert(false,
-             'There should always be a current profile, but none found.');
+      assertNotReached('There should always be a current profile.');
     },
 
     /**

chrome/test/data/webui/link_controller_test.html

@@ -24,11 +24,10 @@ var URL2 = '#hash';
 var BASE = 'http://www.google.com/';
 
 function setUp() {
-
   mockController = new MockController();
   mockController.createFunctionMock(chrome.tabs, 'create');
   mockController.createFunctionMock(chrome.windows, 'create');
-  oldIsMac = cr.isMac;
+  oldIsMac = Object.getOwnPropertyDescriptor(cr, 'isMac');
 
   mockWindow = {
     confirm: mockController.createFunctionMock(),
@@ -47,7 +46,7 @@ function setUp() {
 function tearDown() {
   mockController.verifyMocks();
   mockController.reset();
-  cr.isMac = oldIsMac;
+  Object.defineProperty(cr, 'isMac', oldIsMac);
 }
 
 function testGetWarningMessage() {
@@ -59,7 +58,7 @@ function testGetWarningMessage() {
 function openUrlFromEventHelper(event, isMac, expectedKind) {
   var lc = new cr.LinkController(localStrings);
 
-  cr.isMac = isMac;
+  Object.defineProperty(cr, 'isMac', {get: function() { return isMac }});
 
   var mock = lc.openUrls = mockController.createFunctionMock();
   mock.addExpectation([URL1], expectedKind);

third_party/closure_compiler/checker.py

@@ -94,6 +94,8 @@ class Checker(object):
     "--jscomp_error=unknownDefines",
     "--jscomp_error=uselessCode",
     "--jscomp_error=visibility",
+    # TODO(dbeam): happens when the same file is <include>d multiple times.
+    "--jscomp_off=duplicate",
   ]
 
   _found_java = False

third_party/closure_compiler/externs/chrome_send_externs.js

+// Copyright 2014 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.
+
+/**
+ * @fileoverview Externs for global |chrome| object.
+ * @externs
+ */
+
+/**
+ * @param {string} msg
+ * @param {Array=} opt_args
+ */
+chrome.send = function(msg, opt_args) {};

third_party/closure_compiler/externs/pending_compiler_externs.js

+// Copyright 2014 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.
+
+/**
+ * @fileoverview Temporary externs until compiler/compiler.jar catches up.
+ * @externs
+ */
+
+/**
+ * @see http://dom.spec.whatwg.org/#dom-domimplementation-createhtmldocument
+ * @param {string=} opt_title The title to give the new HTML document.
+ * @return {!HTMLDocument}
+ */
+DOMImplementation.prototype.createHTMLDocument = function(opt_title) {};

ui/webui/resources/js/assert.js

@@ -22,3 +22,28 @@ function assert(condition, opt_message) {
     throw new Error(msg);
   }
 }
+
+/**
+ * Call this from places in the code that should never be reached.
+ *
+ * For example, handling all the values of enum with a switch() like this:
+ *
+ *   function getValueFromEnum(enum) {
+ *     switch (enum) {
+ *       case ENUM_FIRST_OF_TWO:
+ *         return first
+ *       case ENUM_LAST_OF_TWO:
+ *         return last;
+ *     }
+ *     assertNotReached();
+ *     return document;
+ *   }
+ *
+ * This code should only be hit in the case of serious programmer error or
+ * unexpected input.
+ *
+ * @param {string=} opt_message A message to show when this is hit.
+ */
+function assertNotReached(opt_message) {
+  throw new Error(opt_message || "Unreachable code hit");
+}

ui/webui/resources/js/chromeos/ui_account_tweaks.js

@@ -109,7 +109,7 @@ cr.define('uiAccountTweaks', function() {
    * disables interactive elements (input/select/button), and removes href
    * attribute from <a> elements.
    *
-   * @param {Element} element Root element of DOM subtree that should be
+   * @param {!Element} element Root element of DOM subtree that should be
    *     disabled.
    * @param {string} sessionType session type specificator.
    */
@@ -125,7 +125,8 @@ cr.define('uiAccountTweaks', function() {
 
     var node = walker.nextNode();
     while (node) {
-      UIAccountTweaks.disableElementForSessionType_(node, sessionType);
+      UIAccountTweaks.disableElementForSessionType_(
+          /** @type {!Element} */(node), sessionType);
       node = walker.nextNode();
     }
   };
@@ -137,7 +138,7 @@ cr.define('uiAccountTweaks', function() {
    * <a> element.
    *
    * @private
-   * @param {Element} element Element that should be disabled.
+   * @param {!Element} element Element that should be disabled.
    * @param {string} sessionType account session Type specificator.
    */
   UIAccountTweaks.disableElementForSessionType_ = function(element,
@@ -145,9 +146,9 @@ cr.define('uiAccountTweaks', function() {
     element.classList.add(sessionType + '-disabled');
     if (element.nodeName == 'INPUT' ||
         element.nodeName == 'SELECT' ||
-        element.nodeName == 'BUTTON')
+        element.nodeName == 'BUTTON') {
       element.disabled = true;
-    if (element.nodeName == 'A') {
+    } else if (element.nodeName == 'A') {
       element.onclick = function() {
         return false;
       };

ui/webui/resources/js/cr.js

@@ -10,7 +10,7 @@
 var global = this;
 
 /** Platform, package, object property, and Event support. **/
-this.cr = (function() {
+var cr = function() {
   'use strict';
 
   /**
@@ -68,7 +68,7 @@ this.cr = (function() {
 
   /**
    * The kind of property to define in {@code defineProperty}.
-   * @enum {number}
+   * @enum {string}
    * @const
    */
   var PropertyKind = {
@@ -94,7 +94,7 @@ this.cr = (function() {
    * Helper function for defineProperty that returns the getter to use for the
    * property.
    * @param {string} name The name of the property.
-   * @param {cr.PropertyKind} kind The kind of the property.
+   * @param {PropertyKind} kind The kind of the property.
    * @return {function():*} The getter for the property.
    */
   function getGetter(name, kind) {
@@ -115,6 +115,10 @@ this.cr = (function() {
           return this.hasAttribute(attributeName);
         };
     }
+
+    // TODO(dbeam): replace with assertNotReached() in assert.js when I can coax
+    // the browser/unit tests to preprocess this file through grit.
+    throw 'not reached';
   }
 
   /**
@@ -122,10 +126,10 @@ this.cr = (function() {
    * kind.
    * @param {string} name The name of the property we are defining the setter
    *     for.
-   * @param {cr.PropertyKind} kind The kind of property we are getting the
+   * @param {PropertyKind} kind The kind of property we are getting the
    *     setter for.
-   * @param {function(*):void} opt_setHook A function to run after the property
-   *     is set, but before the propertyChange event is fired.
+   * @param {function(*, *):void=} opt_setHook A function to run after the
+   *     property is set, but before the propertyChange event is fired.
    * @return {function(*):void} The function to use as a setter.
    */
   function getSetter(name, kind, opt_setHook) {
@@ -172,6 +176,10 @@ this.cr = (function() {
           }
         };
     }
+
+    // TODO(dbeam): replace with assertNotReached() in assert.js when I can coax
+    // the browser/unit tests to preprocess this file through grit.
+    throw 'not reached';
   }
 
   /**
@@ -179,15 +187,15 @@ this.cr = (function() {
    * property change event with the type {@code name + 'Change'} is fired.
    * @param {!Object} obj The object to define the property for.
    * @param {string} name The name of the property.
-   * @param {cr.PropertyKind=} opt_kind What kind of underlying storage to use.
-   * @param {function(*):void} opt_setHook A function to run after the
+   * @param {PropertyKind=} opt_kind What kind of underlying storage to use.
+   * @param {function(*, *):void=} opt_setHook A function to run after the
    *     property is set, but before the propertyChange event is fired.
    */
   function defineProperty(obj, name, opt_kind, opt_setHook) {
     if (typeof obj == 'function')
       obj = obj.prototype;
 
-    var kind = opt_kind || PropertyKind.JS;
+    var kind = /** @type {PropertyKind} */ (opt_kind || PropertyKind.JS);
 
     if (!obj.__lookupGetter__(name))
       obj.__defineGetter__(name, getGetter(name, kind));
@@ -283,56 +291,6 @@ this.cr = (function() {
     };
   }
 
-  /**
-   * Initialization which must be deferred until run-time.
-   */
-  function initialize() {
-    // If 'document' isn't defined, then we must be being pre-compiled,
-    // so set a trap so that we're initialized on first access at run-time.
-    if (!global.document) {
-      var originalCr = cr;
-
-      Object.defineProperty(global, 'cr', {
-        get: function() {
-          Object.defineProperty(global, 'cr', {value: originalCr});
-          originalCr.initialize();
-          return originalCr;
-        },
-        configurable: true
-      });
-
-      return;
-    }
-
-    cr.doc = document;
-
-    /**
-     * Whether we are using a Mac or not.
-     */
-    cr.isMac = /Mac/.test(navigator.platform);
-
-    /**
-     * Whether this is on the Windows platform or not.
-     */
-    cr.isWindows = /Win/.test(navigator.platform);
-
-    /**
-     * Whether this is on chromeOS or not.
-     */
-    cr.isChromeOS = /CrOS/.test(navigator.userAgent);
-
-    /**
-     * Whether this is on vanilla Linux (not chromeOS).
-     */
-    cr.isLinux = /Linux/.test(navigator.userAgent);
-
-    /**
-     * Whether this uses the views toolkit or not.
-     */
-    cr.isViews = typeof chrome.getVariableValue == 'function' &&
-        /views/.test(chrome.getVariableValue('toolkit'));
-  }
-
   return {
     addSingletonGetter: addSingletonGetter,
     createUid: createUid,
@@ -341,14 +299,36 @@ this.cr = (function() {
     dispatchPropertyChange: dispatchPropertyChange,
     dispatchSimpleEvent: dispatchSimpleEvent,
     getUid: getUid,
-    initialize: initialize,
-    PropertyKind: PropertyKind
+    PropertyKind: PropertyKind,
+
+    get doc() {
+      return document;
+    },
+
+    /** Whether we are using a Mac or not. */
+    get isMac() {
+      return /Mac/.test(navigator.platform);
+    },
+
+    /** Whether this is on the Windows platform or not. */
+    get isWindows() {
+      return /Win/.test(navigator.platform);
+    },
+
+    /** Whether this is on chromeOS or not. */
+    get isChromeOS() {
+      return /CrOS/.test(navigator.userAgent);
+    },
+
+    /** Whether this is on vanilla Linux (not chromeOS). */
+    get isLinux() {
+      return /Linux/.test(navigator.userAgent);
+    },
+
+    /** Whether this uses the views toolkit or not. */
+    get isViews() {
+      return typeof chrome.getVariableValue == 'function' &&
+          /views/.test(chrome.getVariableValue('toolkit'));
+    },
   };
-})();
-
-
-/**
- * TODO(kgr): Move this to another file which is to be loaded last.
- * This will be done as part of future work to make this code pre-compilable.
- */
-cr.initialize();
+}();

ui/webui/resources/js/event_tracker.js

@@ -2,43 +2,46 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-/** @fileoverview EventTracker is a simple class that manages the addition and
- *  removal of DOM event listeners. In particular, it keeps track of all
- *  listeners that have been added and makes it easy to remove some or all of
- *  them without requiring all the information again. This is particularly
- *  handy when the listener is a generated function such as a lambda or the
- *  result of calling Function.bind.
+/**
+ * @fileoverview EventTracker is a simple class that manages the addition and
+ * removal of DOM event listeners. In particular, it keeps track of all
+ * listeners that have been added and makes it easy to remove some or all of
+ * them without requiring all the information again. This is particularly handy
+ * when the listener is a generated function such as a lambda or the result of
+ * calling Function.bind.
  */
 
+/**
+ * The type of the internal tracking entry. TODO(dbeam): move this back to
+ * EventTracker.Entry when https://github.com/google/closure-compiler/issues/544
+ * is fixed.
+ * @typedef {{node: !Node,
+ *            eventType: string,
+ *            listener: Function,
+ *            capture: boolean}}
+ */
+var EventTrackerEntry;
+
 // Use an anonymous function to enable strict mode just for this file (which
-// will be concatenated with other files when embedded in Chrome)
+// will be concatenated with other files when embedded in Chrome).
 var EventTracker = (function() {
   'use strict';
 
   /**
-   *  Create an EventTracker to track a set of events.
-   *  EventTracker instances are typically tied 1:1 with other objects or
-   *  DOM elements whose listeners should be removed when the object is disposed
-   *  or the corresponding elements are removed from the DOM.
-   *  @constructor
+   * Create an EventTracker to track a set of events.
+   * EventTracker instances are typically tied 1:1 with other objects or
+   * DOM elements whose listeners should be removed when the object is disposed
+   * or the corresponding elements are removed from the DOM.
+   * @constructor
    */
   function EventTracker() {
     /**
-     *  @type {Array.<EventTracker.Entry>}
-     *  @private
+     * @type {Array.<EventTrackerEntry>}
+     * @private
      */
     this.listeners_ = [];
   }
 
-  /**
-   * The type of the internal tracking entry.
-   *  @typedef {{node: !Node,
-   *            eventType: string,
-   *            listener: Function,
-   *            capture: boolean}}
-   */
-  EventTracker.Entry;
-
   EventTracker.prototype = {
     /**
      * Add an event listener - replacement for Node.addEventListener.
@@ -83,9 +86,9 @@ var EventTracker = (function() {
   };
 
   /**
-   * Remove a single event listener given it's tracker entry.  It's up to the
+   * Remove a single event listener given it's tracking entry. It's up to the
    * caller to ensure the entry is removed from listeners_.
-   * @param {EventTracker.Entry} h The entry describing the listener to remove.
+   * @param {EventTrackerEntry} h The entry describing the listener to remove.
    * @private
    */
   EventTracker.removeEventListener_ = function(h) {

ui/webui/resources/js/i18n_template.js

@@ -25,12 +25,19 @@
  *     });
  */
 
+/**
+ * @typedef {function(!Element, string, Object)}
+ * TODO(dbeam): move inside (function() {...})() after
+ * https://github.com/google/closure-compiler/issues/544 is fixed.
+ */
+var Handler;
+
 var i18nTemplate = (function() {
   /**
    * This provides the handlers for the templating engine. The key is used as
    * the attribute name and the value is the function that gets called for every
    * single node that has this attribute.
-   * @type {Object}
+   * @type {Object.<Handler>}
    */
   var handlers = {
     /**
@@ -79,9 +86,8 @@ var i18nTemplate = (function() {
                 object[path] = value;
                 // In case we set innerHTML (ignoring others) we need to
                 // recursively check the content
-                if (path == 'innerHTML') {
+                if (path == 'innerHTML')
                   process(element, obj);
-                }
               }
             } else {
               element.setAttribute(propName, value);
@@ -102,6 +108,8 @@ var i18nTemplate = (function() {
 
   /**
    * Processes a DOM tree with the {@code obj} map.
+   * @param {Node} node A node to process.
+   * @param {Object} obj Values to process |node| with.
    */
   function process(node, obj) {
     var elements = node.querySelectorAll(selector);
@@ -109,9 +117,8 @@ var i18nTemplate = (function() {
       for (var j = 0; j < attributeNames.length; j++) {
         var name = attributeNames[j];
         var att = element.getAttribute(name);
-        if (att != null) {
+        if (att != null)
           handlers[name](element, att, obj);
-        }
       }
     }
   }

ui/webui/resources/js/i18n_template_no_process.js

@@ -30,7 +30,7 @@ var i18nTemplate = (function() {
    * This provides the handlers for the templating engine. The key is used as
    * the attribute name and the value is the function that gets called for every
    * single node that has this attribute.
-   * @type {Object}
+   * @type {!Object}
    */
   var handlers = {
     /**
@@ -104,7 +104,7 @@ var i18nTemplate = (function() {
               process(element, dictionary);
           }
         } else {
-          element.setAttribute(propName, value);
+          element.setAttribute(propName, /** @type {string} */(value));
         }
       });
     }

ui/webui/resources/js/load_time_data.js

@@ -12,12 +12,14 @@
 
 var loadTimeData;
 
+// Expose this type globally as a temporary work around until
+// https://github.com/google/closure-compiler/issues/544 is fixed.
+/** @constructor */
+function LoadTimeData() {}
+
 (function() {
   'use strict';
 
-  function LoadTimeData() {
-  }
-
   LoadTimeData.prototype = {
     /**
      * Sets the backing object.
@@ -29,6 +31,7 @@ var loadTimeData;
     },
 
     /**
+     * @param {string} id An ID of a value that might exist.
      * @return {boolean} True if |id| is a key in the dictionary.
      */
     valueExists: function(id) {
@@ -55,20 +58,21 @@ var loadTimeData;
     getString: function(id) {
       var value = this.getValue(id);
       expectIsType(id, value, 'string');
-      return value;
+      return /** @type {string} */ (value);
     },
 
     /**
      * Returns a formatted localized string where $1 to $9 are replaced by the
      * second to the tenth argument.
      * @param {string} id The ID of the string we want.
-     * @param {...string} The extra values to include in the formatted output.
+     * @param {...string} var_args The extra values to include in the formatted
+     *     output.
      * @return {string} The formatted string.
      */
-    getStringF: function(id) {
+    getStringF: function(id, var_args) {
       var value = this.getString(id);
       if (!value)
-        return;
+        return '';
 
       var varArgs = arguments;
       return value.replace(/\$[$1-9]/g, function(m) {
@@ -84,7 +88,7 @@ var loadTimeData;
     getBoolean: function(id) {
       var value = this.getValue(id);
       expectIsType(id, value, 'boolean');
-      return value;
+      return /** @type {boolean} */ (value);
     },
 
     /**
@@ -96,7 +100,7 @@ var loadTimeData;
       var value = this.getValue(id);
       expectIsType(id, value, 'number');
       expect(value == Math.floor(value), 'Number isn\'t integer: ' + value);
-      return value;
+      return /** @type {number} */ (value);
     },
 
     /**

ui/webui/resources/js/local_strings.js

@@ -9,10 +9,11 @@
  * {@code templateData}. This class provides a simpler interface to access those
  * strings.
  *
- * @param {Object} opt_templateData Optional object containing translated
- *     strings.  If this is not supplied during construction, it can be
- *     assigned to the templateData property after construction.  If all else
- *     fails, the value of window.templateDate will be used.
+ * @param {Object=} opt_templateData Object containing translated strings.  If
+ *     this is not supplied during construction, it can be assigned to the
+ *     templateData property after construction.  If all else fails, the value
+ *     of window.templateDate will be used.
+
  * @constructor
  */
 function LocalStrings(opt_templateData) {
@@ -26,7 +27,8 @@ function LocalStrings(opt_templateData) {
  * Returns a formatted string where $1 to $9 are replaced by the second to the
  * tenth argument.
  * @param {string} s The format string.
- * @param {...string} The extra values to include in the formatted output.
+ * @param {Arguments} args The extra values to include in the formatted
+ *     output.
  * @return {string} The string after format substitution.
  */
 function replaceArgs(s, args) {
@@ -49,13 +51,13 @@ function trimAccelerators(s) {
 LocalStrings.prototype = {
   /**
    * The template data object.
-   * @type {Object}
+   * @type {Object|undefined}
    */
-  templateData: null,
+  templateData: undefined,
 
   /**
    * Gets a localized string by its id.
-   * @param {string} s The ID of the string we want.
+   * @param {string} id The ID of the string we want.
    * @return {string} The localized string.
    */
   getString: function(id) {
@@ -72,7 +74,8 @@ LocalStrings.prototype = {
    * Returns a formatted localized string where $1 to $9 are replaced by the
    * second to the tenth argument.
    * @param {string} id The ID of the string we want.
-   * @param {...string} The extra values to include in the formatted output.
+   * @param {...string} var_args The extra values to include in the formatted
+   *     output.
    * @return {string} The formatted string.
    */
   getStringF: function(id, var_args) {

ui/webui/resources/js/parse_html_subset.js

@@ -37,7 +37,8 @@ var parseHtmlSubset = (function() {
    */
   var allowedTags = ['A', 'B', 'STRONG'];
 
-  function merge() {
+  /** @param {...Object} var_args Objects to merge. */
+  function merge(var_args) {
     var clone = {};
     for (var i = 0; i < arguments.length; ++i) {
       if (typeof arguments[i] == 'object') {

ui/webui/resources/js/template_data_externs.js

+// Copyright 2014 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.
+
+/**
+ * @fileoverview Externs for |templateData| global.
+ * @externs
+ */
+
+/** @type {!Object|undefined} */
+var templateData;

ui/webui/resources/js/util.js

@@ -4,13 +4,6 @@
 
 <include src="assert.js">
 
-/**
- * The global object.
- * @type {!Object}
- * @const
- */
-var global = this;
-
 /**
  * Alias for document.getElementById.
  * @param {string} id The ID of the element to find.
@@ -60,7 +53,7 @@ function chromeSend(name, params, callbackName, callback) {
 
 /**
  * Returns the scale factors supported by this platform.
- * @return {array} The supported scale factors.
+ * @return {Array} The supported scale factors.
  */
 function getSupportedScaleFactors() {
   var supportedScaleFactors = [];
@@ -97,8 +90,8 @@ function url(s) {
  * Returns the URL of the image, or an image set of URLs for the profile avatar.
  * Default avatars have resources available for multiple scalefactors, whereas
  * the GAIA profile image only comes in one size.
-
- * @param {string} url The path of the image.
+ *
+ * @param {string} path The path of the image.
  * @return {string} The url, or an image set of URLs of the avatar image.
  */
 function getProfileAvatarIcon(path) {
@@ -140,8 +133,8 @@ function imageset(path) {
 
 /**
  * Parses query parameters from Location.
- * @param {string} location The URL to generate the CSS url for.
- * @return {object} Dictionary containing name value pairs for URL
+ * @param {Location} location The URL to generate the CSS url for.
+ * @return {Object} Dictionary containing name value pairs for URL
  */
 function parseQueryParams(location) {
   var params = {};
@@ -157,7 +150,7 @@ function parseQueryParams(location) {
 /**
  * Creates a new URL by appending or replacing the given query key and value.
  * Not supporting URL with username and password.
- * @param {object} location The original URL.
+ * @param {Location} location The original URL.
  * @param {string} key The query parameter name.
  * @param {string} value The query parameter value.
  * @return {string} The constructed new URL.
@@ -174,18 +167,21 @@ function setQueryParam(location, key, value) {
   return location.origin + location.pathname + newQuery + location.hash;
 }
 
+/**
+ * @param {Node} el An element to search for ancestors with |className|.
+ * @param {string} className A class to search for.
+ * @return {Node} A node with class of |className| or null if none is found.
+ */
 function findAncestorByClass(el, className) {
   return findAncestor(el, function(el) {
-    if (el.classList)
-      return el.classList.contains(className);
-    return null;
+    return el.classList && el.classList.contains(className);
   });
 }
 
 /**
  * Return the first ancestor for which the {@code predicate} returns true.
  * @param {Node} node The node to check.
- * @param {function(Node) : boolean} predicate The function that tests the
+ * @param {function(Node):boolean} predicate The function that tests the
  *     nodes.
  * @return {Node} The found ancestor or null if not found.
  */
@@ -236,7 +232,7 @@ function disableTextSelectAndDrag(opt_allowSelectStart, opt_allowDragStart) {
  */
 function preventDefaultOnPoundLinkClicks() {
   document.addEventListener('click', function(e) {
-    var anchor = findAncestor(e.target, function(el) {
+    var anchor = findAncestor(/** @type {Node} */(e.target), function(el) {
       return el.tagName == 'A';
     });
     // Use getAttribute() to prevent URL normalization.
@@ -263,7 +259,7 @@ function isRTL() {
 function getRequiredElement(id) {
   var element = $(id);
   assert(element, 'Missing required element: ' + id);
-  return element;
+  return /** @type {!Element} */(element);
 }
 
 // Handle click on a link. If the link points to a chrome: or file: url, then