Don't set metainfo ID for enhanced bookmarks extension

The enhanced bookmarks extension is being updated to rely solely on
server-generated persistent IDs, eliminating the need to create IDs
in Chrome client code.

This is effectively a revert of 0f7d5081
and 224e1d18.

BUG=413378

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

Cr-Commit-Position: refs/heads/master@{#294694}

chrome/browser/extensions/api/bookmarks/bookmarks_api.cc

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <algorithm>
-
 #include "chrome/browser/extensions/api/bookmarks/bookmarks_api.h"
 
 #include "base/bind.h"
@@ -14,7 +12,6 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
 #include "base/prefs/pref_service.h"
-#include "base/rand_util.h"
 #include "base/sha1.h"
 #include "base/stl_util.h"
 #include "base/strings/string16.h"
@@ -47,9 +44,7 @@
 #include "content/public/browser/web_contents.h"
 #include "extensions/browser/event_router.h"
 #include "extensions/browser/extension_function_dispatcher.h"
-#include "extensions/browser/extension_registry.h"
 #include "extensions/browser/notification_types.h"
-#include "extensions/common/permissions/permissions_data.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if defined(OS_WIN)
@@ -92,56 +87,6 @@ base::FilePath GetDefaultFilepathForBookmarkExport() {
   return default_path.Append(filename);
 }
 
-bool IsEnhancedBookmarksExtensionActive(Profile* profile) {
-  static const char *enhanced_extension_hashes[] = {
-    "D5736E4B5CF695CB93A2FB57E4FDC6E5AFAB6FE2",  // http://crbug.com/312900
-    "D57DE394F36DC1C3220E7604C575D29C51A6C495",  // http://crbug.com/319444
-    "3F65507A3B39259B38C8173C6FFA3D12DF64CCE9"   // http://crbug.com/371562
-  };
-  const ExtensionSet& extensions =
-      ExtensionRegistry::Get(profile)->enabled_extensions();
-  for (ExtensionSet::const_iterator it = extensions.begin();
-       it != extensions.end(); ++it) {
-    const Extension* extension = it->get();
-    if (extension->permissions_data()->HasAPIPermission(
-            APIPermission::kBookmarkManagerPrivate)) {
-      std::string hash = base::SHA1HashString(extension->id());
-      hash = base::HexEncode(hash.c_str(), hash.length());
-      for (size_t i = 0; i < arraysize(enhanced_extension_hashes); i++)
-        if (hash == enhanced_extension_hashes[i])
-          return true;
-    }
-  }
-  return false;
-}
-
-std::string ToBase36(int64 value) {
-  DCHECK(value >= 0);
-  std::string str;
-  while (value > 0) {
-    int digit = value % 36;
-    value /= 36;
-    str += (digit < 10 ? '0' + digit : 'a' + digit - 10);
-  }
-  std::reverse(str.begin(), str.end());
-  return str;
-}
-
-// Generate a metadata ID based on a the current time and a random number for
-// enhanced bookmarks, to be assigned pre-sync.
-std::string GenerateEnhancedBookmarksID(bool is_folder) {
-  static const char bookmark_prefix[] = "cc_";
-  static const char folder_prefix[] = "cf_";
-  // Use [0..range_mid) for bookmarks, [range_mid..2*range_mid) for folders.
-  int range_mid = 36*36*36*36 / 2;
-  int rand = base::RandInt(0, range_mid - 1);
-  int64 unix_epoch_time_in_ms =
-      (base::Time::Now() - base::Time::UnixEpoch()).InMilliseconds();
-  return std::string(is_folder ? folder_prefix : bookmark_prefix) +
-      ToBase36(is_folder ? range_mid + rand : rand) +
-      ToBase36(unix_epoch_time_in_ms);
-}
-
 }  // namespace
 
 bool BookmarksFunction::RunAsync() {
@@ -338,20 +283,6 @@ void BookmarkEventRouter::BookmarkNodeMoved(BookmarkModel* model,
       bookmarks::OnMoved::Create(base::Int64ToString(node->id()), move_info));
 }
 
