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

Side by Side Diff: components/bookmarks/browser/bookmark_model.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 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 unified diff | Download patch
OLDNEW
1 // Copyright 2014 The Chromium Authors. All rights reserved. 1 // Copyright 2014 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "components/bookmarks/browser/bookmark_model.h" 5 #include "components/bookmarks/browser/bookmark_model.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <functional> 8 #include <functional>
9 9
10 #include "base/bind.h" 10 #include "base/bind.h"
11 #include "base/bind_helpers.h" 11 #include "base/bind_helpers.h"
12 #include "base/i18n/string_compare.h" 12 #include "base/i18n/string_compare.h"
13 #include "base/logging.h" 13 #include "base/logging.h"
14 #include "base/macros.h" 14 #include "base/macros.h"
15 #include "base/metrics/histogram_macros.h" 15 #include "base/metrics/histogram_macros.h"
16 #include "base/profiler/scoped_tracker.h" 16 #include "base/profiler/scoped_tracker.h"
17 #include "base/strings/string_util.h" 17 #include "base/strings/string_util.h"
18 #include "base/tuple.h"
18 #include "components/bookmarks/browser/bookmark_expanded_state_tracker.h" 19 #include "components/bookmarks/browser/bookmark_expanded_state_tracker.h"
19 #include "components/bookmarks/browser/bookmark_index.h" 20 #include "components/bookmarks/browser/bookmark_index.h"
20 #include "components/bookmarks/browser/bookmark_match.h" 21 #include "components/bookmarks/browser/bookmark_match.h"
21 #include "components/bookmarks/browser/bookmark_model_observer.h" 22 #include "components/bookmarks/browser/bookmark_model_observer.h"
22 #include "components/bookmarks/browser/bookmark_node_data.h" 23 #include "components/bookmarks/browser/bookmark_node_data.h"
23 #include "components/bookmarks/browser/bookmark_storage.h" 24 #include "components/bookmarks/browser/bookmark_storage.h"
25 #include "components/bookmarks/browser/bookmark_undo_delegate.h"
24 #include "components/bookmarks/browser/bookmark_utils.h" 26 #include "components/bookmarks/browser/bookmark_utils.h"
25 #include "components/favicon_base/favicon_types.h" 27 #include "components/favicon_base/favicon_types.h"
26 #include "grit/components_strings.h" 28 #include "grit/components_strings.h"
27 #include "ui/base/l10n/l10n_util.h" 29 #include "ui/base/l10n/l10n_util.h"
28 #include "ui/gfx/favicon_size.h" 30 #include "ui/gfx/favicon_size.h"
29 31
30 using base::Time; 32 using base::Time;
31 33
32 namespace bookmarks { 34 namespace bookmarks {
33 35
(...skipping 48 matching lines...) Expand 10 before | Expand all | Expand 10 after
82 *collator_, n1->GetTitle(), n2->GetTitle()) == UCOL_LESS; 84 *collator_, n1->GetTitle(), n2->GetTitle()) == UCOL_LESS;
83 } 85 }
84 // Types differ, sort such that folders come first. 86 // Types differ, sort such that folders come first.
85 return n1->is_folder(); 87 return n1->is_folder();
86 } 88 }
87 89
88 private: 90 private:
89 icu::Collator* collator_; 91 icu::Collator* collator_;
90 }; 92 };
91 93
94 // Delegate that does nothing.
95 class EmptyUndoDelegate : public BookmarkUndoDelegate {
96 public:
97 EmptyUndoDelegate() {}
98 ~EmptyUndoDelegate() override {}
99
100 private:
101 // BookmarkUndoDelegate:
102 void SetUndoProvider(BookmarkUndoProvider* provider) override {}
103 void OnBookmarkNodeRemoved(BookmarkModel* model,
104 const BookmarkNode* parent,
105 int index,
106 scoped_ptr<BookmarkNode> node) override {}
107
108 DISALLOW_COPY_AND_ASSIGN(EmptyUndoDelegate);
109 };
110
92 } // namespace 111 } // namespace
93 112
94 // BookmarkModel -------------------------------------------------------------- 113 // BookmarkModel --------------------------------------------------------------
95 114
96 BookmarkModel::BookmarkModel(BookmarkClient* client) 115 BookmarkModel::BookmarkModel(BookmarkClient* client)
97 : client_(client), 116 : client_(client),
98 loaded_(false), 117 loaded_(false),
99 root_(GURL()), 118 root_(GURL()),
100 bookmark_bar_node_(NULL), 119 bookmark_bar_node_(NULL),
101 other_node_(NULL), 120 other_node_(NULL),
102 mobile_node_(NULL), 121 mobile_node_(NULL),
103 next_node_id_(1), 122 next_node_id_(1),
104 observers_( 123 observers_(
105 base::ObserverList<BookmarkModelObserver>::NOTIFY_EXISTING_ONLY), 124 base::ObserverList<BookmarkModelObserver>::NOTIFY_EXISTING_ONLY),
106 loaded_signal_(true, false), 125 loaded_signal_(true, false),
107 extensive_changes_(0) { 126 extensive_changes_(0),
127 undo_delegate_(nullptr),
128 empty_undo_delegate_(new EmptyUndoDelegate) {
108 DCHECK(client_); 129 DCHECK(client_);
109 } 130 }
110 131
111 BookmarkModel::~BookmarkModel() { 132 BookmarkModel::~BookmarkModel() {
112 FOR_EACH_OBSERVER(BookmarkModelObserver, observers_, 133 FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
113 BookmarkModelBeingDeleted(this)); 134 BookmarkModelBeingDeleted(this));
114 135
115 if (store_.get()) { 136 if (store_.get()) {
116 // The store maintains a reference back to us. We need to tell it we're gone 137 // The store maintains a reference back to us. We need to tell it we're gone
117 // so that it doesn't try and invoke a method back on us again. 138 // so that it doesn't try and invoke a method back on us again.
(...skipping 74 matching lines...) Expand 10 before | Expand all | Expand 10 after
192 213
193 void BookmarkModel::Remove(const BookmarkNode* node) { 214 void BookmarkModel::Remove(const BookmarkNode* node) {
194 DCHECK(loaded_); 215 DCHECK(loaded_);
195 DCHECK(node); 216 DCHECK(node);
196 DCHECK(!is_root_node(node)); 217 DCHECK(!is_root_node(node));
197 RemoveAndDeleteNode(AsMutable(node)); 218 RemoveAndDeleteNode(AsMutable(node));
198 } 219 }
199 220
200 void BookmarkModel::RemoveAllUserBookmarks() { 221 void BookmarkModel::RemoveAllUserBookmarks() {
201 std::set<GURL> removed_urls; 222 std::set<GURL> removed_urls;
202 ScopedVector<BookmarkNode> removed_nodes; 223 std::vector<base::Tuple<const BookmarkNode*, int, BookmarkNode*>>
sky 2015/10/09 16:01:19 Using a tuple certainly works, but it's not partic
jianli 2015/10/09 21:51:16 Done.
224 removed_node_data;
203 225
204 FOR_EACH_OBSERVER(BookmarkModelObserver, observers_, 226 FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
205 OnWillRemoveAllUserBookmarks(this)); 227 OnWillRemoveAllUserBookmarks(this));
206 228
207 BeginExtensiveChanges(); 229 BeginExtensiveChanges();
208 // Skip deleting permanent nodes. Permanent bookmark nodes are the root and 230 // Skip deleting permanent nodes. Permanent bookmark nodes are the root and
209 // its immediate children. For removing all non permanent nodes just remove 231 // its immediate children. For removing all non permanent nodes just remove
210 // all children of non-root permanent nodes. 232 // all children of non-root permanent nodes.
211 { 233 {
212 base::AutoLock url_lock(url_lock_); 234 base::AutoLock url_lock(url_lock_);
213 for (int i = 0; i < root_.child_count(); ++i) { 235 for (int i = 0; i < root_.child_count(); ++i) {
214 BookmarkNode* permanent_node = root_.GetChild(i); 236 const BookmarkNode* permanent_node = root_.GetChild(i);
215 237
216 if (!client_->CanBeEditedByUser(permanent_node)) 238 if (!client_->CanBeEditedByUser(permanent_node))
217 continue; 239 continue;
218 240
219 for (int j = permanent_node->child_count() - 1; j >= 0; --j) { 241 for (int j = permanent_node->child_count() - 1; j >= 0; --j) {
220 BookmarkNode* child_node = permanent_node->GetChild(j); 242 BookmarkNode* child_node = AsMutable(permanent_node->GetChild(j));
221 removed_nodes.push_back(child_node);
222 RemoveNodeAndGetRemovedUrls(child_node, &removed_urls); 243 RemoveNodeAndGetRemovedUrls(child_node, &removed_urls);
244 removed_node_data.push_back(
245 base::MakeTuple(permanent_node, j, child_node));
223 } 246 }
224 } 247 }
225 } 248 }
226 EndExtensiveChanges(); 249 EndExtensiveChanges();
227 if (store_.get()) 250 if (store_.get())
228 store_->ScheduleSave(); 251 store_->ScheduleSave();
229 252
230 FOR_EACH_OBSERVER(BookmarkModelObserver, observers_, 253 FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
231 BookmarkAllUserNodesRemoved(this, removed_urls)); 254 BookmarkAllUserNodesRemoved(this, removed_urls));
255
256 BeginGroupedChanges();
257 for (const auto& tuple : removed_node_data) {
258 undo_delegate()->OnBookmarkNodeRemoved(
259 this,
260 base::get<0>(tuple),
261 base::get<1>(tuple),
262 scoped_ptr<BookmarkNode>(base::get<2>(tuple)));
263 }
264 EndGroupedChanges();
232 } 265 }
233 266
234 void BookmarkModel::Move(const BookmarkNode* node, 267 void BookmarkModel::Move(const BookmarkNode* node,
235 const BookmarkNode* new_parent, 268 const BookmarkNode* new_parent,
236 int index) { 269 int index) {
237 if (!loaded_ || !node || !IsValidIndex(new_parent, index, true) || 270 if (!loaded_ || !node || !IsValidIndex(new_parent, index, true) ||
238 is_root_node(new_parent) || is_permanent_node(node)) { 271 is_root_node(new_parent) || is_permanent_node(node)) {
239 NOTREACHED(); 272 NOTREACHED();
240 return; 273 return;
241 } 274 }
(...skipping 469 matching lines...) Expand 10 before | Expand all | Expand 10 after
711 case BookmarkNode::OTHER_NODE: 744 case BookmarkNode::OTHER_NODE:
712 return other_node_; 745 return other_node_;
713 case BookmarkNode::MOBILE: 746 case BookmarkNode::MOBILE:
714 return mobile_node_; 747 return mobile_node_;
715 default: 748 default:
716 NOTREACHED(); 749 NOTREACHED();
717 return NULL; 750 return NULL;
718 } 751 }
719 } 752 }
720 753
754 void BookmarkModel::RestoreRemovedNode(const BookmarkNode* parent,
755 int index,
756 scoped_ptr<BookmarkNode> scoped_node) {
757 BookmarkNode* node = scoped_node.release();
758 AddNode(AsMutable(parent), index, node);
759
760 // We might be restoring a folder node that have already contained a set of
761 // child nodes. We need to notify all of them.
762 NotifyNodeAddedForAllDescendents(node);
763 }
764
765 void BookmarkModel::NotifyNodeAddedForAllDescendents(const BookmarkNode* node) {
766 for (int i = 0; i < node->child_count(); ++i) {
767 FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
768 BookmarkNodeAdded(this, node, i));
769 NotifyNodeAddedForAllDescendents(node->GetChild(i));
770 }
771 }
772
721 bool BookmarkModel::IsBookmarkedNoLock(const GURL& url) { 773 bool BookmarkModel::IsBookmarkedNoLock(const GURL& url) {
722 BookmarkNode tmp_node(url); 774 BookmarkNode tmp_node(url);
723 return (nodes_ordered_by_url_set_.find(&tmp_node) != 775 return (nodes_ordered_by_url_set_.find(&tmp_node) !=
724 nodes_ordered_by_url_set_.end()); 776 nodes_ordered_by_url_set_.end());
725 } 777 }
726 778
727 void BookmarkModel::RemoveNode(BookmarkNode* node, 779 void BookmarkModel::RemoveNode(BookmarkNode* node,
728 std::set<GURL>* removed_urls) { 780 std::set<GURL>* removed_urls) {
729 if (!loaded_ || !node || is_permanent_node(node)) { 781 if (!loaded_ || !node || is_permanent_node(node)) {
730 NOTREACHED(); 782 NOTREACHED();
(...skipping 121 matching lines...) Expand 10 before | Expand all | Expand 10 after
852 RemoveNodeAndGetRemovedUrls(node.get(), &removed_urls); 904 RemoveNodeAndGetRemovedUrls(node.get(), &removed_urls);
853 } 905 }
854 906
855 if (store_.get()) 907 if (store_.get())
856 store_->ScheduleSave(); 908 store_->ScheduleSave();
857 909
858 FOR_EACH_OBSERVER( 910 FOR_EACH_OBSERVER(
859 BookmarkModelObserver, 911 BookmarkModelObserver,
860 observers_, 912 observers_,
861 BookmarkNodeRemoved(this, parent, index, node.get(), removed_urls)); 913 BookmarkNodeRemoved(this, parent, index, node.get(), removed_urls));
914
915 undo_delegate()->OnBookmarkNodeRemoved(this, parent, index, node.Pass());
862 } 916 }
863 917
864 void BookmarkModel::RemoveNodeFromInternalMaps(BookmarkNode* node) { 918 void BookmarkModel::RemoveNodeFromInternalMaps(BookmarkNode* node) {
865 index_->Remove(node); 919 index_->Remove(node);
866 // NOTE: this is called in such a way that url_lock_ is already held. As 920 // NOTE: this is called in such a way that url_lock_ is already held. As
867 // such, this doesn't explicitly grab the lock. 921 // such, this doesn't explicitly grab the lock.
868 url_lock_.AssertAcquired(); 922 url_lock_.AssertAcquired();
869 NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.find(node); 923 NodesOrderedByURLSet::iterator i = nodes_ordered_by_url_set_.find(node);
870 DCHECK(i != nodes_ordered_by_url_set_.end()); 924 DCHECK(i != nodes_ordered_by_url_set_.end());
871 // i points to the first node with the URL, advance until we find the 925 // i points to the first node with the URL, advance until we find the
(...skipping 30 matching lines...) Expand all
902 } 956 }
903 957
904 BookmarkNode* BookmarkModel::AddNode(BookmarkNode* parent, 958 BookmarkNode* BookmarkModel::AddNode(BookmarkNode* parent,
905 int index, 959 int index,
906 BookmarkNode* node) { 960 BookmarkNode* node) {
907 parent->Add(node, index); 961 parent->Add(node, index);
908 962
909 if (store_.get()) 963 if (store_.get())
910 store_->ScheduleSave(); 964 store_->ScheduleSave();
911 965
912 if (node->type() == BookmarkNode::URL) { 966 {
913 base::AutoLock url_lock(url_lock_); 967 base::AutoLock url_lock(url_lock_);
914 AddNodeToInternalMaps(node); 968 AddNodeToInternalMaps(node);
915 } else {
916 index_->Add(node);
917 } 969 }
918 970
919 FOR_EACH_OBSERVER(BookmarkModelObserver, observers_, 971 FOR_EACH_OBSERVER(BookmarkModelObserver, observers_,
920 BookmarkNodeAdded(this, parent, index)); 972 BookmarkNodeAdded(this, parent, index));
921 973
922 return node; 974 return node;
923 } 975 }
924 976
925 void BookmarkModel::AddNodeToInternalMaps(BookmarkNode* node) { 977 void BookmarkModel::AddNodeToInternalMaps(BookmarkNode* node) {
926 index_->Add(node);
927 url_lock_.AssertAcquired(); 978 url_lock_.AssertAcquired();
928 nodes_ordered_by_url_set_.insert(node); 979 if (node->is_url()) {
980 index_->Add(node);
981 nodes_ordered_by_url_set_.insert(node);
982 }
983 for (int i = 0; i < node->child_count(); ++i)
984 AddNodeToInternalMaps(node->GetChild(i));
929 } 985 }
930 986
931 bool BookmarkModel::IsValidIndex(const BookmarkNode* parent, 987 bool BookmarkModel::IsValidIndex(const BookmarkNode* parent,
932 int index, 988 int index,
933 bool allow_end) { 989 bool allow_end) {
934 return (parent && parent->is_folder() && 990 return (parent && parent->is_folder() &&
935 (index >= 0 && (index < parent->child_count() || 991 (index >= 0 && (index < parent->child_count() ||
936 (allow_end && index == parent->child_count())))); 992 (allow_end && index == parent->child_count()))));
937 } 993 }
938 994
(...skipping 102 matching lines...) Expand 10 before | Expand all | Expand 10 after
1041 CreatePermanentNode(BookmarkNode::MOBILE); 1097 CreatePermanentNode(BookmarkNode::MOBILE);
1042 return scoped_ptr<BookmarkLoadDetails>(new BookmarkLoadDetails( 1098 return scoped_ptr<BookmarkLoadDetails>(new BookmarkLoadDetails(
1043 bb_node, 1099 bb_node,
1044 other_node, 1100 other_node,
1045 mobile_node, 1101 mobile_node,
1046 client_->GetLoadExtraNodesCallback(), 1102 client_->GetLoadExtraNodesCallback(),
1047 new BookmarkIndex(client_, accept_languages), 1103 new BookmarkIndex(client_, accept_languages),
1048 next_node_id_)); 1104 next_node_id_));
1049 } 1105 }
1050 1106
1107 void BookmarkModel::set_undo_delegate(BookmarkUndoDelegate* undo_delegate) {
1108 undo_delegate_ = undo_delegate;
1109 if (undo_delegate_)
1110 undo_delegate_->SetUndoProvider(this);
1111 }
1112
1113 BookmarkUndoDelegate* BookmarkModel::undo_delegate() const {
1114 return undo_delegate_ ? undo_delegate_ : empty_undo_delegate_.get();
1115 }
1116
1051 } // namespace bookmarks 1117 } // namespace bookmarks
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698