Add an optional set_hook argument to defineProperty.

chrome/browser/resources/shared/js/cr/ui/menu.js

 // Copyright (c) 2010 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('cr.ui', function() {
 
   const MenuItem = cr.ui.MenuItem;
 
   /**
    * Creates a new menu element.
    * @param {Object=} opt_propertyBag Optional properties.
    * @constructor
    * @extends {HTMLMenuElement}
    */
   var Menu = cr.ui.define('menu');
 
   Menu.prototype = {
     __proto__: HTMLMenuElement.prototype,
 
     /**
      * Initializes the menu element.
      */
     decorate: function() {
       this.addEventListener('mouseover', this.handleMouseOver_);
       this.addEventListener('mouseout', this.handleMouseOut_);
 
+      this.selectedIndex = -1;

[arv (Not doing code reviews), 2011/02/10 21:30:14]

This should probably be done using

{
  ...
  selectedIndex_: -1,
  ...
}

[nduca, 2011/02/11 20:40:04]

Isn't this worse? You'd be baking in knowledge that cr.js uses propertyName_ for
defining property backings...

On 2011/02/10 21:30:14, arv wrote:

+
       // Decorate the children as menu items.
       var children = this.children;
       for (var i = 0, child; child = children[i]; i++) {
         cr.ui.decorate(child, MenuItem);
       }
     },
 
     /**
      * Walks up the ancestors until a menu item belonging to this menu is found.
      * @param {Element} el
@@ skipping 20 matching lines @@
     /**
      * Handles mouseout events and deselects any selected item.
      * @param {Event} e The mouseout event.
      * @private
      */
     handleMouseOut_: function(e) {
       this.selectedItem = null;
     },
 
     /**
-     * The index of the selected item.
-     * @type {boolean}
-     */
-    // getter and default value is defined using cr.defineProperty.
-    set selectedIndex(selectedIndex) {
-      if (this.selectedIndex_ != selectedIndex) {
-        var oldSelectedItem = this.selectedItem;
-        this.selectedIndex_ = selectedIndex;
-        if (oldSelectedItem)
-          oldSelectedItem.selected = false;
-        var item = this.selectedItem;
-        if (item)
-          item.selected = true;
-
-        cr.dispatchSimpleEvent(this, 'change');
-      }
-    },
-
-    /**
      * The selected menu item or null if none.
      * @type {cr.ui.MenuItem}
      */
     get selectedItem() {
       return this.children[this.selectedIndex];
     },
     set selectedItem(item) {
       var index = Array.prototype.indexOf.call(this.children, item);
       this.selectedIndex = index;
     },
@@ skipping 45 matching lines @@
       }
 
       return false;
     }
   };
 
   /**
    * The selected menu item.
    * @type {number}
    */
-  cr.defineProperty(Menu, 'selectedIndex', cr.PropertyKind.JS, -1);
+  cr.defineProperty(Menu, 'selectedIndex', cr.PropertyKind.JS,
+      function(selectedIndex, oldSelectedIndex) {

[arv (Not doing code reviews), 2011/02/10 21:30:14]

This is fine but I think this would be easier to read if you created a named
function and used that instead.

function selectedIndexChanged(selectedIndex, oldSelectedIndex) {
  ...
}

cr.defineProperty(Menu, 'selectedIndex', cr.PropertyKind.JS,
selectedIndexChanged);

[nduca, 2011/02/11 20:40:04]

Done.

+        var oldSelectedItem = this.chidren[oldSelectedIndex];
+        if (oldSelectedItem)
+          oldSelectedItem.selected = false;
+        var item = this.selectedItem;
+        if (item)
+          item.selected = true;
+      });
 
   // Export
   return {
     Menu: Menu
   };
 });