-void BookmarkEventRouter::OnWillAddBookmarkNode(BookmarkModel* model,
-                                                BookmarkNode* node) {
-  // TODO(wittman): Remove this once extension hooks are in place to allow the
-  // enhanced bookmarks extension to manage all bookmark creation code
-  // paths. See http://crbug.com/383557.
-  if (IsEnhancedBookmarksExtensionActive(Profile::FromBrowserContext(
-          browser_context_))) {
-    static const char key[] = "stars.id";
-    std::string value;
-    if (!node->GetMetaInfo(key, &value))
-      node->SetMetaInfo(key, GenerateEnhancedBookmarksID(node->is_folder()));
-  }
-}
-
 void BookmarkEventRouter::BookmarkNodeAdded(BookmarkModel* model,
                                             const BookmarkNode* parent,
                                             int index) {

chrome/browser/extensions/api/bookmarks/bookmarks_api.h

@@ -53,8 +53,6 @@ class BookmarkEventRouter : public BookmarkModelObserver {
                                  int old_index,
                                  const BookmarkNode* new_parent,
                                  int new_index) OVERRIDE;
-  virtual void OnWillAddBookmarkNode(BookmarkModel* model,
-                                     BookmarkNode* node) OVERRIDE;
   virtual void BookmarkNodeAdded(BookmarkModel* model,
                                  const BookmarkNode* parent,
                                  int index) OVERRIDE;

components/bookmarks/browser/bookmark_model.cc

@@ -878,9 +878,6 @@ 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())

components/bookmarks/browser/bookmark_model_observer.h

@@ -29,12 +29,6 @@ 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,

components/bookmarks/browser/bookmark_model_unittest.cc

