WebUI: Convert cr.defineProperty() to Object.defineProperty()

Change the use of cr.defineProperty() to Object.defineProperty() + type
declaration.  This is basically what Closure ChromePass was doing [1],
but we don't want to depend on ChromePass when using JS modules.

NOTE that this changes the property definition for both module and
non-module versions, but they should have the same behaviour, because it
uses the same getters and setters.

[1] - https://github.com/google/closure-compiler/blob/master/test/com/google/javascript/jscomp/ChromePassTest.java#L415-L427

Bug: 1133198, 1134497
Change-Id: I787dc3d7f2fa8dc290f7839c76c347b5695a0038
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2440331
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: dpapad <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816527}

ui/webui/resources/js/cr.js

@@ -222,6 +222,25 @@ var cr = cr || function(global) {
     }
   }
 
+  /**
+   * Returns a getter and setter to be used as property descriptor in
+   * Object.defineProperty(). When the setter changes the value a property
+   * change event with the type {@code name + 'Change'} is fired.
+   * @param {string} name The name of the property.
+   * @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 getPropertyDescriptor(name, opt_kind, opt_setHook) {
+    const kind = /** @type {PropertyKind} */ (opt_kind || PropertyKind.JS);
+
+    const desc = {
+      get: getGetter(name, kind),
+      set: getSetter(name, kind, opt_setHook),
+    };
+    return desc;
+  }
+
   /**
    * Counter for use with createUid
    */
