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

chrome/browser/resources/vr_shell/vr_shell_ui_api.js

 // Copyright 2016 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.
 
-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.
-   */
-  function sendCommands(commands) {
-    chrome.send('updateScene', commands);
-  }
-
-  /**
-   * Returns the element ID of the browser content quad, allowing the content to
-   * be manimulated in the scene.
-   */
-  function getContentElementId() {
-    return 0;
-  }
-
-  /**
-   * 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
-   */
-  var XAnchoring = Object.freeze({
-    'XNONE': 0,
-    'XLEFT': 1,
-    'XRIGHT': 2
-  });
-
-  /**
-   * Enumeration of valid Anchroing for Y axis.
-   * @enum {number}
-   * @const
-   */
-  var YAnchoring = Object.freeze({
-    'YNONE': 0,
-    'YTOP': 1,
-    'YBOTTOM': 2
-  });
-
-  /**
-   * Enumeration of actions that can be triggered by the HTML UI.
-   * @enum {number}
-   * @const
-   */
-  var Action = Object.freeze({
-    '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
-   */
-  var Mode = Object.freeze({
-    'UNKNOWN': -1,
-    'STANDARD': 0,
-    'WEB_VR': 1,
-  });
-
-  /**
-   * Triggers an 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.
-   */
-  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.
-   */
-  class UiElementUpdate {
-
-    setIsContentQuad() {
-      this.contentQuad = true;
-    }
+};
+
+/**
+ * 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.

 
     /**
-     * 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.
+     * Handlers that are listening to this logger.
+     * @private {number}
      */
-    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;
-    }
-  };
-
-  /**
-   * Represents a new UI element. This object builds on UiElementUpdate,
-   * forcing the underlying texture coordinates to be specified.
-   */
-  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
-      };
-    }
-  };
-
-  /**
-   * Enumeration of animatable properties.
-   * @enum {number}
-   * @const
-   */
-  var Property = Object.freeze({
-    'COPYRECT': 0,
-    'SIZE': 1,
-    'TRANSLATION': 2,
-    'ORIENTATION': 3,
-    'ROTATION': 4
-  });
-
-  /**
-   * Enumeration of easing type.
-   * @enum {number}
-   * @const
-   */
-  var Easing = Object.freeze({
-    'LINEAR': 0,
-    'CUBICBEZIER': 1,
-    'EASEIN': 2,
-    'EASEOUT': 3
-  });
-
-  /**
-   * Base animation class. An animation can vary only one object property.
-   */
-  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;
-    }
-
-    /**
-     * 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,
-  };
-})();
+    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.

+  }
+
+  /**
+  * Operates on an instance of MyClass and returns something.

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

Bad copy paste?

+  */
+  setIsContentQuad() {
+    this.properties['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.
+   * @param {number} id
+   */
+  setParentId(id) {
+    this.properties['parentId'] = id;
+  }
+
+  /**
+   * Specify the width and height (in meters) of an element.
+   * @param {number} x
+   * @param {number} y
+   */
+  setSize(x, y) {
+    this.properties['size'] = { x: x, y: y };
+  }
+
+  /**
+   * Specify optional scaling of the element, and any children.
+   * @param {number} x
+   * @param {number} y
+   * @param {number} z
+   */
+  setScale(x, y, z) {
+    this.properties['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).
+   * @param {number} x
+   * @param {number} y
+   * @param {number} z
+   * @param {number} a
+   */
+  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.

+  }
+
+  /**
+   * 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
+   */
+
+  // TODO - also test private member enforcement.
+
+  setTranslation(x, y, z) {
+    this.properties['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);
+   * @param {number} x
+   * @param {number} y
+   */
+  setAnchoring(x, y) {
+    this.properties['xAnchoring'] = x;
+    this.properties['yAnchoring'] = y;
+  }
+
+  /**
+   * Visibility controls whether the element is rendered.
+   * @param {boolean} visible
+   */
+  setVisible(visible) {
+    this.properties['visible'] = !!visible;
+  }
+
+  /**
+   * Hit-testable implies that the reticle will hit the element, if visible.
+   * @param {boolean} testable
+   */
+  setHitTestable(testable) {
+    this.properties['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.
+   * @param {boolean} 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 {
+  /**
+   * 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
+    };
+  }
+};
+
+/**
+ * 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;
+  }
+
+  /**
+   * Set the animation's final element size.
+   * @param {number} width
+   * @param {number} height
+   */
+  setSize(width, height) {
+    this.property = api.Property.SIZE;
+    this.to.x = width;
+    this.to.y = height;
+  }
+
+  /**
+   * Set the animation's final element scale.
+   * @param {number} x
+   * @param {number} y
+   * @param {number} z
+   */
+  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.
+   * @param {number} x
+   * @param {number} y
+   * @param {number} z
+   * @param {number} 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.
+   * @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;
+  }
+};