Revert "Always persist bookmark IDs."

This reverts commit r20532 because valgrind was complaining
about uninitialized memory:
http://build.chromium.org/buildbot/waterfall/builders/Chromium%20Linux%20(valgrind)/builds/697/steps/valgrind%20test:%20unit/logs/stdio

TBR=munjal

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


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

4 files changed, 28 insertions, 68 deletions

diff --git a/chrome/browser/extensions/extension_bookmarks_module.cc b/chrome/browser/extensions/extension_bookmarks_module.cc
index 808050a..9c9d635 100644
--- a/chrome/browser/extensions/extension_bookmarks_module.cc
+++ b/chrome/browser/extensions/extension_bookmarks_module.cc
@@ -5,7 +5,6 @@
 #include "chrome/browser/extensions/extension_bookmarks_module.h"
 
 #include "base/json_writer.h"
-#include "base/string_util.h"
 #include "chrome/browser/bookmarks/bookmark_codec.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
 #include "chrome/browser/bookmarks/bookmark_utils.h"
@@ -26,11 +25,11 @@ class ExtensionBookmarks {
   static DictionaryValue* GetNodeDictionary(const BookmarkNode* node,
                                             bool recurse) {
     DictionaryValue* dict = new DictionaryValue();
-    dict->SetString(keys::kIdKey, Int64ToString(node->id()));
+    dict->SetInteger(keys::kIdKey, node->id());
 
     const BookmarkNode* parent = node->GetParent();
     if (parent)
-      dict->SetString(keys::kParentIdKey, Int64ToString(parent->id()));
+      dict->SetInteger(keys::kParentIdKey, parent->id());
 
     if (!node->is_folder()) {
       dict->SetString(keys::kUrlKey, node->GetURL().spec());
@@ -69,7 +68,7 @@ class ExtensionBookmarks {
     list->Append(dict);
   }
 
-  static bool RemoveNode(BookmarkModel* model, int64 id, bool recursive,
+  static bool RemoveNode(BookmarkModel* model, int id, bool recursive,
                          std::string* error) {
     const BookmarkNode* node = model->GetNodeByID(id);
     if (!node) {
@@ -115,15 +114,6 @@ void BookmarksFunction::Run() {
   SendResponse(RunImpl());
 }
 
-bool BookmarksFunction::GetBookmarkIdAsInt64(
-    const std::string& id_string, int64* id) {
-  if (StringToInt64(id_string, id))
-    return true;
-
-  error_ = keys::kInvalidIdError;
-  return false;
-}
-
 void BookmarksFunction::Observe(NotificationType type,
                                 const NotificationSource& source,
                                 const NotificationDetails& details) {
@@ -171,12 +161,11 @@ void ExtensionBookmarkEventRouter::BookmarkNodeMoved(
     int new_index) {
   ListValue args;
   const BookmarkNode* node = new_parent->GetChild(new_index);
-  args.Append(new StringValue(Int64ToString(node->id())));
+  args.Append(new FundamentalValue(node->id()));
   DictionaryValue* object_args = new DictionaryValue();
-  object_args->SetString(keys::kParentIdKey, Int64ToString(new_parent->id()));
+  object_args->SetInteger(keys::kParentIdKey, new_parent->id());
   object_args->SetInteger(keys::kIndexKey, new_index);
-  object_args->SetString(keys::kOldParentIdKey,
-                         Int64ToString(old_parent->id()));
+  object_args->SetInteger(keys::kOldParentIdKey, old_parent->id());
   object_args->SetInteger(keys::kOldIndexKey, old_index);
   args.Append(object_args);
 
@@ -190,7 +179,7 @@ void ExtensionBookmarkEventRouter::BookmarkNodeAdded(BookmarkModel* model,
                                                      int index) {
   ListValue args;
   const BookmarkNode* node = parent->GetChild(index);
-  args.Append(new StringValue(Int64ToString(node->id())));
+  args.Append(new FundamentalValue(node->id()));
   DictionaryValue* obj = ExtensionBookmarks::GetNodeDictionary(node, false);
 
   // Remove id since it's already being passed as the first argument.
@@ -216,9 +205,9 @@ void ExtensionBookmarkEventRouter::BookmarkNodeRemoved(
     int index,
     const BookmarkNode* node) {
   ListValue args;
-  args.Append(new StringValue(Int64ToString(node->id())));
+  args.Append(new FundamentalValue(node->id()));
   DictionaryValue* object_args = new DictionaryValue();
-  object_args->SetString(keys::kParentIdKey, Int64ToString(parent->id()));
+  object_args->SetInteger(keys::kParentIdKey, parent->id());
   object_args->SetInteger(keys::kIndexKey, index);
   args.Append(object_args);
 
@@ -230,7 +219,7 @@ void ExtensionBookmarkEventRouter::BookmarkNodeRemoved(
 void ExtensionBookmarkEventRouter::BookmarkNodeChanged(
     BookmarkModel* model, const BookmarkNode* node) {
   ListValue args;
-  args.Append(new StringValue(Int64ToString(node->id())));
+  args.Append(new FundamentalValue(node->id()));
 
   // TODO(erikkay) The only two things that BookmarkModel sends this
   // notification for are title and favicon.  Since we're currently ignoring
@@ -254,12 +243,12 @@ void ExtensionBookmarkEventRouter::BookmarkNodeFavIconLoaded(
 void ExtensionBookmarkEventRouter::BookmarkNodeChildrenReordered(
     BookmarkModel* model, const BookmarkNode* node) {
   ListValue args;
-  args.Append(new StringValue(Int64ToString(node->id())));
+  args.Append(new FundamentalValue(node->id()));
   int childCount = node->GetChildCount();
   ListValue* children = new ListValue();
   for (int i = 0; i < childCount; ++i) {
     const BookmarkNode* child = node->GetChild(i);
-    Value* child_id = new StringValue(Int64ToString(child->id()));
+    Value* child_id = new FundamentalValue(child->id());
     children->Append(child_id);
   }
   args.Append(children);
@@ -279,11 +268,8 @@ bool GetBookmarksFunction::RunImpl() {
     size_t count = ids->GetSize();
     EXTENSION_FUNCTION_VALIDATE(count > 0);
     for (size_t i = 0; i < count; ++i) {
-      int64 id;
-      std::string id_string;
-      EXTENSION_FUNCTION_VALIDATE(ids->GetString(i, &id_string));
-      if (!GetBookmarkIdAsInt64(id_string, &id))
-        return false;
+      int id = 0;
+      EXTENSION_FUNCTION_VALIDATE(ids->GetInteger(i, &id));
       const BookmarkNode* node = model->GetNodeByID(id);
       if (!node) {
         error_ = keys::kNoNodeError;
@@ -293,11 +279,8 @@ bool GetBookmarksFunction::RunImpl() {
       }
     }
   } else {
-    int64 id;
-    std::string id_string;
-    EXTENSION_FUNCTION_VALIDATE(args_->GetAsString(&id_string));
-    if (!GetBookmarkIdAsInt64(id_string, &id))
-      return false;
+    int id;
+    EXTENSION_FUNCTION_VALIDATE(args_->GetAsInteger(&id));
     const BookmarkNode* node = model->GetNodeByID(id);
     if (!node) {
       error_ = keys::kNoNodeError;
@@ -312,11 +295,8 @@ bool GetBookmarksFunction::RunImpl() {
 
 bool GetBookmarkChildrenFunction::RunImpl() {
   BookmarkModel* model = profile()->GetBookmarkModel();
-  int64 id;
-  std::string id_string;
-  EXTENSION_FUNCTION_VALIDATE(args_->GetAsString(&id_string));
-  if (!GetBookmarkIdAsInt64(id_string, &id))
-    return false;
+  int id;
+  EXTENSION_FUNCTION_VALIDATE(args_->GetAsInteger(&id));
   scoped_ptr<ListValue> json(new ListValue());
   const BookmarkNode* node = model->GetNodeByID(id);
   if (!node) {
@@ -370,9 +350,8 @@ bool RemoveBookmarkFunction::RunImpl() {
   EXTENSION_FUNCTION_VALIDATE(args->GetBoolean(1, &recursive));
 
   BookmarkModel* model = profile()->GetBookmarkModel();
-  int64 id;
-  std::string id_string;
-  if (args->GetString(0, &id_string) && StringToInt64(id_string, &id)) {
+  int id;
+  if (args->GetInteger(0, &id)) {
     return ExtensionBookmarks::RemoveNode(model, id, recursive, &error_);
   } else {
     ListValue* ids;
@@ -380,9 +359,7 @@ bool RemoveBookmarkFunction::RunImpl() {
     size_t count = ids->GetSize();
     EXTENSION_FUNCTION_VALIDATE(count > 0);
     for (size_t i = 0; i < count; ++i) {
-      EXTENSION_FUNCTION_VALIDATE(ids->GetString(i, &id_string));
-      if (!GetBookmarkIdAsInt64(id_string, &id))
-        return false;
+      EXTENSION_FUNCTION_VALIDATE(ids->GetInteger(i, &id));
       if (!ExtensionBookmarks::RemoveNode(model, id, recursive, &error_))
         return false;
     }
@@ -395,16 +372,13 @@ bool CreateBookmarkFunction::RunImpl() {
   DictionaryValue* json = static_cast<DictionaryValue*>(args_);
 
   BookmarkModel* model = profile()->GetBookmarkModel();
-  int64 parentId;
+  int parentId;
   if (!json->HasKey(keys::kParentIdKey)) {
     // Optional, default to "other bookmarks".
     parentId = model->other_node()->id();
   } else {
-    std::string parentId_string;
-    EXTENSION_FUNCTION_VALIDATE(json->GetString(keys::kParentIdKey,
-                                                &parentId_string));
-    if (!GetBookmarkIdAsInt64(parentId_string, &parentId))
-      return false;
+    EXTENSION_FUNCTION_VALIDATE(json->GetInteger(keys::kParentIdKey,
+                                                 &parentId));
   }
   const BookmarkNode* parent = model->GetNodeByID(parentId);
   if (!parent) {
@@ -457,11 +431,8 @@ bool CreateBookmarkFunction::RunImpl() {
 bool MoveBookmarkFunction::RunImpl() {
   EXTENSION_FUNCTION_VALIDATE(args_->IsType(Value::TYPE_LIST));
   const ListValue* args = static_cast<const ListValue*>(args_);
-  int64 id;
-  std::string id_string;
-  EXTENSION_FUNCTION_VALIDATE(args->GetString(0, &id_string));
-  if (!GetBookmarkIdAsInt64(id_string, &id))
-    return false;
+  int id;
+  EXTENSION_FUNCTION_VALIDATE(args->GetInteger(0, &id));
   DictionaryValue* destination;
   EXTENSION_FUNCTION_VALIDATE(args->GetDictionary(1, &destination));
 
@@ -522,11 +493,8 @@ bool SetBookmarkTitleFunction::RunImpl() {
   json->GetString(keys::kTitleKey, &title);  // Optional (empty is clear).
 
   BookmarkModel* model = profile()->GetBookmarkModel();
-  int64 id = 0;
-  std::string id_string;
-  EXTENSION_FUNCTION_VALIDATE(json->GetString(keys::kIdKey, &id_string));
-  if (!GetBookmarkIdAsInt64(id_string, &id))
-    return false;
+  int id = 0;
+  EXTENSION_FUNCTION_VALIDATE(json->GetInteger(keys::kIdKey, &id));
   const BookmarkNode* node = model->GetNodeByID(id);
   if (!node) {
     error_ = keys::kNoNodeError;
diff --git a/chrome/browser/extensions/extension_bookmarks_module.h b/chrome/browser/extensions/extension_bookmarks_module.h
index 7cfc1ca2..cfa32de 100644
--- a/chrome/browser/extensions/extension_bookmarks_module.h
+++ b/chrome/browser/extensions/extension_bookmarks_module.h
@@ -70,12 +70,6 @@ class BookmarksFunction : public AsyncExtensionFunction,
   virtual void Run();
   virtual bool RunImpl() = 0;
 
- protected:
-  // Helper to get the bookmark id as int64 from the given string id.
-  // Sets error_ to an errro string if the given id string can't be parsed
-  // as an int64. In case of error, doesn't change id and returns false.
-  bool GetBookmarkIdAsInt64(const std::string& id_string, int64* id);
-
  private:
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
diff --git a/chrome/browser/extensions/extension_bookmarks_module_constants.cc b/chrome/browser/extensions/extension_bookmarks_module_constants.cc
index 24d7c92..cdf602d 100644
--- a/chrome/browser/extensions/extension_bookmarks_module_constants.cc
+++ b/chrome/browser/extensions/extension_bookmarks_module_constants.cc
@@ -22,7 +22,6 @@ const char kNoNodeError[] = "Can't find bookmark for id.";
 const char kNoParentError[] = "Can't find parent bookmark for id.";
 const char kFolderNotEmptyError[] =
     "Can't remove non-empty folder (use recursive to force).";
-const char kInvalidIdError[] = "Bookmark id is invalid.";
 const char kInvalidIndexError[] = "Index out of bounds.";
 const char kInvalidUrlError[] = "Invalid URL.";
 const char kModifySpecialError[] = "Can't modify the root bookmark folders.";
diff --git a/chrome/browser/extensions/extension_bookmarks_module_constants.h b/chrome/browser/extensions/extension_bookmarks_module_constants.h
index 665747c..3b504e6 100644
--- a/chrome/browser/extensions/extension_bookmarks_module_constants.h
+++ b/chrome/browser/extensions/extension_bookmarks_module_constants.h
@@ -26,7 +26,6 @@ extern const wchar_t kDateGroupModifiedKey[];
 extern const char kNoNodeError[];
 extern const char kNoParentError[];
 extern const char kFolderNotEmptyError[];
-extern const char kInvalidIdError[];
 extern const char kInvalidIndexError[];
 extern const char kInvalidUrlError[];
 extern const char kModifySpecialError[];