Makes the chevron throb after a user sync'ed his bookmarks (if the overflow menu is shown)

This also makes the chevron throb when no bookmarks is added during a sync and merge. 

BUG=24306
TEST=sync with over 10 bookmarks in the bookmark bar so that the chevron is visible.

Patch by Bruno Calvignac<BrunoCalvignac@gmail.com>
Original Review: http://codereview.chromium.org/306054/show

Review URL: http://codereview.chromium.org/454010

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

3 files changed, 60 insertions, 46 deletions

diff --git a/chrome/browser/views/bookmark_bar_view.cc b/chrome/browser/views/bookmark_bar_view.cc
index 9bd479c..47a13be0 100644
--- a/chrome/browser/views/bookmark_bar_view.cc
+++ b/chrome/browser/views/bookmark_bar_view.cc
@@ -252,6 +252,25 @@ class BookmarkFolderButton : public views::MenuButton {
   DISALLOW_COPY_AND_ASSIGN(BookmarkFolderButton);
 };
 
+// OverFlowButton (chevron) --------------------------------------------------
+
+class OverFlowButton : public views::MenuButton {
+ public:
+  explicit OverFlowButton(BookmarkBarView* owner)
+      : MenuButton(NULL, std::wstring(), owner, false),
+        owner_(owner) {}
+
+  virtual bool OnMousePressed(const views::MouseEvent& e) {
+    owner_->StopThrobbing(true);
+    return views::MenuButton::OnMousePressed(e);
+  }
+
+ private:
+  BookmarkBarView* owner_;
+
+  DISALLOW_COPY_AND_ASSIGN(OverFlowButton);
+};
+
 }  // namespace
 
 // DropInfo -------------------------------------------------------------------
