Try the original CL "Always persist bookmark IDs" again with the fix to...

chrome/browser/bookmarks/bookmark_model.cc

 // Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/bookmarks/bookmark_model.h"
 
 #include "app/l10n_util.h"
 #include "base/gfx/png_decoder.h"
 #include "base/scoped_vector.h"
 #include "build/build_config.h"
@@ skipping 17 matching lines @@
 
 }  // anonymous namespace
 
 // BookmarkNode ---------------------------------------------------------------
 
 BookmarkNode::BookmarkNode(const GURL& url)
     : url_(url) {
   Initialize(0);
 }
 
-BookmarkNode::BookmarkNode(int id, const GURL& url)
+BookmarkNode::BookmarkNode(int64 id, const GURL& url)
     : url_(url){
   Initialize(id);
 }
 
-void BookmarkNode::Initialize(int id) {
+void BookmarkNode::Initialize(int64 id) {
   id_ = id;
   loaded_favicon_ = false;
   favicon_load_handle_ = 0;
   type_ = !url_.is_empty() ? URL : BOOKMARK_BAR;
   date_added_ = Time::Now();
 }
 
 void BookmarkNode::Reset(const history::StarredEntry& entry) {
   DCHECK(entry.type != history::StarredEntry::URL || entry.url == url_);
 
@@ skipping 16 matching lines @@
   }
   date_added_ = entry.date_added;
   date_group_modified_ = entry.date_group_modified;
   SetTitle(entry.title);
 }
 
 // BookmarkModel --------------------------------------------------------------
 
 namespace {
 
-// Constant for persist IDs prefernece.
-const wchar_t kPrefPersistIDs[] = L"bookmarks.persist_ids";
-
 // Comparator used when sorting bookmarks. Folders are sorted first, then
   // bookmarks.
 class SortComparator : public std::binary_function<const BookmarkNode*,
                                                    const BookmarkNode*,
                                                    bool> {
  public:
   explicit SortComparator(Collator* collator) : collator_(collator) { }
 
   // Returns true if lhs preceeds rhs.
   bool operator() (const BookmarkNode* n1, const BookmarkNode* n2) {
@@ skipping 10 matching lines @@
 
  private:
   Collator* collator_;
 };
 
 }  // namespace
 
 BookmarkModel::BookmarkModel(Profile* profile)
     : profile_(profile),
       loaded_(false),
-      persist_ids_(false),
       file_changed_(false),
       root_(GURL()),
       bookmark_bar_node_(NULL),
       other_node_(NULL),
       next_node_id_(1),
       observers_(ObserverList<BookmarkModelObserver>::NOTIFY_EXISTING_ONLY),
       loaded_signal_(TRUE, FALSE) {
   if (!profile_) {
     // Profile is null during testing.
     DoneLoading(CreateLoadDetails());
   }
-  RegisterPreferences();
-  LoadPreferences();
 }
 
 BookmarkModel::~BookmarkModel() {
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     BookmarkModelBeingDeleted(this));
 
   if (store_) {
     // The store maintains a reference back to us. We need to tell it we're gone
     // so that it doesn't try and invoke a method back on us again.
     store_->BookmarkModelDeleted();
@@ skipping 138 matching lines @@
       urls->push_back(*url);
     last_url = url;
   }
 }
 
 bool BookmarkModel::IsBookmarked(const GURL& url) {
   AutoLock url_lock(url_lock_);
   return IsBookmarkedNoLock(url);
 }
 
