Lands http://codereview.chromium.org/155428 for Thiago, which changes

from me to get the thing to work.

Merging two versions of BookmarkNodeRemoved into one.

BUG=None
TEST=Remove some bookmarks from BookmarkBar or Bookmarks Manager and make sure
everything works ok.

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

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

20 files changed, 57 insertions, 57 deletions

diff --git a/chrome/browser/bookmarks/base_bookmark_model_observer.h b/chrome/browser/bookmarks/base_bookmark_model_observer.h
index 045afb9..1242d18 100644
--- a/chrome/browser/bookmarks/base_bookmark_model_observer.h
+++ b/chrome/browser/bookmarks/base_bookmark_model_observer.h
@@ -35,7 +35,8 @@ class BaseBookmarkModelObserver : public BookmarkModelObserver {
   }
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index) {
+                                   int old_index,
+                                   const BookmarkNode* node) {
     BookmarkModelChanged();
   }
   virtual void BookmarkNodeChanged(BookmarkModel* model,
diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h
index c8ba083..4157089 100644
--- a/chrome/browser/bookmarks/bookmark_model.h
+++ b/chrome/browser/bookmarks/bookmark_model.h
@@ -181,16 +181,14 @@ class BookmarkModelObserver {
                                  int index) = 0;
 
   // Invoked when a node has been removed, the item may still be starred though.
-  // TODO(sky): merge these two into one.
-  virtual void BookmarkNodeRemoved(BookmarkModel* model,
-                                   const BookmarkNode* parent,
-                                   int index) {}
+  // |parent| the parent of the node that was removed.
+  // |old_index| the index of the removed node in |parent| before it was
+  // removed.
+  // |node| is the node that was removed.
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
                                    int old_index,
-                                   const BookmarkNode* node) {
-    BookmarkNodeRemoved(model, parent, old_index);
-  }
+                                   const BookmarkNode* node) = 0;
 
   // Invoked when the title or favicon of a node has changed.
   virtual void BookmarkNodeChanged(BookmarkModel* model,
diff --git a/chrome/browser/bookmarks/bookmark_model_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc
index 627a058..d0160e5 100644
--- a/chrome/browser/bookmarks/bookmark_model_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_model_unittest.cc
@@ -96,9 +96,10 @@ class BookmarkModelTest : public testing::Test, public BookmarkModelObserver {
 
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index) {
+                                   int old_index,
+                                   const BookmarkNode* node) {
     removed_count++;
-    observer_details.Set(parent, NULL, index, -1);
+    observer_details.Set(parent, NULL, old_index, -1);
   }
 
   virtual void BookmarkNodeChanged(BookmarkModel* model,
@@ -639,7 +640,8 @@ class BookmarkModelTestWithProfile : public testing::Test,
                                  int index) {}
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index) {}
+                                   int old_index,
+                                   const BookmarkNode* node) {}
   virtual void BookmarkNodeChanged(BookmarkModel* model,
                                    const BookmarkNode* node) {}
   virtual void BookmarkNodeChildrenReordered(BookmarkModel* model,
diff --git a/chrome/browser/cocoa/bookmark_bar_bridge.h b/chrome/browser/cocoa/bookmark_bar_bridge.h
index a6661cc..114197d 100644
--- a/chrome/browser/cocoa/bookmark_bar_bridge.h
+++ b/chrome/browser/cocoa/bookmark_bar_bridge.h
@@ -17,7 +17,6 @@ class Browser;
 @class BookmarkBarController;
 
 class BookmarkBarBridge : public BookmarkModelObserver {
-
  public:
   BookmarkBarBridge(BookmarkBarController* controller, BookmarkModel* model);
   virtual ~BookmarkBarBridge();
@@ -35,7 +34,8 @@ class BookmarkBarBridge : public BookmarkModelObserver {
                                  int index);
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index);
+                                   int old_index,
+                                   const BookmarkNode* node);
   virtual void BookmarkNodeChanged(BookmarkModel* model,
                                    const BookmarkNode* node);
   virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model,
@@ -50,4 +50,4 @@ class BookmarkBarBridge : public BookmarkModelObserver {
   DISALLOW_COPY_AND_ASSIGN(BookmarkBarBridge);
 };
 