@@ -919,7 +938,7 @@ MenuButton* BookmarkBarView::CreateOtherBookmarkedButton() {
 }
 
 MenuButton* BookmarkBarView::CreateOverflowButton() {
-  MenuButton* button = new MenuButton(NULL, std::wstring(), this, false);
+  MenuButton* button = new OverFlowButton(this);
   button->SetIcon(*ResourceBundle::GetSharedInstance().
                   GetBitmapNamed(IDR_BOOKMARK_BAR_CHEVRONS));
 
@@ -976,18 +995,14 @@ void BookmarkBarView::BookmarkNodeMoved(BookmarkModel* model,
                                         int old_index,
                                         const BookmarkNode* new_parent,
                                         int new_index) {
-  StopThrobbing(true);
   BookmarkNodeRemovedImpl(model, old_parent, old_index);
   BookmarkNodeAddedImpl(model, new_parent, new_index);
-  StartThrobbing();
 }
 
 void BookmarkBarView::BookmarkNodeAdded(BookmarkModel* model,
                                         const BookmarkNode* parent,
                                         int index) {
-  StopThrobbing(true);
   BookmarkNodeAddedImpl(model, parent, index);
-  StartThrobbing();
 }
 
 void BookmarkBarView::BookmarkNodeAddedImpl(BookmarkModel* model,
@@ -999,7 +1014,11 @@ void BookmarkBarView::BookmarkNodeAddedImpl(BookmarkModel* model,
     return;
   }
   DCHECK(index >= 0 && index <= GetBookmarkButtonCount());
-  AddChildView(index, CreateBookmarkButton(parent->GetChild(index)));
+  const BookmarkNode* node = parent->GetChild(index);
+  if (!throbbing_view_ && sync_service_ && sync_service_->SetupInProgress()) {
+    StartThrobbing(node, true);
+  }
+  AddChildView(index, CreateBookmarkButton(node));
   UpdateColors();
   Layout();
   SchedulePaint();
@@ -1009,9 +1028,7 @@ void BookmarkBarView::BookmarkNodeRemoved(BookmarkModel* model,
                                           const BookmarkNode* parent,
                                           int old_index,
                                           const BookmarkNode* node) {
-  StopThrobbing(true);
   BookmarkNodeRemovedImpl(model, parent, old_index);
-  StartThrobbing();
 }
 
 void BookmarkBarView::BookmarkNodeRemovedImpl(BookmarkModel* model,
@@ -1291,15 +1308,17 @@ void BookmarkBarView::Observe(NotificationType type,
       }
       break;
 
-    case NotificationType::BOOKMARK_BUBBLE_SHOWN:
+    case NotificationType::BOOKMARK_BUBBLE_SHOWN: {
       StopThrobbing(true);
-      bubble_url_ = *(Details<GURL>(details).ptr());
-      StartThrobbing();
+      GURL url = *(Details<GURL>(details).ptr());
+      const BookmarkNode* node = model_->GetMostRecentlyAddedNodeForURL(url);
+      if (!node)
+        return;  // Generally shouldn't happen.
+      StartThrobbing(node, false);
       break;
-
+    }
     case NotificationType::BOOKMARK_BUBBLE_HIDDEN:
       StopThrobbing(false);
-      bubble_url_ = GURL();
       break;
 
     default:
@@ -1494,40 +1513,35 @@ int BookmarkBarView::GetFirstHiddenNodeIndex() {
   return bb_count;
 }
 
-void BookmarkBarView::StartThrobbing() {
+void BookmarkBarView::StartThrobbing(const BookmarkNode* node,
+                                     bool overflow_only) {
   DCHECK(!throbbing_view_);
 
-  if (bubble_url_.is_empty())
-    return;  // Bubble isn't showing; nothing to throb.
-
-  if (!GetWidget())
-    return;  // We're not showing, don't do anything.
-
-  const BookmarkNode* node =
-      model_->GetMostRecentlyAddedNodeForURL(bubble_url_);
-  if (!node)
-    return;  // Generally shouldn't happen.
-
-  // Determine which visible button is showing the url (or is an ancestor of
-  // the url).
-  if (node->HasAncestor(model_->GetBookmarkBarNode())) {
-    const BookmarkNode* bbn = model_->GetBookmarkBarNode();
-    const BookmarkNode* parent_on_bb = node;
-    while (parent_on_bb->GetParent() != bbn)
-      parent_on_bb = parent_on_bb->GetParent();
+  // Determine which visible button is showing the bookmark (or is an ancestor
+  // of the bookmark).
+  const BookmarkNode* bbn = model_->GetBookmarkBarNode();
+  const BookmarkNode* parent_on_bb = node;
+  while (parent_on_bb) {
+    const BookmarkNode* parent = parent_on_bb->GetParent();
+    if (parent == bbn)
+      break;
+    parent_on_bb = parent;
+  }
+  if (parent_on_bb) {
     int index = bbn->IndexOfChild(parent_on_bb);
     if (index >= GetFirstHiddenNodeIndex()) {
       // Node is hidden, animate the overflow button.
       throbbing_view_ = overflow_button_;
-    } else {
+    } else if (!overflow_only) {
       throbbing_view_ = static_cast<CustomButton*>(GetChildViewAt(index));
     }
-  } else {
+  } else if (!overflow_only) {
     throbbing_view_ = other_bookmarked_button_;
   }
 
   // Use a large number so that the button continues to throb.
-  throbbing_view_->StartThrobbing(std::numeric_limits<int>::max());
+  if (throbbing_view_)
+    throbbing_view_->StartThrobbing(std::numeric_limits<int>::max());
 }
 
 void BookmarkBarView::StopThrobbing(bool immediate) {
diff --git a/chrome/browser/views/bookmark_bar_view.h b/chrome/browser/views/bookmark_bar_view.h
index 92efeb8..a9cc397 100644
--- a/chrome/browser/views/bookmark_bar_view.h
+++ b/chrome/browser/views/bookmark_bar_view.h
@@ -210,6 +210,11 @@ class BookmarkBarView : public DetachableToolbarView,
   // Maximum size of buttons on the bookmark bar.
   static const int kMaxButtonWidth;
 
+  // If a button is currently throbbing, it is stopped. If immediate is true
+  // the throb stops immediately, otherwise it stops after a couple more
+  // throbs.
+  void StopThrobbing(bool immediate);
+
   // If true we're running tests. This short circuits a couple of animations.
   static bool testing_;
 
@@ -399,14 +404,11 @@ class BookmarkBarView : public DetachableToolbarView,
   // visible, this returns GetBookmarkButtonCount().
   int GetFirstHiddenNodeIndex();
 
-  // If the bookmark bubble is showing this determines which view should throb
-  // and starts it throbbing. Does nothing if bookmark bubble isn't showing.
-  void StartThrobbing();
-
-  // If a button is currently throbbing, it is stopped. If immediate is true
-  // the throb stops immediately, otherwise it stops after a couple more
-  // throbs.
-  void StopThrobbing(bool immediate);
+  // This determines which view should throb and starts it
+  // throbbing (e.g when the bookmark bubble is showing).
+  // If |overflow_only| is true, start throbbing only if |node| is hidden in
+  // the overflow menu.
+  void StartThrobbing(const BookmarkNode* node, bool overflow_only);
 
   // Updates the colors for all the child objects in the bookmarks bar.
   void UpdateColors();
@@ -478,9 +480,6 @@ class BookmarkBarView : public DetachableToolbarView,
   // Animation controlling showing and hiding of the bar.
   scoped_ptr<SlideAnimation> size_animation_;
 
-  // If the bookmark bubble is showing, this is the URL.
-  GURL bubble_url_;
-
   // If the bookmark bubble is showing, this is the visible ancestor of the URL.
   // The visible ancestor is either the other_bookmarked_button_,
   // overflow_button_ or a button on the bar.
diff --git a/chrome/browser/views/bookmark_bar_view_test.cc b/chrome/browser/views/bookmark_bar_view_test.cc
index 8fb2bda..8e24366 100644
--- a/chrome/browser/views/bookmark_bar_view_test.cc
+++ b/chrome/browser/views/bookmark_bar_view_test.cc
@@ -85,6 +85,7 @@ class BookmarkBarViewEventTestBase : public ViewEventTestBase {
     profile_->CreateBookmarkModel(true);
     profile_->BlockUntilBookmarkModelLoaded();
     profile_->GetPrefs()->SetBoolean(prefs::kShowBookmarkBar, true);
+    profile_->CreateProfileSyncService();
 
     model_ = profile_->GetBookmarkModel();
     model_->ClearStore();