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

Unified Diff: components/undo/bookmark_undo_service.cc

Issue 1379983002: Supporting undoing bookmark deletion without creating new ID. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address some more feedback Created 5 years, 2 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
« no previous file with comments | « components/undo/bookmark_undo_service.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/undo/bookmark_undo_service.cc
diff --git a/components/undo/bookmark_undo_service.cc b/components/undo/bookmark_undo_service.cc
index 28fd451d7b080d5f6fe434629ab07f14c538ee20..0a348b5fda64f2e31fb1e10a11555e1cf8a5ec8b 100644
--- a/components/undo/bookmark_undo_service.cc
+++ b/components/undo/bookmark_undo_service.cc
@@ -6,15 +6,16 @@
#include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/browser/bookmark_node_data.h"
+#include "components/bookmarks/browser/bookmark_undo_provider.h"
#include "components/bookmarks/browser/bookmark_utils.h"
#include "components/bookmarks/browser/scoped_group_bookmark_actions.h"
-#include "components/undo/bookmark_renumber_observer.h"
#include "components/undo/undo_operation.h"
#include "grit/components_strings.h"
using bookmarks::BookmarkModel;
using bookmarks::BookmarkNode;
using bookmarks::BookmarkNodeData;
+using bookmarks::BookmarkUndoProvider;
namespace {
@@ -22,24 +23,16 @@ namespace {
// Base class for all bookmark related UndoOperations that facilitates access to
// the BookmarkUndoService.
-class BookmarkUndoOperation : public UndoOperation,
- public BookmarkRenumberObserver {
+class BookmarkUndoOperation : public UndoOperation {
public:
- BookmarkUndoOperation(BookmarkModel* bookmark_model,
- BookmarkRenumberObserver* undo_renumber_observer)
- : bookmark_model_(bookmark_model),
- undo_renumber_observer_(undo_renumber_observer) {}
+ explicit BookmarkUndoOperation(BookmarkModel* bookmark_model)
+ : bookmark_model_(bookmark_model) {}
~BookmarkUndoOperation() override {}
BookmarkModel* bookmark_model() { return bookmark_model_; }
- BookmarkRenumberObserver* undo_renumber_observer() {
- return undo_renumber_observer_;
- }
-
private:
BookmarkModel* bookmark_model_;
- BookmarkRenumberObserver* undo_renumber_observer_;
};
// BookmarkAddOperation -------------------------------------------------------
@@ -48,7 +41,6 @@ class BookmarkUndoOperation : public UndoOperation,
class BookmarkAddOperation : public BookmarkUndoOperation {
public:
BookmarkAddOperation(BookmarkModel* bookmark_model,
- BookmarkRenumberObserver* undo_renumber_observer,
const BookmarkNode* parent,
int index);
~BookmarkAddOperation() override {}
@@ -58,9 +50,6 @@ class BookmarkAddOperation : public BookmarkUndoOperation {
int GetUndoLabelId() const override;
int GetRedoLabelId() const override;
- // BookmarkRenumberObserver:
- void OnBookmarkRenumbered(int64 old_id, int64 new_id) override;
-
private:
int64 parent_id_;
const int index_;
@@ -70,10 +59,9 @@ class BookmarkAddOperation : public BookmarkUndoOperation {
BookmarkAddOperation::BookmarkAddOperation(
BookmarkModel* bookmark_model,
- BookmarkRenumberObserver* undo_renumber_observer,
const BookmarkNode* parent,
int index)
- : BookmarkUndoOperation(bookmark_model, undo_renumber_observer),
+ : BookmarkUndoOperation(bookmark_model),
parent_id_(parent->id()),
index_(index) {
}
@@ -95,11 +83,6 @@ int BookmarkAddOperation::GetRedoLabelId() const {
return IDS_BOOKMARK_BAR_REDO_DELETE;
}
-void BookmarkAddOperation::OnBookmarkRenumbered(int64 old_id, int64 new_id) {
- if (parent_id_ == old_id)
- parent_id_ = new_id;
-}
-
// BookmarkRemoveOperation ----------------------------------------------------
// Handles the undo of the deletion of a bookmark node. For a bookmark folder,
@@ -108,55 +91,51 @@ void BookmarkAddOperation::OnBookmarkRenumbered(int64 old_id, int64 new_id) {
// The BookmarkModel allows only single bookmark node to be removed.
class BookmarkRemoveOperation : public BookmarkUndoOperation {
public:
- BookmarkRemoveOperation(BookmarkModel* bookmark_model,
- BookmarkRenumberObserver* undo_renumber_observer,
+ BookmarkRemoveOperation(BookmarkModel* model,
+ BookmarkUndoProvider* undo_provider,
const BookmarkNode* parent,
- int old_index,
- const BookmarkNode* node);
- ~BookmarkRemoveOperation() override {}
+ int index,
+ scoped_ptr<BookmarkNode> node);
+ ~BookmarkRemoveOperation() override;
// UndoOperation:
void Undo() override;
int GetUndoLabelId() const override;
int GetRedoLabelId() const override;
- // BookmarkRenumberObserver:
- void OnBookmarkRenumbered(int64 old_id, int64 new_id) override;
-
private:
- void UpdateBookmarkIds(const BookmarkNodeData::Element& element,
- const BookmarkNode* parent,
- int index_added_at);
-
- int64 parent_id_;
- const int old_index_;
- BookmarkNodeData removed_node_;
+ BookmarkUndoProvider* undo_provider_;
+ const int64_t parent_node_id_;
+ const int index_;
+ scoped_ptr<BookmarkNode> node_;
DISALLOW_COPY_AND_ASSIGN(BookmarkRemoveOperation);
};
BookmarkRemoveOperation::BookmarkRemoveOperation(
- BookmarkModel* bookmark_model,
- BookmarkRenumberObserver* undo_renumber_observer,
+ BookmarkModel* model,
+ BookmarkUndoProvider* undo_provider,
const BookmarkNode* parent,
- int old_index,
- const BookmarkNode* node)
- : BookmarkUndoOperation(bookmark_model, undo_renumber_observer),
- parent_id_(parent->id()),
- old_index_(old_index),
- removed_node_(node) {
+ int index,
+ scoped_ptr<BookmarkNode> node)
+ : BookmarkUndoOperation(model),
+ undo_provider_(undo_provider),
+ parent_node_id_(parent->id()),
+ index_(index),
+ node_(node.Pass()) {
+}
+
+BookmarkRemoveOperation::~BookmarkRemoveOperation() {
}
void BookmarkRemoveOperation::Undo() {
- DCHECK(removed_node_.is_valid());
- BookmarkModel* model = bookmark_model();
- const BookmarkNode* parent =
- bookmarks::GetBookmarkNodeByID(model, parent_id_);
+ DCHECK(node_.get());
+
+ const BookmarkNode* parent = bookmarks::GetBookmarkNodeByID(
+ bookmark_model(), parent_node_id_);
DCHECK(parent);
- bookmarks::CloneBookmarkNode(
- model, removed_node_.elements, parent, old_index_, false);
- UpdateBookmarkIds(removed_node_.elements[0], parent, old_index_);
+ undo_provider_->RestoreRemovedNode(parent, index_, node_.Pass());
}
int BookmarkRemoveOperation::GetUndoLabelId() const {
@@ -167,31 +146,12 @@ int BookmarkRemoveOperation::GetRedoLabelId() const {
return IDS_BOOKMARK_BAR_REDO_ADD;
}
-void BookmarkRemoveOperation::UpdateBookmarkIds(
- const BookmarkNodeData::Element& element,
- const BookmarkNode* parent,
- int index_added_at) {
- const BookmarkNode* node = parent->GetChild(index_added_at);
- if (element.id() != node->id())
- undo_renumber_observer()->OnBookmarkRenumbered(element.id(), node->id());
- if (!element.is_url) {
- for (int i = 0; i < static_cast<int>(element.children.size()); ++i)
- UpdateBookmarkIds(element.children[i], node, i);
- }
-}
-
-void BookmarkRemoveOperation::OnBookmarkRenumbered(int64 old_id, int64 new_id) {
- if (parent_id_ == old_id)
- parent_id_ = new_id;
-}
-
// BookmarkEditOperation ------------------------------------------------------
// Handles the undo of the modification of a bookmark node.
class BookmarkEditOperation : public BookmarkUndoOperation {
public:
BookmarkEditOperation(BookmarkModel* bookmark_model,
- BookmarkRenumberObserver* undo_renumber_observer,
const BookmarkNode* node);
~BookmarkEditOperation() override {}
@@ -200,9 +160,6 @@ class BookmarkEditOperation : public BookmarkUndoOperation {
int GetUndoLabelId() const override;
int GetRedoLabelId() const override;
- // BookmarkRenumberObserver:
- void OnBookmarkRenumbered(int64 old_id, int64 new_id) override;
-
private:
int64 node_id_;
BookmarkNodeData original_bookmark_;
@@ -212,9 +169,8 @@ class BookmarkEditOperation : public BookmarkUndoOperation {
BookmarkEditOperation::BookmarkEditOperation(
BookmarkModel* bookmark_model,
- BookmarkRenumberObserver* undo_renumber_observer,
const BookmarkNode* node)
- : BookmarkUndoOperation(bookmark_model, undo_renumber_observer),
+ : BookmarkUndoOperation(bookmark_model),
node_id_(node->id()),
original_bookmark_(node) {
}
@@ -238,18 +194,12 @@ int BookmarkEditOperation::GetRedoLabelId() const {
return IDS_BOOKMARK_BAR_REDO_EDIT;
}
-void BookmarkEditOperation::OnBookmarkRenumbered(int64 old_id, int64 new_id) {
- if (node_id_ == old_id)
- node_id_ = new_id;
-}
-
// BookmarkMoveOperation ------------------------------------------------------
// Handles the undo of a bookmark being moved to a new location.
class BookmarkMoveOperation : public BookmarkUndoOperation {
public:
BookmarkMoveOperation(BookmarkModel* bookmark_model,
- BookmarkRenumberObserver* undo_renumber_observer,
const BookmarkNode* old_parent,
int old_index,
const BookmarkNode* new_parent,
@@ -261,9 +211,6 @@ class BookmarkMoveOperation : public BookmarkUndoOperation {
// UndoOperation:
void Undo() override;
- // BookmarkRenumberObserver:
- void OnBookmarkRenumbered(int64 old_id, int64 new_id) override;
-
private:
int64 old_parent_id_;
int64 new_parent_id_;
@@ -275,12 +222,11 @@ class BookmarkMoveOperation : public BookmarkUndoOperation {
BookmarkMoveOperation::BookmarkMoveOperation(
BookmarkModel* bookmark_model,
- BookmarkRenumberObserver* undo_renumber_observer,
const BookmarkNode* old_parent,
int old_index,
const BookmarkNode* new_parent,
int new_index)
- : BookmarkUndoOperation(bookmark_model, undo_renumber_observer),
+ : BookmarkUndoOperation(bookmark_model),
old_parent_id_(old_parent->id()),
new_parent_id_(new_parent->id()),
old_index_(old_index),
@@ -316,13 +262,6 @@ int BookmarkMoveOperation::GetRedoLabelId() const {
return IDS_BOOKMARK_BAR_REDO_MOVE;
}
-void BookmarkMoveOperation::OnBookmarkRenumbered(int64 old_id, int64 new_id) {
- if (old_parent_id_ == old_id)
- old_parent_id_ = new_id;
- if (new_parent_id_ == old_id)
- new_parent_id_ = new_id;
-}
-
// BookmarkReorderOperation ---------------------------------------------------
// Handle the undo of reordering of bookmarks that can happen as a result of
@@ -332,7 +271,6 @@ void BookmarkMoveOperation::OnBookmarkRenumbered(int64 old_id, int64 new_id) {
class BookmarkReorderOperation : public BookmarkUndoOperation {
public:
BookmarkReorderOperation(BookmarkModel* bookmark_model,
- BookmarkRenumberObserver* undo_renumber_observer,
const BookmarkNode* parent);
~BookmarkReorderOperation() override;
@@ -341,9 +279,6 @@ class BookmarkReorderOperation : public BookmarkUndoOperation {
int GetUndoLabelId() const override;
int GetRedoLabelId() const override;
- // BookmarkRenumberObserver:
- void OnBookmarkRenumbered(int64 old_id, int64 new_id) override;
-
private:
int64 parent_id_;
std::vector<int64> ordered_bookmarks_;
@@ -353,9 +288,8 @@ class BookmarkReorderOperation : public BookmarkUndoOperation {
BookmarkReorderOperation::BookmarkReorderOperation(
BookmarkModel* bookmark_model,
- BookmarkRenumberObserver* undo_renumber_observer,
const BookmarkNode* parent)
- : BookmarkUndoOperation(bookmark_model, undo_renumber_observer),
+ : BookmarkUndoOperation(bookmark_model),
parent_id_(parent->id()) {
ordered_bookmarks_.resize(parent->child_count());
for (int i = 0; i < parent->child_count(); ++i)
@@ -388,32 +322,28 @@ int BookmarkReorderOperation::GetRedoLabelId() const {
return IDS_BOOKMARK_BAR_REDO_REORDER;
}
-void BookmarkReorderOperation::OnBookmarkRenumbered(int64 old_id,
- int64 new_id) {
- if (parent_id_ == old_id)
- parent_id_ = new_id;
- for (size_t i = 0; i < ordered_bookmarks_.size(); ++i) {
- if (ordered_bookmarks_[i] == old_id)
- ordered_bookmarks_[i] = new_id;
- }
-}
-
} // namespace
// BookmarkUndoService --------------------------------------------------------
-BookmarkUndoService::BookmarkUndoService() : scoped_observer_(this) {
+BookmarkUndoService::BookmarkUndoService()
+ : model_(nullptr), scoped_observer_(this) {
}
BookmarkUndoService::~BookmarkUndoService() {
}
void BookmarkUndoService::Start(BookmarkModel* model) {
+ DCHECK(!model_);
+ model_ = model;
scoped_observer_.Add(model);
+ model->SetUndoDelegate(this);
}
void BookmarkUndoService::Shutdown() {
+ DCHECK(model_);
scoped_observer_.RemoveAll();
+ model_->SetUndoDelegate(nullptr);
}
void BookmarkUndoService::BookmarkModelLoaded(BookmarkModel* model,
@@ -431,7 +361,7 @@ void BookmarkUndoService::BookmarkNodeMoved(BookmarkModel* model,
const BookmarkNode* new_parent,
int new_index) {
scoped_ptr<UndoOperation> op(new BookmarkMoveOperation(
- model, this, old_parent, old_index, new_parent, new_index));
+ model, old_parent, old_index, new_parent, new_index));
undo_manager()->AddUndoOperation(op.Pass());
}
@@ -439,40 +369,19 @@ void BookmarkUndoService::BookmarkNodeAdded(BookmarkModel* model,
const BookmarkNode* parent,
int index) {
scoped_ptr<UndoOperation> op(
- new BookmarkAddOperation(model, this, parent, index));
+ new BookmarkAddOperation(model, parent, index));
undo_manager()->AddUndoOperation(op.Pass());
}
-void BookmarkUndoService::OnWillRemoveBookmarks(BookmarkModel* model,
- const BookmarkNode* parent,
- int old_index,
- const BookmarkNode* node) {
- scoped_ptr<UndoOperation> op(
- new BookmarkRemoveOperation(model, this, parent, old_index, node));
- undo_manager()->AddUndoOperation(op.Pass());
-}
-
-void BookmarkUndoService::OnWillRemoveAllUserBookmarks(BookmarkModel* model) {
- bookmarks::ScopedGroupBookmarkActions merge_removes(model);
- for (int i = 0; i < model->root_node()->child_count(); ++i) {
- const BookmarkNode* permanent_node = model->root_node()->GetChild(i);
- for (int j = permanent_node->child_count() - 1; j >= 0; --j) {
- scoped_ptr<UndoOperation> op(new BookmarkRemoveOperation(
- model, this, permanent_node, j, permanent_node->GetChild(j)));
- undo_manager()->AddUndoOperation(op.Pass());
- }
- }
-}
-
void BookmarkUndoService::OnWillChangeBookmarkNode(BookmarkModel* model,
const BookmarkNode* node) {
- scoped_ptr<UndoOperation> op(new BookmarkEditOperation(model, this, node));
+ scoped_ptr<UndoOperation> op(new BookmarkEditOperation(model, node));
undo_manager()->AddUndoOperation(op.Pass());
}
void BookmarkUndoService::OnWillReorderBookmarkNode(BookmarkModel* model,
const BookmarkNode* node) {
- scoped_ptr<UndoOperation> op(new BookmarkReorderOperation(model, this, node));
+ scoped_ptr<UndoOperation> op(new BookmarkReorderOperation(model, node));
undo_manager()->AddUndoOperation(op.Pass());
}
@@ -485,11 +394,16 @@ void BookmarkUndoService::GroupedBookmarkChangesEnded(BookmarkModel* model) {
undo_manager()->EndGroupingActions();
}
-void BookmarkUndoService::OnBookmarkRenumbered(int64 old_id, int64 new_id) {
- std::vector<UndoOperation*> all_operations =
- undo_manager()->GetAllUndoOperations();
- for (UndoOperation* op : all_operations) {
- static_cast<BookmarkUndoOperation*>(op)
- ->OnBookmarkRenumbered(old_id, new_id);
- }
+void BookmarkUndoService::SetUndoProvider(BookmarkUndoProvider* undo_provider) {
+ undo_provider_ = undo_provider;
+}
+
+void BookmarkUndoService::OnBookmarkNodeRemoved(BookmarkModel* model,
+ const BookmarkNode* parent,
+ int index,
+ scoped_ptr<BookmarkNode> node) {
+ DCHECK(undo_provider_);
+ scoped_ptr<UndoOperation> op(new BookmarkRemoveOperation(
+ model, undo_provider_, parent, index, node.Pass()));
+ undo_manager()->AddUndoOperation(op.Pass());
}
« no previous file with comments | « components/undo/bookmark_undo_service.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698