Add an optional set_hook argument to defineProperty.

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

Index: chrome/browser/resources/shared/js/cr/ui/menu.js
diff --git a/chrome/browser/resources/shared/js/cr/ui/menu.js b/chrome/browser/resources/shared/js/cr/ui/menu.js
index 1145d0f75a5a264fa54d44908d001827107d5bf0..59923e84101049925e4b7c4efc17ceb6516849ce 100644
--- a/chrome/browser/resources/shared/js/cr/ui/menu.js
+++ b/chrome/browser/resources/shared/js/cr/ui/menu.js
@@ -24,6 +24,8 @@ cr.define('cr.ui', function() {
       this.addEventListener('mouseover', this.handleMouseOver_);
       this.addEventListener('mouseout', this.handleMouseOut_);
 
+      this.selectedIndex = -1;
+
       // Decorate the children as menu items.
       var children = this.children;
       for (var i = 0, child; child = children[i]; i++) {
@@ -64,22 +66,16 @@ cr.define('cr.ui', function() {
     },
 
     /**
-     * The index of the selected item.
+     * Called by cr.defineProperty when the selectedIndex property changes.
      * @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');
-      }
+    set _selectedIndexChange(selectedIndex,oldSelectedIndex) {

[arv (Not doing code reviews), 2011/02/10 20:18:31]

Google style is to use trailing underscore

[arv (Not doing code reviews), 2011/02/10 20:18:31]

whitespace after ,

[arv (Not doing code reviews), 2011/02/10 20:18:31]

Shouldn't this be a function to work as set hook?

...also there is no need to put this on the prototype.

Just create a local function inside the "define" scope.

[nduca, 2011/02/10 20:47:17]

Wow, major fail on my part. :)

On 2011/02/10 20:18:31, arv wrote:

+      var oldSelectedItem = this.chidren[oldSelectedIndex];
+      if (oldSelectedItem)
+        oldSelectedItem.selected = false;
+      var item = this.selectedItem;
+      if (item)
+        item.selected = true;
     },
 
     /**
@@ -148,7 +144,8 @@ cr.define('cr.ui', function() {
    * The selected menu item.
    * @type {number}
    */
-  cr.defineProperty(Menu, 'selectedIndex', cr.PropertyKind.JS, -1);
+  cr.defineProperty(Menu, 'selectedIndex', cr.PropertyKind.JS,
+      Menu.prototype._selectedIndexChange);

[arv (Not doing code reviews), 2011/02/10 20:18:31]

selectedIndexChanged

 
   // Export
   return {