Fix menu scrolling and scroll-related item-bounds tracking in non-stick menu mode.

Refactor DraggableButton to remove some of the menu-related complexity I added and move it to BookmarkButton where it belongs.
Fix leak of the BookmarkButton tracking area.
Add support for receiving nil NSEvents.
BUG=75076, 75077
Review URL: http://codereview.chromium.org/6657027

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@77678 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 167 insertions, 69 deletions

diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
index ddde6e4..ac05aff 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller.mm
@@ -2134,7 +2134,8 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
     [[hoverButton_ target]
      performSelector:@selector(openBookmarkFolderFromButton:)
      withObject:hoverButton_
-     afterDelay:bookmarks::kDragHoverOpenDelay];
+     afterDelay:bookmarks::kDragHoverOpenDelay
+     inModes:[NSArray arrayWithObject:NSRunLoopCommonModes]];
   }
   if (!button) {
     if (hoverButton_) {
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h
index 54b918c..5c9288c 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.h
@@ -169,6 +169,9 @@
 // Passed up by a child view to tell us of a desire to scroll.
 - (void)scrollWheel:(NSEvent *)theEvent;
 
+- (void)mouseDragged:(NSEvent*)theEvent;
+
+
 // Forwarded to the associated BookmarkBarController.
 - (IBAction)addFolder:(id)sender;
 - (IBAction)addPage:(id)sender;
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
index b74b2cf..6dba84e 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
@@ -103,6 +103,12 @@ struct LayoutMetrics {
 
 }  // namespace
 
+
+// Required to set the right tracking bounds for our fake menus.
+@interface NSView(Private)
+- (void)_updateTrackingAreas;
+@end
+
 @interface BookmarkBarFolderController(Private)
 - (void)configureWindow;
 - (void)addOrUpdateScrollTracking;
@@ -691,6 +697,16 @@ struct LayoutMetrics {
   if (!metrics.preScroll)
     [[scrollView_ documentView] scrollPoint:metrics.scrollPoint];
 
+  // TODO(maf) find a non-SPI way to do this.
+  // Hack. This is the only way I've found to get the tracking area cache
+  // to update properly during a mouse tracking loop.
+  // Without this, the item tracking-areas are wrong when using a scrollable
+  // menu with the mouse held down.
+  NSView *contentView = [[self window] contentView] ;
+  if ([contentView respondsToSelector:@selector(_updateTrackingAreas)])
+    [contentView _updateTrackingAreas];
+
+
   if (metrics.canScrollUp != metrics.couldScrollUp ||
       metrics.canScrollDown != metrics.couldScrollDown ||
       metrics.scrollDelta != 0.0) {
@@ -796,8 +812,8 @@ struct LayoutMetrics {
   [folderView_ setFrame:folderFrame];
   NSSize newSize = NSMakeSize(windowWidth, 0.0);
   [self adjustWindowLeft:newWindowTopLeft.x size:newSize scrollingBy:0.0];
-  [window display];
   [self configureWindowLevel];
+  [window display];
 }
 
 // TODO(mrossetti): See if the following can be moved into view's viewWillDraw:.
@@ -901,43 +917,44 @@ struct LayoutMetrics {
 }
 
 
-// Add a timer to fire at a regular interveral which scrolls the
+// Add a timer to fire at a regular interval which scrolls the
 // window vertically |delta|.
 - (void)addScrollTimerWithDelta:(CGFloat)delta {
   if (scrollTimer_ && verticalScrollDelta_ == delta)
     return;
   [self endScroll];
   verticalScrollDelta_ = delta;
-  scrollTimer_ =
-      [NSTimer scheduledTimerWithTimeInterval:kBookmarkBarFolderScrollInterval
-                                       target:self
-                                     selector:@selector(performScroll:)
-                                     userInfo:nil
-                                      repeats:YES];
+  scrollTimer_ = [NSTimer timerWithTimeInterval:kBookmarkBarFolderScrollInterval
+                                         target:self
+                                       selector:@selector(performScroll:)
+                                       userInfo:nil
+                                        repeats:YES];
+
+  [[NSRunLoop mainRunLoop] addTimer:scrollTimer_ forMode:NSRunLoopCommonModes];
 }
 
+
 // Called as a result of our tracking area.  Warning: on the main
 // screen (of a single-screened machine), the minimum mouse y value is
 // 1, not 0.  Also, we do not get events when the mouse is above the
 // menubar (to be fixed by setting the proper window level; see
 // initializer).
-- (void)mouseMoved:(NSEvent*)theEvent {
-  NSWindow* window = [theEvent window];
-  DCHECK(window == [self window]);
-
+// Note [theEvent window] may not be our window, as we also get these messages
+// forwarded from BookmarkButton's mouse tracking loop.
+- (void)mouseMovedOrDragged:(NSEvent*)theEvent {
   NSPoint eventScreenLocation =
-      [window convertBaseToScreen:[theEvent locationInWindow]];
+      [[theEvent window] convertBaseToScreen:[theEvent locationInWindow]];
 
   // Base hot spot calculations on the positions of the scroll arrow views.
   NSRect testRect = [scrollDownArrowView_ frame];
   NSPoint testPoint = [visibleView_ convertPoint:testRect.origin
                                                   toView:nil];
-  testPoint = [window convertBaseToScreen:testPoint];
+  testPoint = [[self window] convertBaseToScreen:testPoint];
   CGFloat closeToTopOfScreen = testPoint.y;
 
   testRect = [scrollUpArrowView_ frame];
   testPoint = [visibleView_ convertPoint:testRect.origin toView:nil];
-  testPoint = [window convertBaseToScreen:testPoint];
+  testPoint = [[self window] convertBaseToScreen:testPoint];
   CGFloat closeToBottomOfScreen = testPoint.y + testRect.size.height;
   if (eventScreenLocation.y <= closeToBottomOfScreen &&
       ![scrollUpArrowView_ isHidden]) {
@@ -950,6 +967,14 @@ struct LayoutMetrics {
   }
 }
 
+- (void)mouseMoved:(NSEvent*)theEvent {
+  [self mouseMovedOrDragged:theEvent];
+}
+
+- (void)mouseDragged:(NSEvent*)theEvent {
+  [self mouseMovedOrDragged:theEvent];
+}
+
 - (void)mouseExited:(NSEvent*)theEvent {
   [self endScroll];
 }
@@ -964,7 +989,9 @@ struct LayoutMetrics {
                               initWithRect:[view bounds]
                                    options:(NSTrackingMouseMoved |
                                             NSTrackingMouseEnteredAndExited |
-                                            NSTrackingActiveAlways)
+                                            NSTrackingActiveAlways |
+                                            NSTrackingEnabledDuringMouseDrag
+                                            )
                               proxiedOwner:self
                                   userInfo:nil]);
   [view addTrackingArea:scrollTrackingArea_.get()];
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_hover_state.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_hover_state.mm
index b762bb3c..abd3f88 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_hover_state.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_hover_state.mm
@@ -85,7 +85,8 @@
   [self setHoverState:kHoverStateClosing];
   [self performSelector:@selector(closeBookmarkFolderOnHoverButton:)
              withObject:hoverButton_
-             afterDelay:bookmarks::kDragHoverCloseDelay];
+             afterDelay:bookmarks::kDragHoverCloseDelay
+                inModes:[NSArray arrayWithObject:NSRunLoopCommonModes]];
 }
 
 // Cancel pending hover close.  Transition to kHoverStateOpen state.
@@ -104,7 +105,8 @@
   [self setHoverState:kHoverStateOpening];
   [self performSelector:@selector(openBookmarkFolderOnHoverButton:)
              withObject:hoverButton_
-             afterDelay:bookmarks::kDragHoverOpenDelay];
+             afterDelay:bookmarks::kDragHoverOpenDelay
+                inModes:[NSArray arrayWithObject:NSRunLoopCommonModes]];
 }
 
 // Cancel pending hover open.  Transition to kHoverStateClosed state.
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_button.h b/chrome/browser/ui/cocoa/bookmarks/bookmark_button.h
index c04b3eb..3ab16e6 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_button.h
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_button.h
@@ -212,6 +212,10 @@ class ThemeProvider;
 // Return YES if this is a folder button (the node has subnodes).
 - (BOOL)isFolder;
 