@@ -159,12 +159,6 @@ 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 {
@@ -237,15 +231,14 @@ class BookmarkModelTest : public testing::Test,
   }
 
   void ClearCounts() {
-    will_add_count_ = added_count_ = moved_count_ = removed_count_ =
-        changed_count_ = reordered_count_ = extensive_changes_beginning_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 will_add_count,
-                           int added_count,
+  void AssertObserverCount(int added_count,
                            int moved_count,
                            int removed_count,
                            int changed_count,
@@ -254,7 +247,6 @@ 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);
@@ -299,7 +291,6 @@ class BookmarkModelTest : public testing::Test,
   ObserverDetails observer_details_;
 
  private:
-  int will_add_count_;
   int added_count_;
   int moved_count_;
   int removed_count_;
@@ -343,7 +334,7 @@ TEST_F(BookmarkModelTest, AddURL) {
   const GURL url("http://foo.com");
 
   const BookmarkNode* new_node = model_->AddURL(root, 0, title, url);
-  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   ASSERT_EQ(1, root->child_count());
@@ -364,7 +355,7 @@ TEST_F(BookmarkModelTest, AddURLWithUnicodeTitle) {
   const GURL url("https://www.baidu.com/");
 
   const BookmarkNode* new_node = model_->AddURL(root, 0, title, url);
-  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   ASSERT_EQ(1, root->child_count());
@@ -405,7 +396,7 @@ TEST_F(BookmarkModelTest, AddURLWithCreationTimeAndMetaInfo) {
 
   const BookmarkNode* new_node = model_->AddURLWithCreationTimeAndMetaInfo(
       root, 0, title, url, time, &meta_info);
-  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   ASSERT_EQ(1, root->child_count());
@@ -428,7 +419,7 @@ TEST_F(BookmarkModelTest, AddURLToMobileBookmarks) {
   const GURL url("http://foo.com");
 
   const BookmarkNode* new_node = model_->AddURL(root, 0, title, url);
-  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   ASSERT_EQ(1, root->child_count());
@@ -447,7 +438,7 @@ TEST_F(BookmarkModelTest, AddFolder) {
   const base::string16 title(ASCIIToUTF16("foo"));
 
   const BookmarkNode* new_node = model_->AddFolder(root, 0, title);
-  AssertObserverCount(1, 1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 
   ASSERT_EQ(1, root->child_count());
@@ -461,7 +452,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, 1, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(1, 0, 0, 0, 0, 0, 0, 0, 0);
   observer_details_.ExpectEquals(root, NULL, 0, -1);
 }
 
@@ -490,7 +481,7 @@ TEST_F(BookmarkModelTest, RemoveURL) {
 
   model_->Remove(root, 0);
   ASSERT_EQ(0, root->child_count());
-  AssertObserverCount(0, 0, 0, 1, 0, 0, 1, 0, 0, 0);
+  AssertObserverCount(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.
@@ -513,7 +504,7 @@ TEST_F(BookmarkModelTest, RemoveFolder) {
   // Now remove the folder.
   model_->Remove(root, 0);
   ASSERT_EQ(0, root->child_count());
-  AssertObserverCount(0, 0, 0, 1, 0, 0, 1, 0, 0, 0);
+  AssertObserverCount(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.
@@ -534,7 +525,7 @@ TEST_F(BookmarkModelTest, RemoveAllUserBookmarks) {
   const BookmarkNode* folder = model_->AddFolder(bookmark_bar_node, 0, title);
   model_->AddURL(folder, 0, title, url);
 
-  AssertObserverCount(3, 3, 0, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(3, 0, 0, 0, 0, 0, 0, 0, 0);
   ClearCounts();
 
   model_->RemoveAllUserBookmarks();
@@ -542,7 +533,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, 0, 1);
+  AssertObserverCount(0, 0, 0, 0, 0, 0, 0, 0, 1);
   AssertExtensiveChangesObserverCount(1, 1);
   EXPECT_EQ(1, AllNodesRemovedObserverCount());
 }
@@ -557,7 +548,7 @@ TEST_F(BookmarkModelTest, SetTitle) {
 
   title = ASCIIToUTF16("foo2");
   model_->SetTitle(node, title);
-  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 1, 0, 0);
+  AssertObserverCount(0, 0, 0, 1, 0, 0, 1, 0, 0);
   observer_details_.ExpectEquals(node, NULL, -1, -1);
   EXPECT_EQ(title, node->GetTitle());
 }
@@ -586,7 +577,7 @@ TEST_F(BookmarkModelTest, SetURL) {
 
   url = GURL("http://foo2.com");
   model_->SetURL(node, url);
-  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 1, 0, 0);
+  AssertObserverCount(0, 0, 0, 1, 0, 0, 1, 0, 0);
   observer_details_.ExpectEquals(node, NULL, -1, -1);
   EXPECT_EQ(url, node->url());
 }
@@ -601,7 +592,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, 0);
+  AssertObserverCount(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());
 }
@@ -616,7 +607,7 @@ TEST_F(BookmarkModelTest, Move) {
 
   model_->Move(node, folder1, 0);
 
-  AssertObserverCount(0, 0, 1, 0, 0, 0, 0, 0, 0, 0);
+  AssertObserverCount(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());
@@ -627,7 +618,7 @@ TEST_F(BookmarkModelTest, Move) {
   // And remove the folder.
   ClearCounts();
   model_->Remove(root, 0);
-  AssertObserverCount(0, 0, 0, 1, 0, 0, 1, 0, 0, 0);
+  AssertObserverCount(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());
@@ -1021,7 +1012,7 @@ TEST_F(BookmarkModelTest, Sort) {
   model_->SortChildren(parent);
 
   // Make sure we were notified.
-  AssertObserverCount(0, 0, 0, 0, 0, 1, 0, 0, 1, 0);
+  AssertObserverCount(0, 0, 0, 0, 1, 0, 0, 1, 0);
 
   // Make sure the order matches (remember, 'a' and 'C' are folders and
   // come first).
@@ -1049,7 +1040,7 @@ TEST_F(BookmarkModelTest, Reorder) {
   model_->ReorderChildren(parent, new_order);
 
   // Make sure we were notified.
-  AssertObserverCount(0, 0, 0, 0, 0, 1, 0, 0, 1, 0);
+  AssertObserverCount(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());