Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(90)

Unified Diff: components/enhanced_bookmarks/enhanced_bookmark_model.cc

Issue 563363002: Only set remote id during url node creation. (Closed) Base URL: https://chromium.googlesource.com/chromium/src@master
Patch Set: Created 6 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: components/enhanced_bookmarks/enhanced_bookmark_model.cc
diff --git a/components/enhanced_bookmarks/enhanced_bookmark_model.cc b/components/enhanced_bookmarks/enhanced_bookmark_model.cc
index a20e38bac31b494cd8b63e2ff329fd38d645477e..013f45753f28acddfa8c6e295c822ba79f16e0de 100644
--- a/components/enhanced_bookmarks/enhanced_bookmark_model.cc
+++ b/components/enhanced_bookmarks/enhanced_bookmark_model.cc
@@ -9,10 +9,13 @@
#include "base/base64.h"
#include "base/logging.h"
+#include "base/message_loop/message_loop_proxy.h"
#include "base/rand_util.h"
#include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/browser/bookmark_node.h"
+#include "components/enhanced_bookmarks/enhanced_bookmark_model_observer.h"
#include "components/enhanced_bookmarks/proto/metadata.pb.h"
+#include "ui/base/models/tree_node_iterator.h"
#include "url/gurl.h"
namespace {
@@ -21,11 +24,10 @@ const char* kBookmarkBarId = "f_bookmarks_bar";
const char* kIdKey = "stars.id";
const char* kImageDataKey = "stars.imageData";
const char* kNoteKey = "stars.note";
+const char* kOldIdKey = "stars.oldId";
const char* kPageDataKey = "stars.pageData";
-const char* kUserEditKey = "stars.userEdit";
const char* kVersionKey = "stars.version";
-const char* kFolderPrefix = "ebf_";
const char* kBookmarkPrefix = "ebc_";
// Helper method for working with bookmark metainfo.
@@ -62,13 +64,9 @@ bool PopulateImageData(const image::collections::ImageData_ImageInfo& info,
// Generate a random remote id, with a prefix that depends on whether the node
// is a folder or a bookmark.
-std::string GenerateRemoteId(bool is_folder) {
+std::string GenerateRemoteId() {
std::stringstream random_id;
- // Add prefix depending on whether the node is a folder or not.
- if (is_folder)
- random_id << kFolderPrefix;
- else
- random_id << kBookmarkPrefix;
+ random_id << kBookmarkPrefix;
// Generate 32 digit hex string random suffix.
random_id << std::hex << std::setfill('0') << std::setw(16);
@@ -81,17 +79,43 @@ namespace enhanced_bookmarks {
EnhancedBookmarkModel::EnhancedBookmarkModel(BookmarkModel* bookmark_model,
const std::string& version)
- : bookmark_model_(bookmark_model), version_(version) {
+ : bookmark_model_(bookmark_model),
+ loaded_(false),
+ weak_ptr_factory_(this),
+ version_(version) {
+ bookmark_model_->AddObserver(this);
+ if (bookmark_model_->loaded()) {
+ InitializeIdMap();
+ loaded_ = true;
+ }
}
EnhancedBookmarkModel::~EnhancedBookmarkModel() {
}
+void EnhancedBookmarkModel::ShutDown() {
+ FOR_EACH_OBSERVER(EnhancedBookmarkModelObserver,
+ observers_,
+ EnhancedBookmarkModelShuttingDown());
+ weak_ptr_factory_.InvalidateWeakPtrs();
+ bookmark_model_->RemoveObserver(this);
+ bookmark_model_ = NULL;
+}
+
+void EnhancedBookmarkModel::AddObserver(
+ EnhancedBookmarkModelObserver* observer) {
+ observers_.AddObserver(observer);
+}
+
+void EnhancedBookmarkModel::RemoveObserver(
+ EnhancedBookmarkModelObserver* observer) {
+ observers_.RemoveObserver(observer);
+}
+
// Moves |node| to |new_parent| and inserts it at the given |index|.
void EnhancedBookmarkModel::Move(const BookmarkNode* node,
const BookmarkNode* new_parent,
int index) {
- // TODO(rfevang): Update meta info placement fields.
bookmark_model_->Move(node, new_parent, index);
}
@@ -100,12 +124,7 @@ const BookmarkNode* EnhancedBookmarkModel::AddFolder(
const BookmarkNode* parent,
int index,
const base::string16& title) {
- BookmarkNode::MetaInfoMap meta_info;
- meta_info[kIdKey] = GenerateRemoteId(true);
-
- // TODO(rfevang): Set meta info placement fields.
- return bookmark_model_->AddFolderWithMetaInfo(
- parent, index, title, &meta_info);
+ return bookmark_model_->AddFolder(parent, index, title);
}
// Adds a url at the specified position.
@@ -116,9 +135,7 @@ const BookmarkNode* EnhancedBookmarkModel::AddURL(
const GURL& url,
const base::Time& creation_time) {
BookmarkNode::MetaInfoMap meta_info;
- meta_info[kIdKey] = GenerateRemoteId(false);
-
- // TODO(rfevang): Set meta info placement fields.
+ meta_info[kIdKey] = GenerateRemoteId();
return bookmark_model_->AddURLWithCreationTimeAndMetaInfo(
parent, index, title, url, creation_time, &meta_info);
}
@@ -127,24 +144,23 @@ std::string EnhancedBookmarkModel::GetRemoteId(const BookmarkNode* node) {
if (node == bookmark_model_->bookmark_bar_node())
return kBookmarkBarId;
- // Permanent nodes other than the bookmarks bar don't have ids.
- DCHECK(!bookmark_model_->is_permanent_node(node));
-
std::string id;
- if (!node->GetMetaInfo(kIdKey, &id) || id.empty())
- return SetRemoteId(node);
+ if (!node->GetMetaInfo(kIdKey, &id))
+ return std::string();
return id;
}
-std::string EnhancedBookmarkModel::SetRemoteId(const BookmarkNode* node) {
- std::string remote_id = GenerateRemoteId(node->is_folder());
- SetMetaInfo(node, kIdKey, remote_id, false);
- return remote_id;
+const BookmarkNode* EnhancedBookmarkModel::BookmarkForRemoteId(
+ const std::string& remote_id) {
+ IdToNodeMap::iterator it = id_map_.find(remote_id);
+ if (it != id_map_.end())
+ return it->second;
+ return NULL;
}
void EnhancedBookmarkModel::SetDescription(const BookmarkNode* node,
const std::string& description) {
- SetMetaInfo(node, kNoteKey, description, true);
+ SetMetaInfo(node, kNoteKey, description);
}
std::string EnhancedBookmarkModel::GetDescription(const BookmarkNode* node) {
@@ -189,9 +205,7 @@ bool EnhancedBookmarkModel::SetOriginalImage(const BookmarkNode* node,
std::string encoded;
base::Base64Encode(output, &encoded);
- SetMetaInfo(node, kImageDataKey, encoded, true);
- // Ensure that the bookmark has a stars.id, to trigger the server processing.
- GetRemoteId(node);
+ SetMetaInfo(node, kImageDataKey, encoded);
return true;
}
@@ -251,10 +265,120 @@ void EnhancedBookmarkModel::SetVersionSuffix(
version_suffix_ = version_suffix;
}
+void EnhancedBookmarkModel::BookmarkModelChanged() {
+}
+
+void EnhancedBookmarkModel::BookmarkModelLoaded(BookmarkModel* model,
+ bool ids_reassigned) {
+ InitializeIdMap();
+ FOR_EACH_OBSERVER(
+ EnhancedBookmarkModelObserver, observers_, EnhancedBookmarkModelLoaded());
+}
+
+void EnhancedBookmarkModel::BookmarkNodeAdded(BookmarkModel* model,
+ const BookmarkNode* parent,
+ int index) {
+ const BookmarkNode* node = parent->GetChild(index);
+ AddToIdMap(node);
+ ScheduleResetDuplicateRemoteIds();
+ FOR_EACH_OBSERVER(
+ EnhancedBookmarkModelObserver, observers_, EnhancedBookmarkAdded(node));
+}
+
+void EnhancedBookmarkModel::BookmarkNodeRemoved(
+ BookmarkModel* model,
+ const BookmarkNode* parent,
+ int old_index,
+ const BookmarkNode* node,
+ const std::set<GURL>& removed_urls) {
+ std::string remote_id = GetRemoteId(node);
+ id_map_.erase(remote_id);
+ FOR_EACH_OBSERVER(
+ EnhancedBookmarkModelObserver, observers_, EnhancedBookmarkRemoved(node));
+}
+
+void EnhancedBookmarkModel::OnWillChangeBookmarkMetaInfo(
+ BookmarkModel* model,
+ const BookmarkNode* node) {
+ prev_remote_id_ = GetRemoteId(node);
+}
+
+void EnhancedBookmarkModel::BookmarkMetaInfoChanged(BookmarkModel* model,
+ const BookmarkNode* node) {
+ std::string remote_id = GetRemoteId(node);
+ if (remote_id != prev_remote_id_) {
+ id_map_.erase(prev_remote_id_);
+ if (!remote_id.empty()) {
+ AddToIdMap(node);
+ ScheduleResetDuplicateRemoteIds();
+ }
+ FOR_EACH_OBSERVER(
+ EnhancedBookmarkModelObserver,
+ observers_,
+ EnhancedBookmarkRemoteIdChanged(node, prev_remote_id_, remote_id));
+ }
+}
+
+void EnhancedBookmarkModel::BookmarkAllUserNodesRemoved(
+ BookmarkModel* model,
+ const std::set<GURL>& removed_urls) {
+ id_map_.clear();
+ // Re-initialize so non-user nodes with remote ids are present in the map.
+ InitializeIdMap();
+ FOR_EACH_OBSERVER(EnhancedBookmarkModelObserver,
+ observers_,
+ EnhancedBookmarkAllUserNodesRemoved());
+}
+
+void EnhancedBookmarkModel::InitializeIdMap() {
+ ui::TreeNodeIterator<const BookmarkNode> iterator(
+ bookmark_model_->root_node());
+ while (iterator.has_next()) {
+ AddToIdMap(iterator.Next());
+ }
+ ScheduleResetDuplicateRemoteIds();
+}
+
+void EnhancedBookmarkModel::AddToIdMap(const BookmarkNode* node) {
+ std::string remote_id = GetRemoteId(node);
+ if (remote_id.empty())
+ return;
+
+ // Try to insert the node.
+ std::pair<IdToNodeMap::iterator, bool> result =
+ id_map_.insert(make_pair(remote_id, node));
+ if (!result.second) {
+ // Some node already had the same remote id, so add both nodes to the
+ // to-be-reset set.
+ nodes_to_reset_[result.first->second] = remote_id;
+ nodes_to_reset_[node] = remote_id;
+ }
+}
+
+void EnhancedBookmarkModel::ScheduleResetDuplicateRemoteIds() {
+ if (!nodes_to_reset_.empty()) {
+ base::MessageLoopProxy::current()->PostTask(
+ FROM_HERE,
+ base::Bind(&EnhancedBookmarkModel::ResetDuplicateRemoteIds,
+ weak_ptr_factory_.GetWeakPtr()));
+ }
+}
+
+void EnhancedBookmarkModel::ResetDuplicateRemoteIds() {
+ for (NodeToIdMap::iterator it = nodes_to_reset_.begin();
+ it != nodes_to_reset_.end();
+ ++it) {
+ BookmarkNode::MetaInfoMap meta_info;
+ meta_info[kIdKey] = "";
+ meta_info[kOldIdKey] = it->second;
+ SetMultipleMetaInfo(it->first, meta_info);
+ }
+ nodes_to_reset_.clear();
+}
+
void EnhancedBookmarkModel::SetMetaInfo(const BookmarkNode* node,
const std::string& field,
- const std::string& value,
- bool user_edit) {
+ const std::string& value) {
DCHECK(!bookmark_model_->is_permanent_node(node));
BookmarkNode::MetaInfoMap meta_info;
@@ -269,7 +393,6 @@ void EnhancedBookmarkModel::SetMetaInfo(const BookmarkNode* node,
meta_info[field] = value;
meta_info[kVersionKey] = GetVersionString();
- meta_info[kUserEditKey] = user_edit ? "true" : "false";
bookmark_model_->SetNodeMetaInfoMap(node, meta_info);
}
@@ -279,6 +402,37 @@ std::string EnhancedBookmarkModel::GetVersionString() {
return version_ + '/' + version_suffix_;
}
+void EnhancedBookmarkModel::SetMultipleMetaInfo(
+ const BookmarkNode* node,
+ BookmarkNode::MetaInfoMap meta_info) {
+ DCHECK(!bookmark_model_->is_permanent_node(node));
+
+ // Don't update anything if every value is already set correctly.
+ if (node->GetMetaInfoMap()) {
+ bool changed = false;
+ const BookmarkNode::MetaInfoMap* old_meta_info = node->GetMetaInfoMap();
+ for (BookmarkNode::MetaInfoMap::iterator it = meta_info.begin();
+ it != meta_info.end();
+ ++it) {
+ BookmarkNode::MetaInfoMap::const_iterator old_field =
+ old_meta_info->find(it->first);
+ if (old_field == old_meta_info->end() ||
+ old_field->second != it->second) {
+ changed = true;
+ break;
+ }
+ }
+ if (!changed)
+ return;
+
+ // Fill in the values that aren't changing
+ meta_info.insert(old_meta_info->begin(), old_meta_info->end());
+ }
+
+ meta_info[kVersionKey] = GetVersionString();
+ bookmark_model_->SetNodeMetaInfoMap(node, meta_info);
+}
+
bool EnhancedBookmarkModel::SetAllImages(const BookmarkNode* node,
const GURL& image_url,
int image_width,

Powered by Google App Engine
This is Rietveld 408576698