-const BookmarkNode* BookmarkModel::GetNodeByID(int id) {
+const BookmarkNode* BookmarkModel::GetNodeByID(int64 id) {
   // TODO(sky): TreeNode needs a method that visits all nodes using a predicate.
   return GetNodeByID(&root_, id);
 }
 
 const BookmarkNode* BookmarkModel::AddGroup(const BookmarkNode* parent,
                                             int index,
                                             const std::wstring& title) {
   if (!loaded_ || parent == &root_ || !IsValidIndex(parent, index, true)) {
     // Can't add to the root.
     NOTREACHED();
@@ skipping 105 matching lines @@
     return;
 
   index_->GetBookmarksWithTitlesMatching(text, max_count, matches);
 }
 
 void BookmarkModel::ClearStore() {
   registrar_.RemoveAll();
   store_ = NULL;
 }
 
-void BookmarkModel::SetPersistIDs(bool value) {
-  if (value == persist_ids_)
-    return;
-  persist_ids_ = value;
-  if (profile_) {
-    PrefService* pref_service = profile_->GetPrefs();
-    pref_service->SetBoolean(kPrefPersistIDs, persist_ids_);
-  }
-  // Need to save the bookmark data if the value of persist IDs changes.
-  if (store_.get())
-    store_->ScheduleSave();
-}
-
 bool BookmarkModel::IsBookmarkedNoLock(const GURL& url) {
   BookmarkNode tmp_node(url);
   return (nodes_ordered_by_url_set_.find(&tmp_node) !=
           nodes_ordered_by_url_set_.end());
 }
 
 void BookmarkModel::FavIconLoaded(const BookmarkNode* node) {
   // Send out notification to the observer.
   FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
                     BookmarkNodeFavIconLoaded(this, node));
@@ skipping 36 matching lines @@
     // We should only ever be loaded once.
     NOTREACHED();
     return;
   }
 
   bookmark_bar_node_ = details->bb_node();
   other_node_ = details->other_folder_node();
   next_node_id_ = details->max_id();
   if (details->computed_checksum() != details->stored_checksum())
     SetFileChanged();
+  if (details->computed_checksum() != details->stored_checksum() ||
+      details->ids_reassigned()) {
+    // If bookmarks file changed externally, the IDs may have changed
+    // externally. In that case, the decoder may have reassigned IDs to make
+    // them unique. So when the file has changed externally, we should save the
+    // bookmarks file to persist new IDs.
+    if (store_.get())
+      store_->ScheduleSave();
+  }
   index_.reset(details->index());
   details->release();
 
   // WARNING: order is important here, various places assume bookmark bar then
   // other node.
   root_.Add(0, bookmark_bar_node_);
   root_.Add(1, other_node_);
 
   {
     AutoLock url_lock(url_lock_);
@@ skipping 90 matching lines @@
         Details<history::URLsStarredDetails>(&details));
   }
   return node;
 }
 
 void BookmarkModel::BlockTillLoaded() {
   loaded_signal_.Wait();
 }
 
 const BookmarkNode* BookmarkModel::GetNodeByID(const BookmarkNode* node,
-                                               int id) {
+                                               int64 id) {
   if (node->id() == id)
     return node;
 
   for (int i = 0, child_count = node->GetChildCount(); i < child_count; ++i) {
     const BookmarkNode* result = GetNodeByID(node->GetChild(i), id);
     if (result)
       return result;
   }
   return NULL;
 }
@@ skipping 117 matching lines @@
 
 void BookmarkModel::PopulateNodesByURL(BookmarkNode* node) {
   // NOTE: this is called with url_lock_ already held. As such, this doesn't
   // explicitly grab the lock.
   if (node->is_url())
     nodes_ordered_by_url_set_.insert(node);
   for (int i = 0; i < node->GetChildCount(); ++i)
     PopulateNodesByURL(node->GetChild(i));
 }
 
-int BookmarkModel::generate_next_node_id() {
+int64 BookmarkModel::generate_next_node_id() {
   return next_node_id_++;
 }
 
 void BookmarkModel::SetFileChanged() {
   file_changed_ = true;
-  // If bookmarks file changed externally, the IDs may have changed externally.
-  // in that case, the decoder may have reassigned IDs to make them unique.
-  // So when the file has changed externally and IDs are persisted, we should
-  // save the bookmarks file to persist new IDs.
-  if (persist_ids_ && store_.get())
-    store_->ScheduleSave();
 }
 
 BookmarkStorage::LoadDetails* BookmarkModel::CreateLoadDetails() {
   BookmarkNode* bb_node = CreateBookmarkNode();
   BookmarkNode* other_folder_node = CreateOtherBookmarksNode();
   return new BookmarkStorage::LoadDetails(
       bb_node, other_folder_node, new BookmarkIndex(), next_node_id_);
 }
-
-void BookmarkModel::RegisterPreferences() {
-  if (!profile_)
-    return;
-  PrefService* pref_service = profile_->GetPrefs();
-  if (!pref_service->IsPrefRegistered(kPrefPersistIDs))
-    pref_service->RegisterBooleanPref(kPrefPersistIDs, false);
-}
-
-void BookmarkModel::LoadPreferences() {
-  if (!profile_)
-    return;
-  PrefService* pref_service = profile_->GetPrefs();
-  persist_ids_ = pref_service->GetBoolean(kPrefPersistIDs);
-}