Mac: Fix lifetime problem under BookmarkBarController stopPulsingBookmarkNode.

BookmarkBarController has a weak pointer in a |pulsingButton_| data
member. This is an NSView which can be destroyed in
-[BookmarkBarController redistributeButtonsOnBarAsNeeded] when it is
removed from its superview.

Since bubbles close asynchronously, and fade out, the bookmark bubble
can cause a UAF to |pulsingButton_| via stopPulsingBookmarkNode. This
can occur if a resize triggers redistributeButtonsOnBarAsNeeded while a
bookmark bubble is still alive.

There's no need for this to be a weak pointer. Make it scoped_nsobject
instead.

This affects both Cocoa and MacViews.

BUG=616051
TEST=Added unit_test:
BookmarkBarControllerTest.RedistributeButtonsOnBarAsNeeded

Review-Url: https://codereview.chromium.org/2567973002
Cr-Commit-Position: refs/heads/master@{#437954}

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

@@ -294,10 +294,10 @@ willAnimateFromState:(BookmarkBar::State)oldState
   base::scoped_nsobject<BookmarkContextMenuCocoaController>
       contextMenuController_;
 
-  // Weak pointer to the pulsed button for the currently pulsing node. We need
-  // to store this as it may not be possible to determine the pulsing button if
-  // the pulsing node is deleted. Nil if there is no pulsing node.
-  BookmarkButton* pulsingButton_;
+  // The pulsed button for the currently pulsing node. We need to store this as
+  // it may not be possible to determine the pulsing button if the pulsing node
+  // is deleted. Nil if there is no pulsing node.
+  base::scoped_nsobject<BookmarkButton> pulsingButton_;
 
   // Specifically watch the currently pulsing node. This lets us stop pulsing
   // when anything happens to the node. Null if there is no pulsing node.

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

@@ -361,7 +361,8 @@ void RecordAppLaunch(Profile* profile, GURL url) {
 - (void)startPulsingBookmarkNode:(const BookmarkNode*)node {
   [self stopPulsingBookmarkNode];
 
-  pulsingButton_ = [self bookmarkButtonToPulseForNode:node];
+  pulsingButton_.reset([self bookmarkButtonToPulseForNode:node],
+                       base::scoped_policy::RETAIN);
   if (!pulsingButton_)
     return;
 
@@ -379,7 +380,7 @@ void RecordAppLaunch(Profile* profile, GURL url) {
     return;
 
   [pulsingButton_ setPulseIsStuckOn:NO];
-  pulsingButton_ = nil;
+  pulsingButton_.reset();
   pulsingBookmarkObserver_.reset();
 }
 

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

@@ -1568,6 +1568,46 @@ TEST_F(BookmarkBarControllerTest, ShrinkOrHideView) {
   EXPECT_TRUE([view isHidden]);
 }
 
+// Simulate coarse browser window width change and ensure that the bookmark
+// buttons that should be visible are visible.
+TEST_F(BookmarkBarControllerTest, RedistributeButtonsOnBarAsNeeded) {
+  // Hide the apps shortcut.
+  profile()->GetPrefs()->SetBoolean(
+      bookmarks::prefs::kShowAppsShortcutInBookmarkBar, false);
+  ASSERT_TRUE([bar_ appsPageShortcutButtonIsHidden]);
+
+  // Add three buttons to the bookmark bar.
+  BookmarkModel* model = BookmarkModelFactory::GetForBrowserContext(profile());
+  const BookmarkNode* root = model->bookmark_bar_node();
+  // Make long labels to test coarse resizes. After 16 digits, text eliding
+  // starts.
+  const std::string model_string(
+      "0000000000000000 1111111111111111 2222222222222222 ");
+  bookmarks::test::AddNodesFromModelString(model, root, model_string);
+  NSRect frame = [[bar_ view] frame];
+  frame.size.width = 400;  // Typical minimum browser size.
+  [[bar_ view] setFrame:frame];
+  EXPECT_EQ(2, [bar_ displayedButtonCount]);
+
+  {
+    base::mac::ScopedNSAutoreleasePool pool;
+    frame.size.width = 800;
+    [[bar_ view] setFrame:frame];
+    EXPECT_EQ(3, [bar_ displayedButtonCount]);
+
+    const BookmarkNode* last = model->bookmark_bar_node()->GetChild(2);
+    EXPECT_TRUE(last);
+    [bar_ startPulsingBookmarkNode:last];
+
+    frame.size.width = 400;
+    [[bar_ view] setFrame:frame];
+    EXPECT_EQ(2, [bar_ displayedButtonCount]);
+  }
+
+  // Regression test for http://crbug.com/616051.
+  [bar_ stopPulsingBookmarkNode];
+}
+
 // Simiulate browser window width change and ensure that the bookmark buttons
 // that should be visible are visible.
 // Appears to fail on Mac 10.11 bot on the waterfall; http://crbug.com/612640.