summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/browser/bookmarks/bookmark_model.cc40
-rw-r--r--chrome/browser/bookmarks/bookmark_model.h9
-rw-r--r--chrome/browser/bookmarks/bookmark_model_observer.h2
-rw-r--r--chrome/browser/bookmarks/bookmark_model_unittest.cc15
-rw-r--r--chrome/browser/bookmarks/bookmark_utils.cc50
-rw-r--r--chrome/browser/bookmarks/bookmark_utils.h12
-rw-r--r--chrome/browser/sync/glue/bookmark_change_processor.cc17
7 files changed, 83 insertions, 62 deletions
diff --git a/chrome/browser/bookmarks/bookmark_model.cc b/chrome/browser/bookmarks/bookmark_model.cc
index 5385810..f32ac74 100644
--- a/chrome/browser/bookmarks/bookmark_model.cc
+++ b/chrome/browser/bookmarks/bookmark_model.cc
@@ -248,6 +248,46 @@ void BookmarkModel::SetTitle(const BookmarkNode* node, const string16& title) {
BookmarkNodeChanged(this, node));
}
+void BookmarkModel::SetURL(const BookmarkNode* node, const GURL& url) {
+ if (!node) {
+ NOTREACHED();
+ return;
+ }
+
+ // We cannot change the URL of a folder.
+ if (node->is_folder()) {
+ NOTREACHED();
+ return;
+ }
+
+ if (url == node->GetURL())
+ return;
+
+ AsMutable(node)->InvalidateFavicon();
+ CancelPendingFavIconLoadRequests(AsMutable(node));
+
+ {
+ AutoLock url_lock(url_lock_);
+ NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.find(
+ AsMutable(node));
+ DCHECK(i != nodes_ordered_by_url_set_.end());
+ // i points to the first node with the URL, advance until we find the
+ // node we're removing.
+ while (*i != node)
+ ++i;
+ nodes_ordered_by_url_set_.erase(i);
+
+ AsMutable(node)->SetURL(url);
+ nodes_ordered_by_url_set_.insert(AsMutable(node));
+ }
+
+ if (store_.get())
+ store_->ScheduleSave();
+
+ FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
+ BookmarkNodeChanged(this, node));
+}
+
void BookmarkModel::GetNodesByURL(const GURL& url,
std::vector<const BookmarkNode*>* nodes) {
AutoLock url_lock(url_lock_);
diff --git a/chrome/browser/bookmarks/bookmark_model.h b/chrome/browser/bookmarks/bookmark_model.h
index 1ffdcbe..7790790 100644
--- a/chrome/browser/bookmarks/bookmark_model.h
+++ b/chrome/browser/bookmarks/bookmark_model.h
@@ -61,6 +61,8 @@ class BookmarkNode : public TreeNode<BookmarkNode> {
// Returns the URL.
const GURL& GetURL() const { return url_; }
+ // Sets the URL to the given value.
+ void SetURL(const GURL& url) { url_ = url; }
// Returns a unique id for this node.
// For bookmark nodes that are managed by the bookmark model, the IDs are
@@ -144,8 +146,8 @@ class BookmarkNode : public TreeNode<BookmarkNode> {
HistoryService::Handle favicon_load_handle_;
// The URL. BookmarkModel maintains maps off this URL, it is important that
- // it not change once the node has been created.
- const GURL url_;
+ // changes to the URL is done through the bookmark model.
+ GURL url_;
// Type of node.
BookmarkNode::Type type_;
@@ -231,6 +233,9 @@ class BookmarkModel : public NotificationObserver, public BookmarkService {
#endif
void SetTitle(const BookmarkNode* node, const string16& title);
+ // Sets the URL of the specified bookmark node.
+ void SetURL(const BookmarkNode* node, const GURL& url);
+
// Returns true if the model finished loading.
bool IsLoaded() { return loaded_; }
diff --git a/chrome/browser/bookmarks/bookmark_model_observer.h b/chrome/browser/bookmarks/bookmark_model_observer.h
index 2136475..7cc50dc 100644
--- a/chrome/browser/bookmarks/bookmark_model_observer.h
+++ b/chrome/browser/bookmarks/bookmark_model_observer.h
@@ -39,7 +39,7 @@ class BookmarkModelObserver {
int old_index,
const BookmarkNode* node) = 0;
- // Invoked when the title or favicon of a node has changed.
+ // Invoked when the title, url or favicon of a node has changed.
virtual void BookmarkNodeChanged(BookmarkModel* model,
const BookmarkNode* node) = 0;
diff --git a/chrome/browser/bookmarks/bookmark_model_unittest.cc b/chrome/browser/bookmarks/bookmark_model_unittest.cc
index 0624b74..2200f4e 100644
--- a/chrome/browser/bookmarks/bookmark_model_unittest.cc
+++ b/chrome/browser/bookmarks/bookmark_model_unittest.cc
@@ -264,6 +264,21 @@ TEST_F(BookmarkModelTest, SetTitle) {
EXPECT_EQ(title, node->GetTitle());
}
+TEST_F(BookmarkModelTest, SetURL) {
+ const BookmarkNode* root = model.GetBookmarkBarNode();
+ const std::wstring title(L"foo");
+ GURL url("http://foo.com");
+ const BookmarkNode* node = model.AddURL(root, 0, title, url);
+
+ ClearCounts();
+
+ url = GURL("http://foo2.com");
+ model.SetURL(node, url);
+ AssertObserverCount(0, 0, 0, 1, 0);
+ observer_details.AssertEquals(node, NULL, -1, -1);
+ EXPECT_EQ(url, node->GetURL());
+}
+
TEST_F(BookmarkModelTest, Move) {
const BookmarkNode* root = model.GetBookmarkBarNode();
std::wstring title(L"foo");
diff --git a/chrome/browser/bookmarks/bookmark_utils.cc b/chrome/browser/bookmarks/bookmark_utils.cc
index a172312..c6f82b3 100644
--- a/chrome/browser/bookmarks/bookmark_utils.cc
+++ b/chrome/browser/bookmarks/bookmark_utils.cc
@@ -529,24 +529,11 @@ const BookmarkNode* ApplyEditsWithNoGroupChange(BookmarkModel* model,
const BookmarkNode* node = details.existing_node;
DCHECK(node);
- const BookmarkNode* old_parent = node->GetParent();
- int old_index = old_parent ? old_parent->IndexOfChild(node) : -1;
- // If we're not showing the tree we only need to modify the node.
- if (old_index == -1) {
- NOTREACHED();
- return node;
- }
+ if (node->is_url())
+ model->SetURL(node, new_url);
+ model->SetTitle(node, new_title);
- if (new_url != node->GetURL()) {
- // TODO(sky): need SetURL on the model.
- const BookmarkNode* new_node = model->AddURLWithCreationTime(old_parent,
- old_index, new_title, new_url, node->date_added());
- model->Remove(old_parent, old_index + 1);
- return new_node;
- } else {
- model->SetTitle(node, new_title);
- }
return node;
}
@@ -562,31 +549,14 @@ const BookmarkNode* ApplyEditsWithPossibleGroupChange(BookmarkModel* model,
const BookmarkNode* node = details.existing_node;
DCHECK(node);
- const BookmarkNode* old_parent = node->GetParent();
- int old_index = old_parent->IndexOfChild(node);
- const BookmarkNode* return_node = node;
-
- Time date_added = node->date_added();
- if (new_parent == node->GetParent()) {
- // The parent is the same.
- if (node->is_url() && new_url != node->GetURL()) {
- model->Remove(old_parent, old_index);
- return_node = model->AddURLWithCreationTime(old_parent, old_index,
- new_title, new_url, date_added);
- } else {
- model->SetTitle(node, new_title);
- }
- } else if (node->is_url() && new_url != node->GetURL()) {
- // The parent and URL changed.
- model->Remove(old_parent, old_index);
- return_node = model->AddURLWithCreationTime(new_parent,
- new_parent->GetChildCount(), new_title, new_url, date_added);
- } else {
- // The parent and title changed. Move the node and change the title.
+
+ if (new_parent != node->GetParent())
model->Move(node, new_parent, new_parent->GetChildCount());
- model->SetTitle(node, new_title);
- }
- return return_node;
+ if (node->is_url())
+ model->SetURL(node, new_url);
+ model->SetTitle(node, new_title);
+
+ return node;
}
// Formerly in BookmarkBarView
diff --git a/chrome/browser/bookmarks/bookmark_utils.h b/chrome/browser/bookmarks/bookmark_utils.h
index 7117967..0bea122 100644
--- a/chrome/browser/bookmarks/bookmark_utils.h
+++ b/chrome/browser/bookmarks/bookmark_utils.h
@@ -149,9 +149,9 @@ bool DoesBookmarkContainText(const BookmarkNode* node,
const std::wstring& languages);
// Modifies a bookmark node (assuming that there's no magic that needs to be
-// done regarding moving from one folder to another). If the URL changed or a
-// new node is explicitly being added, returns a pointer to the new node that
-// was created. Otherwise the return value is identically |node|.
+// done regarding moving from one folder to another). If a new node is
+// explicitly being added, returns a pointer to the new node that was created.
+// Otherwise the return value is identically |node|.
const BookmarkNode* ApplyEditsWithNoGroupChange(
BookmarkModel* model,
const BookmarkNode* parent,
@@ -161,9 +161,9 @@ const BookmarkNode* ApplyEditsWithNoGroupChange(
BookmarkEditor::Handler* handler);
// Modifies a bookmark node assuming that the parent of the node may have
-// changed and the node will need to be removed and reinserted. If the URL
-// changed or a new node is explicitly being added, returns a pointer to the
-// new node that was created. Otherwise the return value is identically |node|.
+// changed and the node will need to be removed and reinserted. If a new node
+// is explicitly being added, returns a pointer to the new node that was
+// created. Otherwise the return value is identically |node|.
const BookmarkNode* ApplyEditsWithPossibleGroupChange(
BookmarkModel* model,
const BookmarkNode* new_parent,
diff --git a/chrome/browser/sync/glue/bookmark_change_processor.cc b/chrome/browser/sync/glue/bookmark_change_processor.cc
index 4b45a74..2bf9516 100644
--- a/chrome/browser/sync/glue/bookmark_change_processor.cc
+++ b/chrome/browser/sync/glue/bookmark_change_processor.cc
@@ -452,19 +452,10 @@ const BookmarkNode* BookmarkChangeProcessor::CreateOrUpdateBookmarkNode(
// Handle reparenting and/or repositioning.
model->Move(dst, parent, index);
- // Handle title update and URL changes due to possible conflict resolution
- // that can happen if both a local user change and server change occur
- // within a sufficiently small time interval.
- const BookmarkNode* old_dst = dst;
- dst = bookmark_utils::ApplyEditsWithNoGroupChange(model, parent,
- BookmarkEditor::EditDetails(dst),
- src->GetTitle(),
- src->GetIsFolder() ? GURL() : src->GetURL(),
- NULL); // NULL because we don't need a BookmarkEditor::Handler.
- if (dst != old_dst) { // dst was replaced with a new node with new URL.
- model_associator_->Disassociate(src->GetId());
- model_associator_->Associate(dst, src->GetId());
- }
+ if (!src->GetIsFolder())
+ model->SetURL(dst, src->GetURL());
+ model->SetTitle(dst, src->GetTitle());
+
SetBookmarkFavicon(src, dst, model->profile());
}
generated by cgit v1.1 at 2025-02-19 22:18:15 +0000