-#endif // CHROME_BROWSER_COCOA_BOOKMARK_BAR_BRIDGE_H_
+#endif  // CHROME_BROWSER_COCOA_BOOKMARK_BAR_BRIDGE_H_
diff --git a/chrome/browser/cocoa/bookmark_bar_bridge.mm b/chrome/browser/cocoa/bookmark_bar_bridge.mm
index 4e13218..c28fe82 100644
--- a/chrome/browser/cocoa/bookmark_bar_bridge.mm
+++ b/chrome/browser/cocoa/bookmark_bar_bridge.mm
@@ -46,8 +46,9 @@ void BookmarkBarBridge::BookmarkNodeAdded(BookmarkModel* model,
 
 void BookmarkBarBridge::BookmarkNodeRemoved(BookmarkModel* model,
                                             const BookmarkNode* parent,
-                                            int index) {
-  [controller_ nodeRemoved:model parent:parent index:index];
+                                            int old_index,
+                                            const BookmarkNode* node) {
+  [controller_ nodeRemoved:model parent:parent index:old_index];
 }
 
 void BookmarkBarBridge::BookmarkNodeChanged(BookmarkModel* model,
diff --git a/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm b/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm
index 0a21e04..b32f165 100644
--- a/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_bridge_unittest.mm
@@ -131,7 +131,7 @@ TEST_F(BookmarkBarBridgeTest, TestRedirect) {
   bridge->BookmarkNodeChanged(NULL, NULL);
   bridge->BookmarkNodeFavIconLoaded(NULL, NULL);
   bridge->BookmarkNodeChildrenReordered(NULL, NULL);
-  bridge->BookmarkNodeRemoved(NULL, NULL, 0);
+  bridge->BookmarkNodeRemoved(NULL, NULL, 0, NULL);
 
   // 8 calls above plus an initial Loaded() in init routine makes 9
   EXPECT_TRUE([controller.get()->callbacks_ count] == 9);
diff --git a/chrome/browser/cocoa/bookmark_menu_bridge.h b/chrome/browser/cocoa/bookmark_menu_bridge.h
index 163514e..345c98a 100644
--- a/chrome/browser/cocoa/bookmark_menu_bridge.h
+++ b/chrome/browser/cocoa/bookmark_menu_bridge.h
@@ -44,6 +44,10 @@ class BookmarkMenuBridge : public BookmarkModelObserver,
   virtual void BookmarkNodeAdded(BookmarkModel* model,
                                  const BookmarkNode* parent,
                                  int index);
+  virtual void BookmarkNodeRemoved(BookmarkModel* model,
+                                   const BookmarkNode* parent,
+                                   int old_index,
+                                   const BookmarkNode* node);
   virtual void BookmarkNodeChanged(BookmarkModel* model,
                                    const BookmarkNode* node);
   virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model,
diff --git a/chrome/browser/cocoa/bookmark_menu_bridge.mm b/chrome/browser/cocoa/bookmark_menu_bridge.mm
index d13a509..32618d9 100644
--- a/chrome/browser/cocoa/bookmark_menu_bridge.mm
+++ b/chrome/browser/cocoa/bookmark_menu_bridge.mm
@@ -80,6 +80,14 @@ void BookmarkMenuBridge::BookmarkNodeAdded(BookmarkModel* model,
   Loaded(model);
 }
 
+void BookmarkMenuBridge::BookmarkNodeRemoved(BookmarkModel* model,
+                                             const BookmarkNode* parent,
+                                             int old_index,
+                                             const BookmarkNode* node) {
+  // TODO(jrg): this is brute force; perhaps we should be nicer.
+  Loaded(model);
+}
+
 void BookmarkMenuBridge::BookmarkNodeChanged(BookmarkModel* model,
                                              const BookmarkNode* node) {
   // TODO(jrg): this is brute force; perhaps we should be nicer.
diff --git a/chrome/browser/dom_ui/new_tab_ui.cc b/chrome/browser/dom_ui/new_tab_ui.cc
index a807e47..e66b557 100644
--- a/chrome/browser/dom_ui/new_tab_ui.cc
+++ b/chrome/browser/dom_ui/new_tab_ui.cc
@@ -1022,7 +1022,8 @@ class RecentlyBookmarkedHandler : public DOMMessageHandler,
                                  int index);
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index);
+                                   int old_index,
+                                   const BookmarkNode* node);
   virtual void BookmarkNodeChanged(BookmarkModel* model,
                                    const BookmarkNode* node);
 
@@ -1094,7 +1095,8 @@ void RecentlyBookmarkedHandler::BookmarkNodeAdded(BookmarkModel* model,
 
 void RecentlyBookmarkedHandler::BookmarkNodeRemoved(BookmarkModel* model,
                                                     const BookmarkNode* parent,
-                                                    int index) {
+                                                    int old_index,
+                                                    const BookmarkNode* node) {
   SendBookmarksToPage();
 }
 
diff --git a/chrome/browser/extensions/extension_bookmarks_module.cc b/chrome/browser/extensions/extension_bookmarks_module.cc
index 808050a..9f978ef 100644
--- a/chrome/browser/extensions/extension_bookmarks_module.cc
+++ b/chrome/browser/extensions/extension_bookmarks_module.cc
@@ -205,14 +205,6 @@ void ExtensionBookmarkEventRouter::BookmarkNodeAdded(BookmarkModel* model,
 void ExtensionBookmarkEventRouter::BookmarkNodeRemoved(
     BookmarkModel* model,
     const BookmarkNode* parent,
-    int index) {
-  // TODO(erikkay) can this version ever be called?
-  NOTREACHED();
-}
-
-void ExtensionBookmarkEventRouter::BookmarkNodeRemoved(
-    BookmarkModel* model,
-    const BookmarkNode* parent,
     int index,
     const BookmarkNode* node) {
   ListValue args;
diff --git a/chrome/browser/extensions/extension_bookmarks_module.h b/chrome/browser/extensions/extension_bookmarks_module.h
index 7cfc1ca2..853a0aa 100644
--- a/chrome/browser/extensions/extension_bookmarks_module.h
+++ b/chrome/browser/extensions/extension_bookmarks_module.h
@@ -36,9 +36,6 @@ class ExtensionBookmarkEventRouter : public BookmarkModelObserver {
                                  int index);
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index);
-  virtual void BookmarkNodeRemoved(BookmarkModel* model,
-                                   const BookmarkNode* parent,
                                    int old_index,
                                    const BookmarkNode* node);
   virtual void BookmarkNodeChanged(BookmarkModel* model,
diff --git a/chrome/browser/gtk/bookmark_bar_gtk.cc b/chrome/browser/gtk/bookmark_bar_gtk.cc
index fc9f4d8..c2753b9 100644
--- a/chrome/browser/gtk/bookmark_bar_gtk.cc
+++ b/chrome/browser/gtk/bookmark_bar_gtk.cc
@@ -237,7 +237,7 @@ void BookmarkBarGtk::BookmarkNodeMoved(BookmarkModel* model,
                                        int old_index,
                                        const BookmarkNode* new_parent,
                                        int new_index) {
-  BookmarkNodeRemoved(model, old_parent, old_index);
+  BookmarkNodeRemoved(model, old_parent, old_index, NULL);
   BookmarkNodeAdded(model, new_parent, new_index);
 }
 
@@ -259,15 +259,16 @@ void BookmarkBarGtk::BookmarkNodeAdded(BookmarkModel* model,
 
 void BookmarkBarGtk::BookmarkNodeRemoved(BookmarkModel* model,
                                          const BookmarkNode* parent,
-                                         int index) {
+                                         int old_index,
+                                         const BookmarkNode* node) {
   if (parent != model_->GetBookmarkBarNode()) {
     // We only care about nodes on the bookmark bar.
     return;
   }
-  DCHECK(index >= 0 && index < GetBookmarkButtonCount());
+  DCHECK(old_index >= 0 && old_index < GetBookmarkButtonCount());
 
   GtkWidget* to_remove = GTK_WIDGET(gtk_toolbar_get_nth_item(
-      GTK_TOOLBAR(bookmark_toolbar_.get()), index));
+      GTK_TOOLBAR(bookmark_toolbar_.get()), old_index));
   gtk_container_remove(GTK_CONTAINER(bookmark_toolbar_.get()),
                        to_remove);
 
diff --git a/chrome/browser/gtk/bookmark_bar_gtk.h b/chrome/browser/gtk/bookmark_bar_gtk.h
index 0c2aa39..0683275 100644
--- a/chrome/browser/gtk/bookmark_bar_gtk.h
+++ b/chrome/browser/gtk/bookmark_bar_gtk.h
@@ -111,7 +111,8 @@ class BookmarkBarGtk : public AnimationDelegate,
                                  int index);
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index);
+                                   int old_index,
+                                   const BookmarkNode* node);
   virtual void BookmarkNodeChanged(BookmarkModel* model,
                                    const BookmarkNode* node);
   // Invoked when a favicon has finished loading.
diff --git a/chrome/browser/gtk/bookmark_editor_gtk.h b/chrome/browser/gtk/bookmark_editor_gtk.h
index 70ed8c4..0b1fa28 100644
--- a/chrome/browser/gtk/bookmark_editor_gtk.h
+++ b/chrome/browser/gtk/bookmark_editor_gtk.h
@@ -52,7 +52,7 @@ class BookmarkEditorGtk : public BookmarkEditor,
                                  int index);
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index,
+                                   int old_index,
                                    const BookmarkNode* node);
   virtual void BookmarkNodeChanged(BookmarkModel* model,
                                    const BookmarkNode* node) {}
