We need to save bookmarks file when the persist_ids settings changes

or when the file is detected to be changed externally.

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

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

4 files changed, 42 insertions, 0 deletions

diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc
index 5fc0af8..1f75e0a 100644
--- a/chrome/browser/bookmarks/bookmark_model.cc
+++ b/chrome/browser/bookmarks/bookmark_model.cc
@@ -89,6 +89,7 @@ BookmarkModel::BookmarkModel(Profile* profile)
     : profile_(profile),
       loaded_(false),
       persist_ids_(false),
+      file_changed_(false),
       root_(GURL()),
       bookmark_bar_node_(NULL),
       other_node_(NULL),
@@ -392,6 +393,9 @@ void BookmarkModel::SetPersistIDs(bool value) {
     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) {
@@ -449,6 +453,8 @@ void BookmarkModel::DoneLoading(
   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();
   index_.reset(details->index());
   details->release();
 
@@ -700,6 +706,16 @@ int 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();
diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h
index 669a0b5..d00e7c6 100644
--- a/chrome/browser/bookmarks/bookmark_model.h
+++ b/chrome/browser/bookmarks/bookmark_model.h
@@ -328,6 +328,9 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   bool PersistIDs() const { return persist_ids_; }
   void SetPersistIDs(bool value);
 
+  // Returns whether the bookmarks file changed externally.
+  bool file_changed() const { return file_changed_; }
+
  private:
   // Used to order BookmarkNodes by URL.
   class NodeURLComparator {
@@ -418,6 +421,9 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   // persisted.
   void set_next_node_id(int id) { next_node_id_ = id; }
 
+  // Records that the bookmarks file was changed externally.
+  void SetFileChanged();
+
   // Creates and returns a new LoadDetails. It's up to the caller to delete
   // the returned object.
   BookmarkStorage::LoadDetails* CreateLoadDetails();
@@ -437,6 +443,10 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
   // Whether to persist bookmark IDs.
   bool persist_ids_;
 
+  // Whether the bookmarks file was changed externally. This is set after
+  // loading is complete and once set the value never changes.
+  bool file_changed_;
+
   // The root node. This contains the bookmark bar node and the 'other' node as
   // children.
   BookmarkNode root_;
diff --git a/chrome/browser/bookmarks/bookmark_storage.cc b/chrome/browser/bookmarks/bookmark_storage.cc
index e438e186..2ec9b57 100644
--- a/chrome/browser/bookmarks/bookmark_storage.cc
+++ b/chrome/browser/bookmarks/bookmark_storage.cc
@@ -92,6 +92,8 @@ class BookmarkStorage::LoadTask : public Task {
         codec.Decode(details_->bb_node(), details_->other_folder_node(),
                      &max_node_id, *root.get());
         details_->set_max_id(std::max(max_node_id, details_->max_id()));
+        details_->set_computed_checksum(codec.computed_checksum());
+        details_->set_stored_checksum(codec.stored_checksum());
         UMA_HISTOGRAM_TIMES("Bookmarks.DecodeTime",
                             TimeTicks::Now() - start_time);
 
diff --git a/chrome/browser/bookmarks/bookmark_storage.h b/chrome/browser/bookmarks/bookmark_storage.h
index f9a080d..cc18b60 100644
--- a/chrome/browser/bookmarks/bookmark_storage.h
+++ b/chrome/browser/bookmarks/bookmark_storage.h
@@ -65,11 +65,25 @@ class BookmarkStorage : public NotificationObserver,
     void set_max_id(int max_id) { max_id_ = max_id; }
     int max_id() const { return max_id_; }
 
+    // Computed checksum.
+    void set_computed_checksum(const std::string& value) {
+      computed_checksum_ = value;
+    }
+    const std::string& computed_checksum() const { return computed_checksum_; }
+
+    // Stored checksum.
+    void set_stored_checksum(const std::string& value) {
+      stored_checksum_ = value;
+    }
+    const std::string& stored_checksum() const { return stored_checksum_; }
+
    private:
     scoped_ptr<BookmarkNode> bb_node_;
     scoped_ptr<BookmarkNode> other_folder_node_;
     scoped_ptr<BookmarkIndex> index_;
     int max_id_;
+    std::string computed_checksum_;
+    std::string stored_checksum_;
 
     DISALLOW_COPY_AND_ASSIGN(LoadDetails);
   };