Set metainfo ID for enhanced bookmarks extension

This is a temporary workaround to allow for reliable creation of a
durable unique ID for the extension. It will go away once we are able to
provide control of all bookmark creation paths via the extension API.

BUG=383557

Review URL: https://codereview.chromium.org/338593002

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

3 files changed, 38 insertions, 20 deletions

diff --git a/components/bookmarks/browser/bookmark_model.cc b/components/bookmarks/browser/bookmark_model.cc
index 1ec1d36..d5c9bdd 100644
--- a/components/bookmarks/browser/bookmark_model.cc
+++ b/components/bookmarks/browser/bookmark_model.cc
@@ -877,6 +877,9 @@ void BookmarkModel::RemoveNodeAndGetRemovedUrls(BookmarkNode* node,
 BookmarkNode* BookmarkModel::AddNode(BookmarkNode* parent,
                                      int index,
                                      BookmarkNode* node) {
+  FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
+                    OnWillAddBookmarkNode(this, node));
+
   parent->Add(node, index);
 
   if (store_.get())
diff --git a/components/bookmarks/browser/bookmark_model_observer.h b/components/bookmarks/browser/bookmark_model_observer.h
index 21d0c34..d5c20a1 100644
--- a/components/bookmarks/browser/bookmark_model_observer.h
+++ b/components/bookmarks/browser/bookmark_model_observer.h
@@ -29,6 +29,12 @@ class BookmarkModelObserver {
                                  const BookmarkNode* new_parent,
                                  int new_index) = 0;
 
+  // Invoked prior to adding a bookmark node, and in particular, prior to adding
+  // it to the parent. This function can be used to alter the contents of the
+  // node before BookmarkNodeAdded listeners know about it.
+  virtual void OnWillAddBookmarkNode(BookmarkModel* model,
+                                     BookmarkNode* node) {}
+
   // Invoked when a node has been added.
   virtual void BookmarkNodeAdded(BookmarkModel* model,
                                  const BookmarkNode* parent,
diff --git a/components/bookmarks/browser/bookmark_model_unittest.cc b/components/bookmarks/browser/bookmark_model_unittest.cc
index 884a4b4..b198ae4 100644
--- a/components/bookmarks/browser/bookmark_model_unittest.cc
+++ b/components/bookmarks/browser/bookmark_model_unittest.cc
@@ -158,6 +158,12 @@ class BookmarkModelTest : public testing::Test,
     observer_details_.Set(old_parent, new_parent, old_index, new_index);
   }
 
+  virtual void OnWillAddBookmarkNode(BookmarkModel* model,
+                                     BookmarkNode* node) OVERRIDE {
+    ++will_add_count_;
+    EXPECT_TRUE(node->parent() == NULL);
+  }
+
   virtual void BookmarkNodeAdded(BookmarkModel* model,
                                  const BookmarkNode* parent,
                                  int index) OVERRIDE {
@@ -230,14 +236,15 @@ class BookmarkModelTest : public testing::Test,
   }
 
   void ClearCounts() {
-    added_count_ = moved_count_ = removed_count_ = changed_count_ =
-        reordered_count_ = extensive_changes_beginning_count_ =
+    will_add_count_ = added_count_ = moved_count_ = removed_count_ =
+        changed_count_ = reordered_count_ = extensive_changes_beginning_count_ =
         extensive_changes_ended_count_ = all_bookmarks_removed_ =
         before_remove_count_ = before_change_count_ = before_reorder_count_ =
         before_remove_all_count_ = 0;
   }
 
-  void AssertObserverCount(int added_count,
+  void AssertObserverCount(int will_add_count,
+                           int added_count,
                            int moved_count,
                            int removed_count,
                            int changed_count,
@@ -246,6 +253,7 @@ class BookmarkModelTest : public testing::Test,
                            int before_change_count,
                            int before_reorder_count,
                            int before_remove_all_count) {
+    EXPECT_EQ(will_add_count_, will_add_count);
     EXPECT_EQ(added_count_, added_count);
     EXPECT_EQ(moved_count_, moved_count);
     EXPECT_EQ(removed_count_, removed_count);
@@ -290,6 +298,7 @@ class BookmarkModelTest : public testing::Test,
   ObserverDetails observer_details_;
 
  private:
+  int will_add_count_;
   int added_count_;
   int moved_count_;
   int removed_count_;
@@ -333,7 +342,7 @@ TEST_F(BookmarkModelTest, AddURL) {
   const GURL url("http://foo.com");
 
   const BookmarkNode* new_node = model_->AddURL(root, 0, title, url);
-  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   ASSERT_EQ(1, root->child_count());
@@ -354,7 +363,7 @@ TEST_F(BookmarkModelTest, AddURLWithUnicodeTitle) {
   const GURL url("https://www.baidu.com/");
 
   const BookmarkNode* new_node = model_->AddURL(root, 0, title, url);
-  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   ASSERT_EQ(1, root->child_count());
@@ -395,7 +404,7 @@ TEST_F(BookmarkModelTest, AddURLWithCreationTimeAndMetaInfo) {
 
   const BookmarkNode* new_node = model_->AddURLWithCreationTimeAndMetaInfo(
       root, 0, title, url, time, &meta_info);
-  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   ASSERT_EQ(1, root->child_count());
@@ -418,7 +427,7 @@ TEST_F(BookmarkModelTest, AddURLToMobileBookmarks) {
   const GURL url("http://foo.com");
 
   const BookmarkNode* new_node = model_->AddURL(root, 0, title, url);
-  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   ASSERT_EQ(1, root->child_count());
@@ -437,7 +446,7 @@ TEST_F(BookmarkModelTest, AddFolder) {
   const base::string16 title(ASCIIToUTF16("foo"));
 
   const BookmarkNode* new_node = model_->AddFolder(root, 0, title);
-  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   ASSERT_EQ(1, root->child_count());
@@ -451,7 +460,7 @@ TEST_F(BookmarkModelTest, AddFolder) {
   // Add another folder, just to make sure folder_ids are incremented correctly.
   ClearCounts();
   model_->AddFolder(root, 0, title);
-  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 }
 
@@ -480,7 +489,7 @@ TEST_F(BookmarkModelTest, RemoveURL) {
 
   model_->Remove(root, 0);
   ASSERT_EQ(0, root->child_count());
-  AssertObserverCount(0, 0, 1, 0, 0, 1, 0, 0, 0);
+  AssertObserverCount(0, 0, 0, 1, 0, 0, 1, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   // Make sure there is no mapping for the URL.
@@ -503,7 +512,7 @@ TEST_F(BookmarkModelTest, RemoveFolder) {
   // Now remove the folder.
   model_->Remove(root, 0);
   ASSERT_EQ(0, root->child_count());
-  AssertObserverCount(0, 0, 1, 0, 0, 1, 0, 0, 0);
+  AssertObserverCount(0, 0, 0, 1, 0, 0, 1, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   // Make sure there is no mapping for the URL.
@@ -524,7 +533,7 @@ TEST_F(BookmarkModelTest, RemoveAllUserBookmarks) {
   const BookmarkNode* folder = model_->AddFolder(bookmark_bar_node, 0, title);
   model_->AddURL(folder, 0, title, url);
 
-  AssertObserverCount(3, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(3, 3, 0, 0, 0, 0, 0, 0, 0, 0);
   ClearCounts();
 
   model_->RemoveAllUserBookmarks();
@@ -532,7 +541,7 @@ TEST_F(BookmarkModelTest, RemoveAllUserBookmarks) {
   EXPECT_EQ(0, bookmark_bar_node->child_count());
   // No individual BookmarkNodeRemoved events are fired, so removed count
   // should be 0.
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 1);
+  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 0, 1);
   AssertExtensiveChangesObserverCount(1, 1);
   EXPECT_EQ(1, AllNodesRemovedObserverCount());
 }
@@ -547,7 +556,7 @@ TEST_F(BookmarkModelTest, SetTitle) {
 
   title = ASCIIToUTF16("foo2");
   model_->SetTitle(node, title);
-  AssertObserverCount(0, 0, 0, 1, 0, 0, 1, 0, 0);
+  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 1, 0, 0);
   observer_details_.ExpectEquals(node, NULL, -1, -1);
   EXPECT_EQ(title, node->GetTitle());
 }
@@ -576,7 +585,7 @@ TEST_F(BookmarkModelTest, SetURL) {
 
   url = GURL("http://foo2.com");
   model_->SetURL(node, url);
-  AssertObserverCount(0, 0, 0, 1, 0, 0, 1, 0, 0);
+  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 1, 0, 0);
   observer_details_.ExpectEquals(node, NULL, -1, -1);
   EXPECT_EQ(url, node->url());
 }
@@ -591,7 +600,7 @@ TEST_F(BookmarkModelTest, SetDateAdded) {
 
   base::Time new_time = base::Time::Now() + base::TimeDelta::FromMinutes(20);
   model_->SetDateAdded(node, new_time);
-  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
   EXPECT_EQ(new_time, node->date_added());
   EXPECT_EQ(new_time, model_->bookmark_bar_node()->date_folder_modified());
 }
@@ -606,7 +615,7 @@ TEST_F(BookmarkModelTest, Move) {
 
   model_->Move(node, folder1, 0);
 
-  AssertObserverCount(0, 1, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(0, 0, 1, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, folder1, 1, 0);
   EXPECT_TRUE(folder1 == node->parent());
   EXPECT_EQ(1, root->child_count());
@@ -617,7 +626,7 @@ TEST_F(BookmarkModelTest, Move) {
   // And remove the folder.
   ClearCounts();
   model_->Remove(root, 0);
-  AssertObserverCount(0, 0, 1, 0, 0, 1, 0, 0, 0);
+  AssertObserverCount(0, 0, 0, 1, 0, 0, 1, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
   EXPECT_TRUE(model_->GetMostRecentlyAddedUserNodeForURL(url) == NULL);
   EXPECT_EQ(0, root->child_count());
@@ -1011,7 +1020,7 @@ TEST_F(BookmarkModelTest, Sort) {
   model_->SortChildren(parent);
 
   // Make sure we were notified.
-  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 1, 0);
+  AssertObserverCount(0, 0, 0, 0, 0, 1, 0, 0, 1, 0);
 
   // Make sure the order matches (remember, 'a' and 'C' are folders and
   // come first).
@@ -1039,7 +1048,7 @@ TEST_F(BookmarkModelTest, Reorder) {
   model_->ReorderChildren(parent, new_order);
 
   // Make sure we were notified.
-  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 1, 0);
+  AssertObserverCount(0, 0, 0, 0, 0, 1, 0, 0, 1, 0);
 
   // Make sure the order matches is correct (it should be reversed).
   ASSERT_EQ(4, parent->child_count());