First cut of JS closure compiler use for VR HTML UI.

chrome/browser/resources/vr_shell/vr_shell_ui_api.js

Index: chrome/browser/resources/vr_shell/vr_shell_ui_api.js
diff --git a/chrome/browser/resources/vr_shell/vr_shell_ui_api.js b/chrome/browser/resources/vr_shell/vr_shell_ui_api.js
index 8f64ed4b0d7fdc07e95808fb2849cac0851b8367..41900563654573ccf5899df5a33691826556e861 100644
--- a/chrome/browser/resources/vr_shell/vr_shell_ui_api.js
+++ b/chrome/browser/resources/vr_shell/vr_shell_ui_api.js
@@ -2,317 +2,345 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-var api = (function() {
-  'use strict';
-
-  /**
-   * Enumeration of scene update commands.
-   * @enum {number}
-   * @const
-   */
-  var Command = Object.freeze({
+var api = new Object();
+
+/**
+ * Enumeration of scene update commands.
+ * @enum {number}
+ * @const
+ */
+api.Command = {
     'ADD_ELEMENT': 0,
     'UPDATE_ELEMENT': 1,
     'REMOVE_ELEMENT': 2,
     'ADD_ANIMATION': 3,
     'REMOVE_ANIMATION': 4
-  });
+};
+
+/**
+ * Sends one or more commands to native scene management. Commands are used
+ * to add, modify or remove elements and animations. For examples of how to
+ * format command parameters, refer to examples in scene.js.
+ * @param {Array} commands

[amp, 2016/12/21 20:58:58]

Doesn't close support specifying the type of the objects in the array as well?
Something like {!Array<TYPE>}.

Are these commands just strings?

[Dan Beam, 2016/12/22 00:18:21]

yes, you can templatize types in closure, so Array<T>, Object<T>, Promise<T>,
etc.

[cjgrant, 2016/12/22 18:15:46]

Cool.  Yep, there's a lot more we can do - this review was basically to show a
proof of concept.  Dan, do you have an opinion on the removal of the "var api =
(function() {" approach?  This appeared to confuse the compiler, and we lost
type checking across files.

+ */
+api.sendCommands = function(commands) {
+  chrome.send('updateScene', commands);
+};
+
+/**
+ * Enumeration of valid Anchroing for X axis.
+ * An element can either be anchored to the left, right, or center of the main
+ * content rect (or it can be absolutely positioned using NONE). Any
+ * translations applied will be relative to this anchoring.
+ * @enum {number}
+ * @const
+ */
+api.XAnchoring = {
+  'XNONE': 0,
+  'XLEFT': 1,
+  'XRIGHT': 2
+};
+
+/**
+ * Enumeration of valid Anchroing for Y axis.
+ * @enum {number}
+ * @const
+ */
+api.YAnchoring = {
+  'YNONE': 0,
+  'YTOP': 1,
+  'YBOTTOM': 2
+};
+
+/**
+ * Enumeration of actions that can be triggered by the HTML UI.
+ * @enum {number}
+ * @const
+ */
+api.Action = {
+  'HISTORY_BACK': 0,
+  'HISTORY_FORWARD': 1,
+  'RELOAD': 2,
+  'ZOOM_OUT': 3,
+  'ZOOM_IN': 4,
+  'RELOAD_UI': 5,
+};
+
+/**
+ * Enumeration of modes that can be specified by the native side.
+ * @enum {number}
+ * @const
+ */
+api.Mode = {
+  'UNKNOWN': -1,
+  'STANDARD': 0,
+  'WEB_VR': 1,
+};
+
+/**
+ * Triggers an Action.
+ * @param {api.Action} action
+ */
+function doAction(action) {
+  chrome.send('doAction', [action]);
+}
+
+/**
+ * Notify native scene management that DOM loading has completed, at the
+ * specified page size.
+ */
+function domLoaded() {
+  chrome.send('domLoaded');
+}
+
+/**
+ * Sets the CSS size for this page.
+ * @param {number} width
+ * @param {number} height
+ * @param {number} dpr
+ */
+function setUiCssSize(width, height, dpr) {
+  chrome.send('setUiCssSize', [width, height, dpr]);
+}
+
+/**
+ * Represents updates to UI element properties. Any properties set on this
+ * object are relayed to an underlying native element via scene command.
+ * Properties that are not set on this object are left unchanged.
+ * @export
+ */
+api.UiElementUpdate = class {
+
+  constructor() {
+
+    this.properties = {};

[amp, 2016/12/21 20:58:58]

It looks like properties could be a well defined object.  Maybe use a @typedef
here to specify what it's properites are (like 'contentQuad', 'parentId', etc)?

Or if things compile fine now I suppose that could be left to a future change.

[cjgrant, 2016/12/22 18:15:46]

As of now, native parses a dictionary, and updates only properties that are in
the dictionary.  Most of the time, we're updating only one of several properties
(like changing the visibility flag).  We could make this cleaner by defining an
enum of keys, but since this module's job is hiding details of the native API, I
don't think an enum buys us much.

+
+    /**
+     * Handlers that are listening to this logger.
+     * @private {number}
+     */
+    this.varvar = 1;

[amp, 2016/12/21 20:58:58]

I can't remember if this was specifically restricted in Chromium javascript, but
aren't private members supposed to end in '_'?

Also, this varvar, and the comments seem to be copied from elsewhere...

[cjgrant, 2016/12/22 18:15:46]

Yeah, this is test code.

+  }
 
   /**
-   * Sends one or more commands to native scene management. Commands are used
-   * to add, modify or remove elements and animations. For examples of how to
-   * format command parameters, refer to examples in scene.js.
-   */
-  function sendCommands(commands) {
-    chrome.send('updateScene', commands);
+  * Operates on an instance of MyClass and returns something.

[amp, 2016/12/21 20:58:58]

Bad copy paste?

+  */
+  setIsContentQuad() {
+    this.properties['contentQuad'] = true;
   }
 
   /**
-   * Returns the element ID of the browser content quad, allowing the content to
-   * be manimulated in the scene.
+   * Specify a parent for this element. If set, this element is positioned
+   * relative to its parent element, rather than absolutely. This allows
+   * elements to automatically move with a parent.
+   * @param {number} id
    */
-  function getContentElementId() {
-    return 0;
+  setParentId(id) {
+    this.properties['parentId'] = id;
   }
 
   /**
-   * Enumeration of valid Anchroing for X axis.
-   * An element can either be anchored to the left, right, or center of the main
-   * content rect (or it can be absolutely positioned using NONE). Any
-   * translations applied will be relative to this anchoring.
-   * @enum {number}
-   * @const
+   * Specify the width and height (in meters) of an element.
+   * @param {number} x
+   * @param {number} y
    */
-  var XAnchoring = Object.freeze({
-    'XNONE': 0,
-    'XLEFT': 1,
-    'XRIGHT': 2
-  });
+  setSize(x, y) {
+    this.properties['size'] = { x: x, y: y };
+  }
 
   /**
-   * Enumeration of valid Anchroing for Y axis.
-   * @enum {number}
-   * @const
+   * Specify optional scaling of the element, and any children.
+   * @param {number} x
+   * @param {number} y
+   * @param {number} z
    */
-  var YAnchoring = Object.freeze({
-    'YNONE': 0,
-    'YTOP': 1,
-    'YBOTTOM': 2
-  });
+  setScale(x, y, z) {
+    this.properties['scale'] = { x: x, y: y, z: z };
+  }
 
   /**
-   * Enumeration of actions that can be triggered by the HTML UI.
-   * @enum {number}
-   * @const
+   * Specify rotation for the element. The rotation is specified in axis-angle
+   * representation (rotate around unit vector [x, y, z] by 'a' radians).
+   * @param {number} x
+   * @param {number} y
+   * @param {number} z
+   * @param {number} a
    */
-  var Action = Object.freeze({
-    'HISTORY_BACK': 0,
-    'HISTORY_FORWARD': 1,
-    'RELOAD': 2,
-    'ZOOM_OUT': 3,
-    'ZOOM_IN': 4,
-    'RELOAD_UI': 5,
-  });
+  setRotation(x, y, z, a) {
+    this.properties['rotation'] = { x: x, y: y, z: z, a: a };
+
+    //this.setTranslation(1, 2, 'cat');

[amp, 2016/12/21 20:58:58]

Remove comment.

+  }
 
   /**
-   * Enumeration of modes that can be specified by the native side.
-   * @enum {number}
-   * @const
+   * Specify the translation of the element. If anchoring is specified, the
+   * offset is applied to the anchoring position rather than the origin.
+   * Translation is applied after scaling and rotation.
+   * @param {number} x
+   * @param {number} y
+   * @param {number} z
    */
-  var Mode = Object.freeze({
-    'UNKNOWN': -1,
-    'STANDARD': 0,
-    'WEB_VR': 1,
-  });
+
+  // TODO - also test private member enforcement.
+
+  setTranslation(x, y, z) {
+    this.properties['translation'] = { x: x, y: y, z: z };
+  }
 
   /**
-   * Triggers an Action.
+   * Anchoring allows a rectangle to be positioned relative to the edge of
+   * its parent, without being concerned about the size of the parent.
+   * Values should be XAnchoring and YAnchoring elements.
+   * Example: element.setAnchoring(XAnchoring.XNONE, YAnchoring.YBOTTOM);
+   * @param {number} x
+   * @param {number} y
    */
-  function doAction(action) {
-    chrome.send('doAction', [action]);
+  setAnchoring(x, y) {
+    this.properties['xAnchoring'] = x;
+    this.properties['yAnchoring'] = y;
   }
 
   /**
-   * Notify native scene management that DOM loading has completed, at the
-   * specified page size.
+   * Visibility controls whether the element is rendered.
+   * @param {boolean} visible
    */
-  function domLoaded() {
-    chrome.send('domLoaded');
+  setVisible(visible) {
+    this.properties['visible'] = !!visible;
   }
 
   /**
-   * Sets the CSS size for this page.
+   * Hit-testable implies that the reticle will hit the element, if visible.
+   * @param {boolean} testable
    */
-  function setUiCssSize(width, height, dpr) {
-    chrome.send('setUiCssSize', [width, height, dpr]);
+  setHitTestable(testable) {
+    this.properties['hitTestable'] = !!testable;
   }
 
   /**
-   * Represents updates to UI element properties. Any properties set on this
-   * object are relayed to an underlying native element via scene command.
-   * Properties that are not set on this object are left unchanged.
+   * Causes an element to be rendered relative to the field of view, rather
+   * than the scene.  Elements locked in this way should not have a parent.
+   * @param {boolean} locked
    */
-  class UiElementUpdate {
-
-    setIsContentQuad() {
-      this.contentQuad = true;
-    }
-
-    /**
-     * Specify a parent for this element. If set, this element is positioned
-     * relative to its parent element, rather than absolutely. This allows
-     * elements to automatically move with a parent.
-     */
-    setParentId(id) {
-      this.parentId = id;
-    }
-
-    /**
-     * Specify the width and height (in meters) of an element.
-     */
-    setSize(x, y) {
-      this.size = { x: x, y: y };
-    }
-
-    /**
-     * Specify optional scaling of the element, and any children.
-     */
-    setScale(x, y, z) {
-      this.scale = { x: x, y: y, z: z };
-    }
-
-    /**
-     * Specify rotation for the element. The rotation is specified in axis-angle
-     * representation (rotate around unit vector [x, y, z] by 'a' radians).
-     */
-    setRotation(x, y, z, a) {
-      this.rotation = { x: x, y: y, z: z, a: a };
-    }
-
-    /**
-     * Specify the translation of the element. If anchoring is specified, the
-     * offset is applied to the anchoring position rather than the origin.
-     * Translation is applied after scaling and rotation.
-     */
-    setTranslation(x, y, z) {
-      this.translation = { x: x, y: y, z: z };
-    }
-
-    /**
-     * Anchoring allows a rectangle to be positioned relative to the edge of
-     * its parent, without being concerned about the size of the parent.
-     * Values should be XAnchoring and YAnchoring elements.
-     * Example: element.setAnchoring(XAnchoring.XNONE, YAnchoring.YBOTTOM);
-     */
-    setAnchoring(x, y) {
-      this.xAnchoring = x;
-      this.yAnchoring = y;
-    }
-
-    /**
-     * Visibility controls whether the element is rendered.
-     */
-    setVisible(visible) {
-      this.visible = !!visible;
-    }
-
-    /**
-     * Hit-testable implies that the reticle will hit the element, if visible.
-     */
-    setHitTestable(testable) {
-      this.hitTestable = !!testable;
-    }
-
-    /**
-     * Causes an element to be rendered relative to the field of view, rather
-     * than the scene.  Elements locked in this way should not have a parent.
-     */
-    setLockToFieldOfView(locked) {
-      this.lockToFov = !!locked;
-    }
-  };
+  setLockToFieldOfView(locked) {
+    this.properties['lockToFov'] = !!locked;
+  }
+};
 
+/**
+ * Represents a new UI element. This object builds on UiElementUpdate,
+ * forcing the underlying texture coordinates to be specified.
+ */
+api.UiElement = class extends api.UiElementUpdate {
   /**
-   * Represents a new UI element. This object builds on UiElementUpdate,
-   * forcing the underlying texture coordinates to be specified.
+   * Constructor of UiElement.
+   * pixelX and pixelY values indicate the left upper corner; pixelWidth and
+   * pixelHeight is width and height of the texture to be copied from the web
+   * contents.
    */
-  class UiElement extends UiElementUpdate {
-    /**
-     * Constructor of UiElement.
-     * pixelX and pixelY values indicate the left upper corner; pixelWidth and
-     * pixelHeight is width and height of the texture to be copied from the web
-     * contents.
-     */
-    constructor(pixelX, pixelY, pixelWidth, pixelHeight) {
-      super();
-
-      this.copyRect = {
-          x: pixelX,
-          y: pixelY,
-          width: pixelWidth,
-          height: pixelHeight
-      };
-    }
-  };
+  constructor(pixelX, pixelY, pixelWidth, pixelHeight) {
+    super();
+
+    this.copyRect = {
+        x: pixelX,
+        y: pixelY,
+        width: pixelWidth,
+        height: pixelHeight
+    };
+  }
+};
+
+/**
+ * Enumeration of animatable properties.
+ * @enum {number}
+ * @const
+ */
+api.Property = {
+  'COPYRECT': 0,
+  'SIZE': 1,
+  'TRANSLATION': 2,
+  'SCALE': 3,
+  'ROTATION': 4
+};
+
+/**
+ * Enumeration of easing type.
+ * @enum {number}
+ * @const
+ */
+api.Easing = {
+  'LINEAR': 0,
+  'CUBICBEZIER': 1,
+  'EASEIN': 2,
+  'EASEOUT': 3
+};
+
+/**
+ * Base animation class. An animation can vary only one object property.
+ */
+api.Animation = class {
+  constructor(elementId, durationMs) {
+    this.meshId = elementId;
+    this.property = -1;
+    this.to = {};
+    this.easing = {};
+    this.easing.type = api.Easing.LINEAR;
+
+    // How many milliseconds in the future to start the animation.
+    this.startInMillis = 0.0;
+
+    // Duration of the animation (milliseconds).
+    this.durationMillis = durationMs;
+  }
 
   /**
-   * Enumeration of animatable properties.
-   * @enum {number}
-   * @const
+   * Set the animation's final element size.
+   * @param {number} width
+   * @param {number} height
    */
-  var Property = Object.freeze({
-    'COPYRECT': 0,
-    'SIZE': 1,
-    'TRANSLATION': 2,
-    'ORIENTATION': 3,
-    'ROTATION': 4
-  });
+  setSize(width, height) {
+    this.property = api.Property.SIZE;
+    this.to.x = width;
+    this.to.y = height;
+  }
 
   /**
-   * Enumeration of easing type.
-   * @enum {number}
-   * @const
+   * Set the animation's final element scale.
+   * @param {number} x
+   * @param {number} y
+   * @param {number} z
    */
-  var Easing = Object.freeze({
-    'LINEAR': 0,
-    'CUBICBEZIER': 1,
-    'EASEIN': 2,
-    'EASEOUT': 3
-  });
+  setScale(x, y, z) {
+    this.property = api.Property.SCALE;
+    this.to.x = x;
+    this.to.y = y;
+    this.to.z = z;
+  }
 
   /**
-   * Base animation class. An animation can vary only one object property.
+   * Set the animation's final element rotation.
+   * @param {number} x
+   * @param {number} y
+   * @param {number} z
+   * @param {number} a
    */
-  class Animation {
-    constructor(elementId, durationMs) {
-      this.meshId = elementId;
-      this.to = {};
-      this.easing = {};
-      this.easing.type = api.Easing.LINEAR;
-
-      // How many milliseconds in the future to start the animation.
-      this.startInMillis = 0.0;
-
-      // Duration of the animation (milliseconds).
-      this.durationMillis = durationMs;
-    }
-
-    /**
-     * Set the animation's final element size.
-     */
-    setSize(width, height) {
-      this.property = api.Property.SIZE;
-      this.to.x = width;
-      this.to.y = height;
-    }
-
-    /**
-     * Set the animation's final element scale.
-     */
-    setScale(x, y, z) {
-      this.property = api.Property.SCALE;
-      this.to.x = x;
-      this.to.y = y;
-      this.to.z = z;
-    }
-
-    /**
-     * Set the animation's final element rotation.
-     */
-    setRotation(x, y, z, a) {
-      this.property = api.Property.ROTATION;
-      this.to.x = x;
-      this.to.y = y;
-      this.to.z = z;
-      this.to.a = a;
-    }
+  setRotation(x, y, z, a) {
+    this.property = api.Property.ROTATION;
+    this.to.x = x;
+    this.to.y = y;
+    this.to.z = z;
+    this.to.a = a;
+  }
 
-    /**
-     * Set the animation's final element translation.
-     */
-    setTranslation(x, y, z) {
-      this.property = api.Property.TRANSLATION;
-      this.to.x = x;
-      this.to.y = y;
-      this.to.z = z;
-    }
-  };
-
-  return {
-    sendCommands: sendCommands,
-    XAnchoring: XAnchoring,
-    YAnchoring: YAnchoring,
-    Property: Property,
-    Easing: Easing,
-    Command: Command,
-    Action: Action,
-    Mode: Mode,
-    getContentElementId: getContentElementId,
-    UiElement: UiElement,
-    UiElementUpdate: UiElementUpdate,
-    Animation: Animation,
-    doAction: doAction,
-    domLoaded: domLoaded,
-    setUiCssSize: setUiCssSize,
-  };
-})();
+  /**
+   * Set the animation's final element translation.
+   * @param {number} x
+   * @param {number} y
+   * @param {number} z
+   */
+  setTranslation(x, y, z) {
+    this.property = api.Property.TRANSLATION;
+    this.to.x = x;
+    this.to.y = y;
+    this.to.z = z;
+  }
+};