@@ -415,6 +434,7 @@ var cr = cr || function(global) {
     addSingletonGetter: addSingletonGetter,
     define: define,
     defineProperty: defineProperty,
+    getPropertyDescriptor: getPropertyDescriptor,
     dispatchPropertyChange: dispatchPropertyChange,
     dispatchSimpleEvent: dispatchSimpleEvent,
     PropertyKind: PropertyKind,

ui/webui/resources/js/cr.m.js

@@ -9,7 +9,7 @@
  * logic in js_modulizer.py only to address the cr.js case, which is not worth
  * it.
  */
-import {assert} from './assert.m.js';
+import {assert, assertNotReached} from './assert.m.js';
 import {PromiseResolver} from './promise_resolver.m.js';
 
 /** @typedef {{eventName: string, uid: number}} */
@@ -227,3 +227,153 @@ export const isAndroid = /Android/.test(navigator.userAgent);
 
 /** Whether this is on iOS. */
 export const isIOS = /CriOS/.test(navigator.userAgent);
+
+/**
+ * Converts a camelCase javascript property name to a hyphenated-lower-case
+ * attribute name.
+ * @param {string} jsName The javascript camelCase property name.
+ * @return {string} The equivalent hyphenated-lower-case attribute name.
+ */
+function getAttributeName(jsName) {
+  return jsName.replace(/([A-Z])/g, '-$1').toLowerCase();
+}
+
+/**
+ * The kind of property to define in {@code getPropertyDescriptor}.
+ * @enum {string}
+ * @const
+ */
+export const PropertyKind = {
+  /**
+   * Plain old JS property where the backing data is stored as a "private"
+   * field on the object.
+   * Use for properties of any type. Type will not be checked.
+   */
+  JS: 'js',
+
+  /**
+   * The property backing data is stored as an attribute on an element.
+   * Use only for properties of type {string}.
+   */
+  ATTR: 'attr',
+
+  /**
+   * The property backing data is stored as an attribute on an element. If the
+   * element has the attribute then the value is true.
+   * Use only for properties of type {boolean}.
+   */
+  BOOL_ATTR: 'boolAttr'
+};
+
+/**
+ * Helper function for getPropertyDescriptor that returns the getter to use for
+ * the property.
+ * @param {string} name The name of the property.
+ * @param {PropertyKind} kind The kind of the property.
+ * @return {function():*} The getter for the property.
+ */
+function getGetter(name, kind) {
+  let attributeName;
+  switch (kind) {
+    case PropertyKind.JS:
+      const privateName = name + '_';
+      return function() {
+        return this[privateName];
+      };
+    case PropertyKind.ATTR:
+      attributeName = getAttributeName(name);
+      return function() {
+        return this.getAttribute(attributeName);
+      };
+    case PropertyKind.BOOL_ATTR:
+      attributeName = getAttributeName(name);
+      return function() {
+        return this.hasAttribute(attributeName);
+      };
+  }
+
+  assertNotReached();
+}
+
+/**
+ * Helper function for getPropertyDescriptor that returns the setter of the
+ * right kind.
+ * @param {string} name The name of the property we are defining the setter
+ *     for.
+ * @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.
+ * @return {function(*):void} The function to use as a setter.
+ */
+function getSetter(name, kind, opt_setHook) {
+  let attributeName;
+  switch (kind) {
+    case PropertyKind.JS:
+      const privateName = name + '_';
+      return function(value) {
+        const oldValue = this[name];
+        if (value !== oldValue) {
+          this[privateName] = value;
+          if (opt_setHook) {
+            opt_setHook.call(this, value, oldValue);
+          }
+          dispatchPropertyChange(this, name, value, oldValue);
+        }
+      };
+
+    case PropertyKind.ATTR:
+      attributeName = getAttributeName(name);
+      return function(value) {
+        const oldValue = this[name];
+        if (value !== oldValue) {
+          if (value === undefined) {
+            this.removeAttribute(attributeName);
+          } else {
+            this.setAttribute(attributeName, value);
+          }
+          if (opt_setHook) {
+            opt_setHook.call(this, value, oldValue);
+          }
+          dispatchPropertyChange(this, name, value, oldValue);
+        }
+      };
+
+    case PropertyKind.BOOL_ATTR:
+      attributeName = getAttributeName(name);
+      return function(value) {
+        const oldValue = this[name];
+        if (value !== oldValue) {
+          if (value) {
+            this.setAttribute(attributeName, name);
+          } else {
+            this.removeAttribute(attributeName);
+          }
+          if (opt_setHook) {
+            opt_setHook.call(this, value, oldValue);
+          }
+          dispatchPropertyChange(this, name, value, oldValue);
+        }
+      };
+  }
+
+  assertNotReached();
+}
+
+/**
+ * Returns a getter and setter to be used as property descriptor in
+ * Object.defineProperty(). When the setter changes the value a property change
+ * event with the type {@code name + 'Change'} is fired.
+ * @param {string} name The name of the property.
+ * @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.
+ */
+export function getPropertyDescriptor(name, opt_kind, opt_setHook) {
+  const kind = /** @type {PropertyKind} */ (opt_kind || PropertyKind.JS);
+
+  return {
+    get: getGetter(name, kind),
+    set: getSetter(name, kind, opt_setHook),
+  };
+}

ui/webui/resources/js/cr/ui/BUILD.gn

@@ -257,6 +257,8 @@ js_modulizer("modulize") {
     "focus_row_behavior.js",
     "focus_without_ink.js",
     "keyboard_shortcut_list.js",
+    "list.js",
+    "list_item.js",
     "list_selection_controller.js",
     "list_selection_model.js",
     "list_single_selection_model.js",
@@ -264,8 +266,11 @@ js_modulizer("modulize") {
     "splitter.js",
     "store.js",
     "store_client.js",
+    "tree.js",
   ]
   namespace_rewrites = [
+    "cr.getPropertyDescriptor|getPropertyDescriptor",
+    "cr.PropertyKind|PropertyKind",
     "cr.dispatchPropertyChange|dispatchPropertyChange",
     "cr.EventTarget|EventTarget",
     "cr.ui.dialogs.BaseDialog|BaseDialog",
@@ -280,8 +285,15 @@ js_modulizer("modulize") {
     "cr.ui.DragWrapperDelegate|DragWrapperDelegate",
     "cr.ui.FocusRowDelegate|FocusRowDelegate",
     "cr.ui.FocusRow|FocusRow",
+    "cr.ui.limitInputWidth|limitInputWidth",
+    "cr.ui.List|List",
+    "cr.ui.Size|Size",
+    "cr.ui.ListItem|ListItem",
     "cr.ui.ListSelectionModel|ListSelectionModel",
+    "cr.ui.ListSelectionController|ListSelectionController",
     "cr.ui.StoreObserver|StoreObserver",
+    "cr.ui.Tree|Tree",
+    "cr.ui.TreeItem|TreeItem",
     "cr.ui.VirtualFocusRow|VirtualFocusRow",
   ]
 }
@@ -298,6 +310,8 @@ js_type_check("ui_resources_modules") {
     ":focus_row_behavior.m",
     ":focus_without_ink.m",
     ":keyboard_shortcut_list.m",
+    ":list.m",
+    ":list_item.m",
     ":list_selection_controller.m",
     ":list_selection_model.m",
     ":list_single_selection_model.m",
@@ -305,6 +319,7 @@ js_type_check("ui_resources_modules") {
     ":splitter.m",
     ":store.m",
     ":store_client.m",
+    ":tree.m",
   ]
 }
 
@@ -383,6 +398,25 @@ js_library("keyboard_shortcut_list.m") {
   extra_deps = [ ":modulize" ]
 }
 
+js_library("list.m") {
+  sources = [ "$root_gen_dir/ui/webui/resources/js/cr/ui/list.m.js" ]
+  deps = [
+    ":array_data_model.m",
+    ":list_item.m",
+    ":list_selection_controller.m",
+    ":list_selection_model.m",
+    "../:ui.m",
+    "../..:cr.m",
+  ]
+  extra_deps = [ ":modulize" ]
+}
+
+js_library("list_item.m") {
+  sources = [ "$root_gen_dir/ui/webui/resources/js/cr/ui/list_item.m.js" ]
+  deps = [ "../..:cr.m" ]
+  extra_deps = [ ":modulize" ]
+}
+
 js_library("list_selection_controller.m") {
   sources = [
     "$root_gen_dir/ui/webui/resources/js/cr/ui/list_selection_controller.m.js",
@@ -440,3 +474,13 @@ js_library("store_client.m") {
   sources = [ "$root_gen_dir/ui/webui/resources/js/cr/ui/store_client.m.js" ]
   extra_deps = [ ":modulize" ]
 }
+
+js_library("tree.m") {
+  sources = [ "$root_gen_dir/ui/webui/resources/js/cr/ui/tree.m.js" ]
+  deps = [
+    "../:ui.m",
+    "../..:assert.m",
+    "../..:cr.m",
+  ]
+  extra_deps = [ ":modulize" ]
+}

ui/webui/resources/js/cr/ui/list.js

@@ -7,14 +7,25 @@
 // require: list_selection_controller.js
 // require: list_item.js
 
+// clang-format off
+// #import {define as crUiDefine} from '../ui.m.js';
+// #import {getPropertyDescriptor, PropertyKind, dispatchSimpleEvent} from '../../cr.m.js';
+// #import {ArrayDataModel} from './array_data_model.m.js';
+// #import {ListSelectionModel} from './list_selection_model.m.js';
+// #import {ListSelectionController} from './list_selection_controller.m.js';
+// #import {ListItem} from './list_item.m.js';
+// clang-format on
+
 /**
  * @fileoverview This implements a list control.
  */
 
 cr.define('cr.ui', function() {
-  /** @const */ const ListSelectionModel = cr.ui.ListSelectionModel;
-  /** @const */ const ListSelectionController = cr.ui.ListSelectionController;
-  /** @const */ const ArrayDataModel = cr.ui.ArrayDataModel;
+  /* #ignore */ /** @const */ const ListSelectionModel =
+      /* #ignore */ cr.ui.ListSelectionModel;
+  /* #ignore */ /** @const */ const ListSelectionController =
+      /* #ignore */ cr.ui.ListSelectionController;
+  /* #ignore */ /** @const */ const ArrayDataModel = cr.ui.ArrayDataModel;
 
   /**
    *  @typedef {{
@@ -26,7 +37,7 @@ cr.define('cr.ui', function() {
    *    width: number
    *  }}
    */
-  let Size;
+  /* #export */ let Size;
 
   /**
    * Whether a mouse event is inside the element viewport. This will return
@@ -55,7 +66,7 @@ cr.define('cr.ui', function() {
    * @constructor
    * @extends {HTMLUListElement}
    */
-  const List = cr.ui.define('list');
+  /* #export */ const List = cr.ui.define('list');
 
   List.prototype = {
     __proto__: HTMLUListElement.prototype,
@@ -1364,15 +1375,23 @@ cr.define('cr.ui', function() {
     },
   };
 
-  cr.defineProperty(List, 'disabled', cr.PropertyKind.BOOL_ATTR);
+  /** @type {boolean} */
+  List.prototype.disabled;
+  Object.defineProperty(
+      List.prototype, 'disabled',
+      cr.getPropertyDescriptor('disabled', cr.PropertyKind.BOOL_ATTR));
 
   /**
    * Whether the list or one of its descendents has focus. This is necessary
    * because list items can contain controls that can be focused, and for some
    * purposes (e.g., styling), the list can still be conceptually focused at
    * that point even though it doesn't actually have the page focus.
+   * @type {boolean}
    */
-  cr.defineProperty(List, 'hasElementFocus', cr.PropertyKind.BOOL_ATTR);
+  List.prototype.hasElementFocus;
+  Object.defineProperty(
+      List.prototype, 'hasElementFocus',
+      cr.getPropertyDescriptor('hasElementFocus', cr.PropertyKind.BOOL_ATTR));
 
   /**
    * Mousedown event handler.
@@ -1453,6 +1472,7 @@ cr.define('cr.ui', function() {
     return false;
   }
 
+  // #cr_define_end
   return {
     List: List,
     Size: Size,

ui/webui/resources/js/cr/ui/list_item.js

@@ -2,14 +2,18 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-cr.define('cr.ui', function() {
+// clang-format off
+// #import {PropertyKind, getPropertyDescriptor} from '../../cr.m.js';
+// #import {define as crUiDefine} from '../ui.m.js';
+// clang-format on
 
+cr.define('cr.ui', function() {
   /**
    * Creates a new list item element.
    * @constructor
    * @extends {HTMLLIElement}
    */
-  const ListItem = cr.ui.define('li');
+  /* #export */ const ListItem = cr.ui.define('li');
 
   /**
    * The next id suffix to use when giving each item an unique id.
@@ -57,23 +61,34 @@ cr.define('cr.ui', function() {
   /**
    * Whether the item is selected. Setting this does not update the underlying
    * selection model. This is only used for display purpose.
+   * @type {boolean}
    */
-  cr.defineProperty(
-      ListItem, 'selected', cr.PropertyKind.BOOL_ATTR, function() {
-        this.selectionChanged();
-      });
+  ListItem.prototype.selected;
+  Object.defineProperty(
+      ListItem.prototype, 'selected',
+      cr.getPropertyDescriptor(
+          'selected', cr.PropertyKind.BOOL_ATTR, function() {
+            this.selectionChanged();
+          }));
 
   /**
    * Whether the item is the lead in a selection. Setting this does not update
    * the underlying selection model. This is only used for display purpose.
+   * @type {boolean}
    */
-  cr.defineProperty(ListItem, 'lead', cr.PropertyKind.BOOL_ATTR);
+  ListItem.prototype.lead;
+  Object.defineProperty(
+      ListItem.prototype, 'lead',
+      cr.getPropertyDescriptor('lead', cr.PropertyKind.BOOL_ATTR));
 
   /**
    * This item's index in the containing list.
    * type {number}
    */
-  cr.defineProperty(ListItem, 'listIndex');
+  ListItem.prototype.listIndex;
+  Object.defineProperty(
+      ListItem.prototype, 'listIndex', cr.getPropertyDescriptor('listIndex'));
 
+  // #cr_define_end
   return {ListItem: ListItem};
 });

ui/webui/resources/js/cr/ui/tree.js

@@ -2,6 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// clang-format off
+// #import {assert, assertInstanceof} from '../../assert.m.js';
+// #import {define as crUiDefine, limitInputWidth} from '../ui.m.js';
+// #import {isMac, dispatchSimpleEvent, getPropertyDescriptor, PropertyKind} from '../../cr.m.js';
+// clang-format on
+
 cr.define('cr.ui', function() {
   // require cr.ui.define
   // require cr.ui.limitInputWidth
@@ -48,7 +54,7 @@ cr.define('cr.ui', function() {
    * @constructor
    * @extends {HTMLElement}
    */
-  const Tree = cr.ui.define('tree');
+  /* #export */ const Tree = cr.ui.define('tree');
 
   Tree.prototype = {
     __proto__: HTMLElement.prototype,
@@ -276,8 +282,12 @@ cr.define('cr.ui', function() {
    * next to expandable parent nodes. If set to 'all' folder icons will be
    * displayed next to all nodes. Icons can be set using the treeItem's icon
    * property.
+   * @type {boolean}
    */
-  cr.defineProperty(Tree, 'iconVisibility', cr.PropertyKind.ATTR);
+  Tree.prototype.iconVisibility;
+  Object.defineProperty(
+      Tree.prototype, 'iconVisibility',
+      cr.getPropertyDescriptor('iconVisibility', cr.PropertyKind.ATTR));
 
   /**
    * Incremental counter for an auto generated ID of the tree item. This will
@@ -328,7 +338,7 @@ cr.define('cr.ui', function() {
    * @constructor
    * @extends {HTMLElement}
    */
-  const TreeItem = cr.ui.define(function() {
+  /* #export */ const TreeItem = cr.ui.define(function() {
     const treeItem = treeItemProto.cloneNode(true);
     treeItem.id = 'tree-item-autogen-id-' + treeItemAutoGeneratedIdCounter++;
     return treeItem;
@@ -772,5 +782,6 @@ cr.define('cr.ui', function() {
   }
 
   // Export
+  // #cr_define_end
   return {Tree: Tree, TreeItem: TreeItem};
 });