diff --git a/chrome/browser/gtk/bookmark_manager_gtk.cc b/chrome/browser/gtk/bookmark_manager_gtk.cc
index dab3508..51a2a40 100644
--- a/chrome/browser/gtk/bookmark_manager_gtk.cc
+++ b/chrome/browser/gtk/bookmark_manager_gtk.cc
@@ -212,12 +212,6 @@ void BookmarkManagerGtk::BookmarkNodeAdded(BookmarkModel* model,
 
 void BookmarkManagerGtk::BookmarkNodeRemoved(BookmarkModel* model,
                                              const BookmarkNode* parent,
-                                             int index) {
-  NOTREACHED();
-}
-
-void BookmarkManagerGtk::BookmarkNodeRemoved(BookmarkModel* model,
-                                             const BookmarkNode* parent,
                                              int old_index,
                                              const BookmarkNode* node) {
   if (node->is_folder()) {
diff --git a/chrome/browser/gtk/bookmark_manager_gtk.h b/chrome/browser/gtk/bookmark_manager_gtk.h
index ae3336b..402e454 100644
--- a/chrome/browser/gtk/bookmark_manager_gtk.h
+++ b/chrome/browser/gtk/bookmark_manager_gtk.h
@@ -48,9 +48,6 @@ class BookmarkManagerGtk : public BookmarkModelObserver,
                                  int index);
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index);
-  virtual void BookmarkNodeRemoved(BookmarkModel* model,
-                                   const BookmarkNode* parent,
                                    int old_index,
                                    const BookmarkNode* node);
   virtual void BookmarkNodeChanged(BookmarkModel* model,
diff --git a/chrome/browser/importer/importer.h b/chrome/browser/importer/importer.h
index 41ef11b..67f7cc4 100644
--- a/chrome/browser/importer/importer.h
+++ b/chrome/browser/importer/importer.h
@@ -181,7 +181,8 @@ class ImporterHost : public base::RefCounted<ImporterHost>,
                                  int index) {}
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index) {}
+                                   int old_index,
+                                   const BookmarkNode* node) {}
   virtual void BookmarkNodeChanged(BookmarkModel* model,
                                    const BookmarkNode* node) {}
   virtual void BookmarkNodeChildrenReordered(BookmarkModel* model,
diff --git a/chrome/browser/views/bookmark_bar_view.cc b/chrome/browser/views/bookmark_bar_view.cc
index 5cc5091..2e364e4 100644
--- a/chrome/browser/views/bookmark_bar_view.cc
+++ b/chrome/browser/views/bookmark_bar_view.cc
@@ -802,7 +802,8 @@ int BookmarkBarView::OnPerformDrop(const DropTargetEvent& event) {
   } else {
     parent_node = root;
   }
-  return bookmark_utils::PerformBookmarkDrop(profile_, data, parent_node, index);
+  return bookmark_utils::PerformBookmarkDrop(profile_, data, parent_node,
+                                            index);
 }
 
 void BookmarkBarView::OnFullscreenToggled(bool fullscreen) {
@@ -932,7 +933,7 @@ void BookmarkBarView::Loaded(BookmarkModel* model) {
   const BookmarkNode* node = model_->GetBookmarkBarNode();
   DCHECK(node && model_->other_node());
   // Create a button for each of the children on the bookmark bar.
-  for (int i = 0; i < node->GetChildCount(); ++i)
+  for (int i = 0, child_count = node->GetChildCount(); i < child_count; ++i)
     AddChildView(i, CreateBookmarkButton(node->GetChild(i)));
 
   // This button is normally created too early to get access to the theme
@@ -995,9 +996,10 @@ void BookmarkBarView::BookmarkNodeAddedImpl(BookmarkModel* model,
 
 void BookmarkBarView::BookmarkNodeRemoved(BookmarkModel* model,
                                           const BookmarkNode* parent,
-                                          int index) {
+                                          int old_index,
+                                          const BookmarkNode* node) {
   StopThrobbing(true);
-  BookmarkNodeRemovedImpl(model, parent, index);
+  BookmarkNodeRemovedImpl(model, parent, old_index);
   StartThrobbing();
 }
 
@@ -1061,7 +1063,7 @@ void BookmarkBarView::BookmarkNodeChildrenReordered(BookmarkModel* model,
   }
 
   // Create the new buttons.
-  for (int i = 0; i < node->GetChildCount(); ++i)
+  for (int i = 0, child_count = node->GetChildCount(); i < child_count; ++i)
     AddChildView(i, CreateBookmarkButton(node->GetChild(i)));
 
   Layout();
@@ -1357,8 +1359,9 @@ void BookmarkBarView::ShowDropFolderForNode(const BookmarkNode* node) {
   } else {
     // Make sure node is still valid.
     int index = -1;
+    const BookmarkNode* bb_node = model_->GetBookmarkBarNode();
     for (int i = 0; i < GetBookmarkButtonCount(); ++i) {
-      if (model_->GetBookmarkBarNode()->GetChild(i) == node) {
+      if (bb_node->GetChild(i) == node) {
         index = i;
         break;
       }
@@ -1512,7 +1515,7 @@ int BookmarkBarView::CalculateDropOperation(const DropTargetEvent& event,
         *is_over_other ? model_->other_node() :
                          model_->GetBookmarkBarNode()->GetChild(*index);
     int operation =
-        bookmark_utils::BookmarkDropOperation(profile_,event, data, parent,
+        bookmark_utils::BookmarkDropOperation(profile_, event, data, parent,
                                               parent->GetChildCount());
     if (!operation && !data.has_single_url() &&
         data.GetFirstNode(profile_) == parent) {
diff --git a/chrome/browser/views/bookmark_bar_view.h b/chrome/browser/views/bookmark_bar_view.h
index e53a2db..53fd699 100644
--- a/chrome/browser/views/bookmark_bar_view.h
+++ b/chrome/browser/views/bookmark_bar_view.h
@@ -255,7 +255,8 @@ class BookmarkBarView : public views::View,
   // is removed.
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index);
+                                   int old_index,
+                                   const BookmarkNode* node);
 
   // Implementation for BookmarkNodeRemoved.
   void BookmarkNodeRemovedImpl(BookmarkModel* model,
diff --git a/chrome/browser/views/bookmark_manager_view.h b/chrome/browser/views/bookmark_manager_view.h
index 6bf103b..c3d7346 100644
--- a/chrome/browser/views/bookmark_manager_view.h
+++ b/chrome/browser/views/bookmark_manager_view.h
@@ -121,9 +121,6 @@ class BookmarkManagerView : public views::View,
                                  int index) {}
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
                                    const BookmarkNode* parent,
-                                   int index) {}
-  virtual void BookmarkNodeRemoved(BookmarkModel* model,
-                                   const BookmarkNode* parent,
                                    int old_index,
                                    const BookmarkNode* node) {}
   virtual void BookmarkNodeChanged(BookmarkModel* model,