Cocoa: Mark the bookmark draggable button action as fired _before_ performing the action.

Opening an entire bookmark folder with more than 20 bookmarks in it
from the bookmarks bar using Cmd+Click on a Cocoa browser is currently
broken. This happens in the bookmark draggable button's action.

The action may run a nested loop, which may see the mouse "up" event and
cause the action to be performed again, inside the nested loop. We also
need to ensure the bookmark button doesn't enter its _own_ nested run
loop, since the mouse up has already been eaten by the nested run loop
inside the action.

Failure to do so causes the action to happen twice, which results in the
nested run loop of the action spawning another, nested, run loop within
itself, which never exits. This puts the browser into a very weird state.

TEST=On Mac (chrome://flags/#views-browser-windows DISABLED), have a folder
     on the bookmarks bar with >20 items and Cmd+Click it. Accept the dialog.
     Bookmarks should open.

Bug: 849135, 840387
Change-Id: I7e1eaf57dff956647cbdcf2cb449bb971cada074
Reviewed-on: https://chromium-review.googlesource.com/1090513
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565463}

chrome/browser/ui/cocoa/bookmarks/bookmark_button.h

@@ -216,6 +216,7 @@ class BookmarkNode;
   NSPoint dragEndScreenLocation_;
   BOOL dragPending_;
   BOOL acceptsTrackIn_;
+  BOOL sawMouseUp_;
   NSTrackingArea* area_;
   NSColor* backgroundColor_;
 }

chrome/browser/ui/cocoa/bookmarks/bookmark_button.mm

@@ -281,11 +281,15 @@ BookmarkButton* gDraggedButton = nil; // Weak
   int eventMask = NSLeftMouseUpMask | NSMouseEnteredMask | NSMouseExitedMask |
       NSLeftMouseDraggedMask;
 
-  BOOL keepGoing = YES;
-  [[self target] performSelector:[self action] withObject:self];
+  // The action may run a nested loop, so ensure the action is marked as fired
+  // before performing the action, and avoid spinning our own nested loop here
+  // if it would have already exited due to mouse up.
   self.draggableButton.actionHasFired = YES;
+  sawMouseUp_ = NO;
+  [[self target] performSelector:[self action] withObject:self];
 
   DraggableButton* insideBtn = nil;
+  BOOL keepGoing = !sawMouseUp_;
 
   while (keepGoing) {
     theEvent = [[self window] nextEventMatchingMask:eventMask];
@@ -370,6 +374,7 @@ BookmarkButton* gDraggedButton = nil; // Weak
 }
 
 - (void)mouseUp:(NSEvent*)theEvent {
+  sawMouseUp_ = YES;
   [super mouseUp:theEvent];
 
   // Update the highlight on mouse up.

chrome/browser/ui/cocoa/bookmarks/bookmark_button_unittest.mm

@@ -24,6 +24,7 @@ using bookmarks::BookmarkNode;
   int exited_;
   BOOL canDragToTrash_;
   int didDragToTrashCount_;
+  int actionCounter_;
 }
 @end
 
@@ -60,6 +61,14 @@ using bookmarks::BookmarkNode;
 - (void)bookmarkDragDidEnd:(BookmarkButton*)button
                  operation:(NSDragOperation)operation {
 }
+
+- (void)sinkRunLoopAction:(id)sender {
+  ++actionCounter_;
+  // Interacting with the real message loop in a unit test is hard, so emulate a
+  // mouseUp: being fired via [NSApp sendEvent:].
+  [sender mouseUp:cocoa_test_event_utils::MouseEventAtPoint(
+                      NSMakePoint(10, 10), NSLeftMouseDown, 0)];
+}
 @end
 
 namespace {
@@ -133,6 +142,26 @@ TEST_F(BookmarkButtonTest, MouseEnterExitRedirect) {
   EXPECT_EQ(2, delegate.get()->exited_);
 }
 
+// Tests a button whose action may process the mouseUp. Regression test for
+// https://crbug.com/849135.
+TEST_F(BookmarkButtonTest, NestedRunLoop) {
+  base::scoped_nsobject<BookmarkButton> button;
+  base::scoped_nsobject<BookmarkButtonCell> cell;
+  base::scoped_nsobject<FakeButtonDelegate> delegate(
+      [[FakeButtonDelegate alloc] init]);
+  button.reset(
+      [[BookmarkButton alloc] initWithFrame:NSMakeRect(0, 0, 500, 500)]);
+  cell.reset([[BookmarkButtonCell alloc] initTextCell:@"hi mum"]);
+  [button setCell:cell];
+  [button setDelegate:delegate];
+  [button setAction:@selector(sinkRunLoopAction:)];
+  [button setTarget:delegate];
+  [[button draggableButton] setActsOnMouseDown:YES];
+  [button mouseDown:cocoa_test_event_utils::LeftMouseDownAtPoint(
+                        NSMakePoint(10, 10))];
+  EXPECT_EQ(1, delegate.get()->actionCounter_);
+}
+
 // Tests to see if the bookmark button is properly highlighted on mouse up/down
 // events.
 TEST_F(BookmarkButtonTest, MouseUpAndDownHighlight) {