+- (void)mouseDragged:(NSEvent*)theEvent;
+
+- (BOOL)acceptsTrackInFrom:(id)sender;
+
 // At this time we represent an empty folder (e.g. the string
 // '(empty)') as a disabled button with no associated node.
 //
diff --git a/chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm b/chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm
index e9f0575..4938f46 100644
--- a/chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm
+++ b/chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm
@@ -9,6 +9,7 @@
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/metrics/user_metrics.h"
 #import "chrome/browser/ui/cocoa/bookmarks/bookmark_button_cell.h"
+#import "chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_window.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
 #import "chrome/browser/ui/cocoa/view_id_util.h"
 
@@ -63,6 +64,12 @@ BookmarkButton* gDraggedButton = nil; // Weak
   if ([[self cell] respondsToSelector:@selector(safelyStopPulsing)])
     [[self cell] safelyStopPulsing];
   view_id_util::UnsetID(self);
+
+  if (area_) {
+    [self removeTrackingArea:area_];
+    [area_ release];
+  }
+
   [super dealloc];
 }
 
@@ -243,6 +250,72 @@ BookmarkButton* gDraggedButton = nil; // Weak
   }
 }
 
+- (void)performMouseDownAction:(NSEvent*)theEvent {
+  int eventMask = NSLeftMouseUpMask | NSMouseEnteredMask | NSMouseExitedMask |
+      NSLeftMouseDraggedMask;
+
+  BOOL keepGoing = YES;
+  [[self target] performSelector:[self action] withObject:self];
+  self.actionHasFired = YES;
+
+  DraggableButton* insideBtn = nil;
+
+  while (keepGoing) {
+    theEvent = [[self window] nextEventMatchingMask:eventMask];
+    if (!theEvent)
+      continue;
+
+    NSPoint mouseLoc = [self convertPoint:[theEvent locationInWindow]
+                                 fromView:nil];
+    BOOL isInside = [self mouse:mouseLoc inRect:[self bounds]];
+
+    switch ([theEvent type]) {
+      case NSMouseEntered:
+      case NSMouseExited: {
+        NSView* trackedView = (NSView*)[[theEvent trackingArea] owner];
+        if (trackedView && [trackedView isKindOfClass:[self class]]) {
+          BookmarkButton* btn = static_cast<BookmarkButton*>(trackedView);
+          if (![btn acceptsTrackInFrom:self])
+            break;
+          if ([theEvent type] == NSMouseEntered) {
+            [[NSCursor arrowCursor] set];
+            [[btn cell] mouseEntered:theEvent];
+            insideBtn = btn;
+          } else {
+            [[btn cell] mouseExited:theEvent];
+            if (insideBtn == btn)
+              insideBtn = nil;
+          }
+        }
+        break;
+      }
+      case NSLeftMouseDragged: {
+        if (insideBtn)
+          [insideBtn mouseDragged:theEvent];
+        break;
+      }
+      case NSLeftMouseUp: {
+        if (!isInside && insideBtn && insideBtn != self) {
+          // Has tracked onto another DraggableButton menu item, and released,
+          // so click it.
+          [insideBtn performClick:self];
+        }
+        self.durationMouseWasDown = [theEvent timestamp] - self.whenMouseDown;
+        [self secondaryMouseUpAction:isInside];
+        [[self cell] mouseExited:theEvent];
+        [[insideBtn cell] mouseExited:theEvent];
+        keepGoing = NO;
+        break;
+      }
+      default:
+        /* Ignore any other kind of event. */
+        break;
+    }
+  }
+}
+
+
+
 // mouseEntered: and mouseExited: are called from our
 // BookmarkButtonCell.  We redirect this information to our delegate.
 // The controller can then perform menu-like actions (e.g. "hover over
