[gtk] correct bookmark bar new item pulsation.

This should avoid the read after free because if the item is destroyed, the signal handler will be disconnected and the StartThrobbing call will be canceled. It will fix bug 55895 because it will make sure StartThrobbing is called after the new item's allocation.

BUG=55895,69928
TEST=manual

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

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

2 files changed, 27 insertions, 5 deletions

diff --git a/chrome/browser/ui/gtk/bookmark_bar_gtk.cc b/chrome/browser/ui/gtk/bookmark_bar_gtk.cc
index 6e76c1e..4a59881 100644
--- a/chrome/browser/ui/gtk/bookmark_bar_gtk.cc
+++ b/chrome/browser/ui/gtk/bookmark_bar_gtk.cc
@@ -438,9 +438,7 @@ void BookmarkBarGtk::BookmarkNodeAdded(BookmarkModel* model,
   SetInstructionState();
   SetChevronState();
 
-  MessageLoop::current()->PostTask(FROM_HERE,
-      method_factory_.NewRunnableMethod(
-          &BookmarkBarGtk::StartThrobbing, node));
+  StartThrobbingAfterAllocation(GTK_WIDGET(item));
 }
 
 void BookmarkBarGtk::BookmarkNodeRemoved(BookmarkModel* model,
@@ -730,8 +728,6 @@ void BookmarkBarGtk::StartThrobbing(const BookmarkNode* node) {
     if (hidden >= 0 && hidden <= idx) {
       widget_to_throb = overflow_button_;
     } else {
-      if (parent_on_bb->is_url())
-        return;
       widget_to_throb = gtk_bin_get_child(GTK_BIN(gtk_toolbar_get_nth_item(
           GTK_TOOLBAR(bookmark_toolbar_.get()), idx)));
     }
@@ -768,6 +764,23 @@ void BookmarkBarGtk::SetThrobbingWidget(GtkWidget* widget) {
   }
 }
 
+void BookmarkBarGtk::OnItemAllocate(GtkWidget* item,
+                                    GtkAllocation* allocation) {
+  // We only want to fire on the item's first allocation.
+  g_signal_handlers_disconnect_by_func(
+      item, reinterpret_cast<gpointer>(&OnItemAllocateThunk), this);
+
+  GtkWidget* button = gtk_bin_get_child(GTK_BIN(item));
+  const BookmarkNode* node = GetNodeForToolButton(button);
+  if (node)
+    StartThrobbing(node);
+}
+
+void BookmarkBarGtk::StartThrobbingAfterAllocation(GtkWidget* item) {
+  g_signal_connect_after(
+      item, "size-allocate", G_CALLBACK(OnItemAllocateThunk), this);
+}
+
 bool BookmarkBarGtk::IsAlwaysShown() {
   return profile_->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar);
 }
diff --git a/chrome/browser/ui/gtk/bookmark_bar_gtk.h b/chrome/browser/ui/gtk/bookmark_bar_gtk.h
index 6e2261f..a898b58 100644
--- a/chrome/browser/ui/gtk/bookmark_bar_gtk.h
+++ b/chrome/browser/ui/gtk/bookmark_bar_gtk.h
@@ -170,6 +170,15 @@ class BookmarkBarGtk : public ui::AnimationDelegate,
   // condition that can happen during testing.
   bool GetTabContentsSize(gfx::Size* size);
 
+  // Connects to the "size-allocate" signal on the given widget, and causes it
+  // to throb after allocation. This is called when a new item is added to the
+  // bar. We can't call StartThrobbing directly because we don't know if it's
+  // visible or not until after the widget is allocated.
+  void StartThrobbingAfterAllocation(GtkWidget* item);
+
+  // Used by StartThrobbingAfterAllocation.
+  CHROMEGTK_CALLBACK_1(BookmarkBarGtk, void, OnItemAllocate, GtkAllocation*);
+
   // Makes the appropriate widget on the bookmark bar stop throbbing
   // (a folder, the overflow chevron, or nothing).
   void StartThrobbing(const BookmarkNode* node);