@@ -256,6 +329,16 @@ BookmarkButton* gDraggedButton = nil; // Weak
   [delegate_ mouseExitedButton:self event:event];
 }
 
+- (void)mouseMoved:(NSEvent*)theEvent {
+  if ([delegate_ respondsToSelector:@selector(mouseMoved:)])
+    [id(delegate_) mouseMoved:theEvent];
+}
+
+- (void)mouseDragged:(NSEvent*)theEvent {
+  if ([delegate_ respondsToSelector:@selector(mouseDragged:)])
+    [id(delegate_) mouseDragged:theEvent];
+}
+
 + (BookmarkButton*)draggedButton {
   return gDraggedButton;
 }
@@ -277,13 +360,17 @@ BookmarkButton* gDraggedButton = nil; // Weak
 
 @implementation BookmarkButton(Private)
 
-- (void)installCustomTrackingArea {
-  if (area_)
-    return;
 
-  NSTrackingAreaOptions options = NSTrackingActiveInActiveApp |
-      NSTrackingMouseEnteredAndExited | NSTrackingEnabledDuringMouseDrag |
-      NSTrackingInVisibleRect;
+- (void)installCustomTrackingArea {
+  const NSTrackingAreaOptions options =
+      NSTrackingActiveAlways |
+      NSTrackingMouseEnteredAndExited |
+      NSTrackingEnabledDuringMouseDrag;
+
+  if (area_) {
+    [self removeTrackingArea:area_];
+    [area_ release];
+  }
 
   area_ = [[NSTrackingArea alloc] initWithRect:[self bounds]
                                        options:options
@@ -310,7 +397,7 @@ BookmarkButton* gDraggedButton = nil; // Weak
   // Make an autoreleased |NSImage|, which will be returned, and draw into it.
   // By default, the |NSImage| will be completely transparent.
   NSImage* dragImage =
-  [[[NSImage alloc] initWithSize:[bitmap size]] autorelease];
+      [[[NSImage alloc] initWithSize:[bitmap size]] autorelease];
   [dragImage lockFocus];
 
   // Draw the image with the appropriate opacity, clipping it tightly.
diff --git a/chrome/browser/ui/cocoa/draggable_button.h b/chrome/browser/ui/cocoa/draggable_button.h
index ab74c2b..4e0d596 100644
--- a/chrome/browser/ui/cocoa/draggable_button.h
+++ b/chrome/browser/ui/cocoa/draggable_button.h
@@ -17,6 +17,13 @@
   NSTimeInterval whenMouseDown_;
 }
 
+@property NSTimeInterval durationMouseWasDown;
+
+@property NSTimeInterval whenMouseDown;
+
+// Whether the action has already fired for this click.
+@property(nonatomic) BOOL actionHasFired;
+
 // Enable or disable dragability for special buttons like "Other Bookmarks".
 @property(nonatomic) BOOL draggable;
 
@@ -31,11 +38,6 @@
 // -drag* methods of NSView when overriding this method.
 - (void)beginDrag:(NSEvent*)dragEvent;
 
-// Called internally. Default impl only returns YES if sender==self.
-// Override if your subclass wants to accept being tracked into while a
-// click is being tracked on another DraggableButton. Needed to support
-// buttons being used as fake menu items or menu titles, as BookmarkButton does.
-- (BOOL)acceptsTrackInFrom:(id)sender;
 
 // Override if you want to do any extra work on mouseUp, after a mouseDown
 // action has already fired.
@@ -65,8 +67,6 @@
                               yHysteresis:(float)yHysteresis;
 
 
-@property(nonatomic) NSTimeInterval durationMouseWasDown;
-
 @end  // @interface DraggableButton
 
 @interface DraggableButton (Private)
diff --git a/chrome/browser/ui/cocoa/draggable_button.mm b/chrome/browser/ui/cocoa/draggable_button.mm
index 654ae25..d5520e4 100644
--- a/chrome/browser/ui/cocoa/draggable_button.mm
+++ b/chrome/browser/ui/cocoa/draggable_button.mm
@@ -22,6 +22,9 @@ const CGFloat kDragExpirationTimeout = 1.0;
 @synthesize draggable = draggable_;
 @synthesize actsOnMouseDown = actsOnMouseDown_;
 @synthesize durationMouseWasDown = durationMouseWasDown_;
+@synthesize actionHasFired = actionHasFired_;
+@synthesize whenMouseDown = whenMouseDown_;
+
 
 - (id)initWithFrame:(NSRect)frame {
   if ((self = [super initWithFrame:frame])) {
@@ -153,62 +156,33 @@ const CGFloat kDragExpirationTimeout = 1.0;
   // action has already fired.
 }
 
-- (BOOL)acceptsTrackInFrom:(id)sender {
-  return (sender == self);
-}
-
 - (void)performMouseDownAction:(NSEvent*)theEvent {
-  int eventMask = NSLeftMouseUpMask | NSMouseEnteredMask | NSMouseExitedMask;
+  int eventMask = NSLeftMouseUpMask;
 
   [[self target] performSelector:[self action] withObject:self];
   actionHasFired_ = YES;
 
-  DraggableButton* insideBtn = nil;
-
   while (1) {
     theEvent = [[self window] nextEventMatchingMask:eventMask];
+    if (!theEvent)
+      continue;
     NSPoint mouseLoc = [self convertPoint:[theEvent locationInWindow]
                                  fromView:nil];
     BOOL isInside = [self mouse:mouseLoc inRect:[self bounds]];
+    [self highlight:isInside];
 
     switch ([theEvent type]) {
-      case NSMouseEntered:
-      case NSMouseExited: {
-        NSView* trackedView = (NSView*)[[theEvent trackingArea] owner];
-        if (trackedView && [trackedView isKindOfClass:[self class]]) {
-          DraggableButton *btn = static_cast<DraggableButton*>(trackedView);
-          if (![btn acceptsTrackInFrom:self])
-            break;
-          if ([theEvent type] == NSMouseEntered) {
-            [[NSCursor arrowCursor] set];
-            [[btn cell] mouseEntered:theEvent];
-            insideBtn = btn;
-          } else {
-            [[btn cell] mouseExited:theEvent];
-            if (insideBtn == btn)
-              insideBtn = nil;
-          }
-        }
-        break;
-      }
-      case NSLeftMouseUp: {
-        if (!isInside && insideBtn && insideBtn != self) {
-          // Has tracked onto another DraggableButton menu item, and released,
-          // so click it.
-          [insideBtn performClick:self];
-        }
+      case NSLeftMouseUp:
         durationMouseWasDown_ = [theEvent timestamp] - whenMouseDown_;
         [self secondaryMouseUpAction:isInside];
-        [[self cell] mouseExited:theEvent];
-        [[insideBtn cell] mouseExited:theEvent];
-        return;
         break;
-      }
       default:
         /* Ignore any other kind of event. */
         break;
     }
   }
+
+  [self highlight:NO];
 }
 
 // Mimic "begin a click" operation visually.  Do NOT follow through