From e73144496772d05fd9b96f2c887e850815d29ceb Mon Sep 17 00:00:00 2001 From: Anthony Tseng Date: Tue, 7 Jan 2020 18:06:44 -0800 Subject: [PATCH 1/5] USSBookmarks migration --- app/brave_main_delegate.cc | 1 - .../bookmark_change_processor.cc | 100 ----------------- .../bookmark_model_type_processor.cc | 9 ++ .../bookmark_remote_updates_handler.cc | 30 ++++++ .../bookmark_specifics_conversions.cc | 25 +++++ components/brave_sync/syncer_helper.cc | 102 +++++------------- components/brave_sync/syncer_helper.h | 14 +-- .../brave_bookmark_model_observer_impl.cc | 31 ++++++ .../brave_bookmark_model_observer_impl.h | 33 ++++++ .../components-sync_bookmarks-BUILD.gn.patch | 13 +++ ...okmarks-bookmark_change_processor.cc.patch | 52 --------- ...s-bookmark_remote_updates_handler.cc.patch | 27 +++++ 12 files changed, 202 insertions(+), 235 deletions(-) delete mode 100644 chromium_src/components/sync_bookmarks/bookmark_change_processor.cc create mode 100644 chromium_src/components/sync_bookmarks/bookmark_model_type_processor.cc create mode 100644 chromium_src/components/sync_bookmarks/bookmark_remote_updates_handler.cc create mode 100644 chromium_src/components/sync_bookmarks/bookmark_specifics_conversions.cc create mode 100644 components/sync_bookmarks/brave_bookmark_model_observer_impl.cc create mode 100644 components/sync_bookmarks/brave_bookmark_model_observer_impl.h create mode 100644 patches/components-sync_bookmarks-BUILD.gn.patch delete mode 100644 patches/components-sync_bookmarks-bookmark_change_processor.cc.patch create mode 100644 patches/components-sync_bookmarks-bookmark_remote_updates_handler.cc.patch diff --git a/app/brave_main_delegate.cc b/app/brave_main_delegate.cc index ac68d14fb9c7..a8064abb0665 100644 --- a/app/brave_main_delegate.cc +++ b/app/brave_main_delegate.cc @@ -173,7 +173,6 @@ bool BraveMainDelegate::BasicStartupComplete(int* exit_code) { features::kWebXr.name, features::kWebXrGamepadModule.name, unified_consent::kUnifiedConsent.name, - switches::kSyncUSSBookmarks.name, }; command_line.AppendFeatures(enabled_features, disabled_features); diff --git a/chromium_src/components/sync_bookmarks/bookmark_change_processor.cc b/chromium_src/components/sync_bookmarks/bookmark_change_processor.cc deleted file mode 100644 index 939c12d14c17..000000000000 --- a/chromium_src/components/sync_bookmarks/bookmark_change_processor.cc +++ /dev/null @@ -1,100 +0,0 @@ -/* Copyright (c) 2019 The Brave Authors. All rights reserved. - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include "components/sync_bookmarks/bookmark_change_processor.h" - -#include "brave/components/brave_sync/syncer_helper.h" -#include "components/bookmarks/browser/bookmark_model.h" -#include "components/sync/syncable/base_transaction.h" -#include "components/sync/syncable/syncable_base_transaction.h" -#include "components/sync/syncable/syncable_write_transaction.h" -#include "components/sync/syncable/write_node.h" -#include "components/sync/syncable/write_transaction.h" - -using bookmarks::BookmarkModel; -using bookmarks::BookmarkModelObserver; -using bookmarks::BookmarkNode; -using sync_bookmarks::BookmarkChangeProcessor; - -namespace { - -class ScopedPauseObserver { - public: - explicit ScopedPauseObserver(BookmarkModel* model, - BookmarkModelObserver* observer) - : model_(model), observer_(observer) { - DCHECK_NE(observer_, nullptr); - DCHECK_NE(model_, nullptr); - model_->RemoveObserver(observer_); - } - ~ScopedPauseObserver() { model_->AddObserver(observer_); } - - private: - BookmarkModel* model_; // Not owned - BookmarkModelObserver* observer_; // Not owned -}; - -} // namespace - -namespace sync_bookmarks { - -void BookmarkChangeProcessor::MoveSyncNode( - int index, - const bookmarks::BookmarkNode* node, - const syncer::BaseTransaction* trans) { - syncer::WriteTransaction write_trans( - FROM_HERE, trans->GetUserShare(), - static_cast( - trans->GetWrappedTrans())); - syncer::WriteNode sync_node(&write_trans); - if (!model_associator_->InitSyncNodeFromChromeId(node->id(), &sync_node)) { - syncer::SyncError error(FROM_HERE, syncer::SyncError::DATATYPE_ERROR, - "Failed to init sync node from chrome node", - syncer::BOOKMARKS); - error_handler()->OnUnrecoverableError(error); - return; - } - - if (!PlaceSyncNode(MOVE, node->parent(), index, &write_trans, &sync_node, - model_associator_)) { - syncer::SyncError error(FROM_HERE, syncer::SyncError::DATATYPE_ERROR, - "Failed to place sync node", syncer::BOOKMARKS); - error_handler()->OnUnrecoverableError(error); - return; - } -} - -} // namespace sync_bookmarks - -#define BRAVE_BOOKMARK_CHANGE_PROCESSOR_BOOKMARK_NODE_FAVICON_CHANGED \ - return; - -#define BRAVE_BOOKMARK_CHANGE_PROCESSOR_UPDATE_SYNC_NODE_PROPERTIES \ - brave_sync::AddBraveMetaInfo(src, model); - -#define BRAVE_BOOKMARK_CHANGE_PROCESSOR_BOOKMARK_NODE_MOVED \ - ScopedPauseObserver pause(bookmark_model_, this); \ - brave_sync::AddBraveMetaInfo(child, model); \ - SetSyncNodeMetaInfo(child, &sync_node); - -#define BRAVE_BOOKMARK_CHANGE_PROCESSOR_CHILDREN_REORDERED \ - ScopedPauseObserver pause(bookmark_model_, this); \ - brave_sync::AddBraveMetaInfo(child, model); \ - SetSyncNodeMetaInfo(child, &sync_child); - -#define BRAVE_BOOKMARK_CHANGE_PROCESSOR_APPLY_CHANGES_FROM_SYNC_MODEL \ - int new_index = \ - brave_sync::GetIndexByCompareOrderStartFrom(parent, it->second, 0); \ - if (it->first != new_index) { \ - model->Move(it->second, parent, new_index); \ - MoveSyncNode(new_index, it->second, trans); \ - } else // NOLINT - -#include "../../../../components/sync_bookmarks/bookmark_change_processor.cc" // NOLINT -#undef BRAVE_BOOKMARK_CHANGE_PROCESSOR_BOOKMARK_NODE_FAVICON_CHANGED -#undef BRAVE_BOOKMARK_CHANGE_PROCESSOR_UPDATE_SYNC_NODE_PROPERTIES -#undef BRAVE_BOOKMARK_CHANGE_PROCESSOR_BOOKMARK_NODE_MOVED -#undef BRAVE_BOOKMARK_CHANGE_PROCESSOR_CHILDREN_REORDERED -#undef BRAVE_BOOKMARK_CHANGE_PROCESSOR_APPLY_CHANGES_FROM_SYNC_MODEL diff --git a/chromium_src/components/sync_bookmarks/bookmark_model_type_processor.cc b/chromium_src/components/sync_bookmarks/bookmark_model_type_processor.cc new file mode 100644 index 000000000000..99b81d6b6887 --- /dev/null +++ b/chromium_src/components/sync_bookmarks/bookmark_model_type_processor.cc @@ -0,0 +1,9 @@ +/* Copyright 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "brave/components/sync_bookmarks/brave_bookmark_model_observer_impl.h" +#define BookmarkModelObserverImpl BraveBookmarkModelObserverImpl +#include "../../../../components/sync_bookmarks/bookmark_model_type_processor.cc" +#undef BookmarkModelObserverImpl diff --git a/chromium_src/components/sync_bookmarks/bookmark_remote_updates_handler.cc b/chromium_src/components/sync_bookmarks/bookmark_remote_updates_handler.cc new file mode 100644 index 000000000000..de8b2dddb3d6 --- /dev/null +++ b/chromium_src/components/sync_bookmarks/bookmark_remote_updates_handler.cc @@ -0,0 +1,30 @@ +/* Copyright 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "brave/components/brave_sync/syncer_helper.h" + +#define BRAVE_APPLY_REMOTE_UPDATE \ + ? brave_sync::GetIndex(new_parent, node) \ + : brave_sync::GetIndex(new_parent, node) + +#define BRAVE_PROCESS_CREATE_1 \ + std::string order; \ + std::string object_id; \ + for (int i = 0; i < update_entity.specifics.bookmark().meta_info_size(); \ + ++i) { \ + if (update_entity.specifics.bookmark().meta_info(i).key() == "order") { \ + order = update_entity.specifics.bookmark().meta_info(i).value(); \ + } else if (update_entity.specifics.bookmark().meta_info(i).key() == \ + "object_id") { \ + object_id = update_entity.specifics.bookmark().meta_info(i).value(); \ + } \ + } + +#define BRAVE_PROCESS_CREATE_2 \ + ? brave_sync::GetIndex(parent_node, order, object_id) \ + : brave_sync::GetIndex(parent_node, order, object_id) +#include "../../../../components/sync_bookmarks/bookmark_remote_updates_handler.cc" +#undef BRAVE_PROCESS_CREATE_1 +#undef BRAVE_PROCESS_CREATE_2 diff --git a/chromium_src/components/sync_bookmarks/bookmark_specifics_conversions.cc b/chromium_src/components/sync_bookmarks/bookmark_specifics_conversions.cc new file mode 100644 index 000000000000..0ec02ddcebf7 --- /dev/null +++ b/chromium_src/components/sync_bookmarks/bookmark_specifics_conversions.cc @@ -0,0 +1,25 @@ +/* Copyright 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "components/sync_bookmarks/bookmark_specifics_conversions.h" + +#include "brave/components/brave_sync/syncer_helper.h" +#define CreateSpecificsFromBookmarkNode \ + CreateSpecificsFromBookmarkNodeChromiumImpl +#include "../../../../components/sync_bookmarks/bookmark_specifics_conversions.cc" +#undef CreateSpecificsFromBookmarkNode + +namespace sync_bookmarks { + +sync_pb::EntitySpecifics CreateSpecificsFromBookmarkNode( + const bookmarks::BookmarkNode* node, + bookmarks::BookmarkModel* model, + bool force_favicon_load) { + brave_sync::AddBraveMetaInfo(node, model); + return CreateSpecificsFromBookmarkNodeChromiumImpl(node, model, + force_favicon_load); +} + +} // namespace sync_bookmarks diff --git a/components/brave_sync/syncer_helper.cc b/components/brave_sync/syncer_helper.cc index b38a2fe4e07c..7962ac48fd25 100644 --- a/components/brave_sync/syncer_helper.cc +++ b/components/brave_sync/syncer_helper.cc @@ -40,64 +40,39 @@ void SetOrder(bookmarks::BookmarkModel* model, model->SetNodeMetaInfo(node, "order", order); } -size_t GetIndexByOrder(const std::string& record_order) { - size_t index = 0; - size_t last_dot_index = record_order.rfind("."); - DCHECK(last_dot_index != std::string::npos); - std::string last_digit = record_order.substr(last_dot_index + 1); - bool result = base::StringToSizeT(last_digit, &index); - DCHECK(result); - DCHECK_GE(index, 1u); - --index; - return index; -} - } // namespace -size_t GetIndexByCompareOrderStartFrom(const bookmarks::BookmarkNode* parent, - const bookmarks::BookmarkNode* src, - size_t index) { - std::string src_order; - src->GetMetaInfo("order", &src_order); - DCHECK(!src_order.empty()); - bool use_order = true; // If false use object_id - std::string src_object_id; - while (index < parent->children().size()) { - const bookmarks::BookmarkNode* node = parent->children()[index].get(); - if (src->id() == node->id()) { - // We reached ourselves, no sense to go further, because we know all - // unsorted elements are in the end - return index; - } - - if (use_order) { - std::string node_order; - node->GetMetaInfo("order", &node_order); - if (!node_order.empty() && - brave_sync::CompareOrder(src_order, node_order)) { - return index; - } - - if (src_order == node_order) { - use_order = false; +size_t GetIndex(const bookmarks::BookmarkNode* parent, + const std::string& order, + const std::string& object_id) { + DCHECK(!order.empty()); + DCHECK(!object_id.empty()); + for (size_t i = 0; i < parent->children().size(); ++i) { + const bookmarks::BookmarkNode* child = parent->children()[i].get(); + std::string child_order; + child->GetMetaInfo("order", &child_order); + if (!child_order.empty() && + brave_sync::CompareOrder(order, child_order)) { + return i; + } else if (order == child_order) { + std::string child_object_id; + child->GetMetaInfo("object_id", &child_object_id); + if (object_id <= child_object_id) { + return i; } } + } + return parent->children().size(); +} - if (!use_order) { - if (src_object_id.empty()) { - src->GetMetaInfo("object_id", &src_object_id); - } - - std::string node_object_id; - node->GetMetaInfo("object_id", &node_object_id); +size_t GetIndex(const bookmarks::BookmarkNode* parent, + const bookmarks::BookmarkNode* node) { + std::string order; + node->GetMetaInfo("order", &order); + std::string object_id; + node->GetMetaInfo("object_id", &object_id); - if (src_object_id < node_object_id) { - return index; - } - } - ++index; - } - return index; + return GetIndex(parent, order, object_id); } void AddBraveMetaInfo(const bookmarks::BookmarkNode* node, @@ -127,27 +102,4 @@ void AddBraveMetaInfo(const bookmarks::BookmarkNode* node, DCHECK(!sync_timestamp.empty()); } -size_t GetIndex(const bookmarks::BookmarkNode* parent, - const bookmarks::BookmarkNode* src) { - DCHECK(parent); - DCHECK(src); - size_t index = 0; - std::string src_order; - src->GetMetaInfo("order", &src_order); - DCHECK(!src_order.empty()); - index = GetIndexByOrder(src_order); - if (index < parent->children().size()) { - const bookmarks::BookmarkNode* node = parent->children()[index].get(); - if (node) { - std::string node_order; - node->GetMetaInfo("order", &node_order); - - DCHECK(!node_order.empty()); - if (CompareOrder(node_order, src_order)) - return index + 1; - } - } - return index; -} - } // namespace brave_sync diff --git a/components/brave_sync/syncer_helper.h b/components/brave_sync/syncer_helper.h index 2e787bd8b5fa..075f7087f130 100644 --- a/components/brave_sync/syncer_helper.h +++ b/components/brave_sync/syncer_helper.h @@ -16,16 +16,16 @@ class BookmarkNode; namespace brave_sync { -void AddBraveMetaInfo(const bookmarks::BookmarkNode* node, - bookmarks::BookmarkModel* bookmark_model); +// Get index to insert by comparing order or object_id of the node +size_t GetIndex(const bookmarks::BookmarkNode* parent, + const std::string& order, + const std::string& object_id); -// |src| is the node which is about to be inserted into |parent| size_t GetIndex(const bookmarks::BookmarkNode* parent, - const bookmarks::BookmarkNode* src); + const bookmarks::BookmarkNode* node); -size_t GetIndexByCompareOrderStartFrom(const bookmarks::BookmarkNode* parent, - const bookmarks::BookmarkNode* src, - size_t index); +void AddBraveMetaInfo(const bookmarks::BookmarkNode* node, + bookmarks::BookmarkModel* bookmark_model); } // namespace brave_sync diff --git a/components/sync_bookmarks/brave_bookmark_model_observer_impl.cc b/components/sync_bookmarks/brave_bookmark_model_observer_impl.cc new file mode 100644 index 000000000000..3da7a03736ca --- /dev/null +++ b/components/sync_bookmarks/brave_bookmark_model_observer_impl.cc @@ -0,0 +1,31 @@ +/* Copyright 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "brave/components/sync_bookmarks/brave_bookmark_model_observer_impl.h" + +#include + +namespace sync_bookmarks { + +BraveBookmarkModelObserverImpl::BraveBookmarkModelObserverImpl( + const base::RepeatingClosure& nudge_for_commit_closure, + base::OnceClosure on_bookmark_model_being_deleted_closure, + SyncedBookmarkTracker* bookmark_tracker) + : BookmarkModelObserverImpl( + nudge_for_commit_closure, + std::move(on_bookmark_model_being_deleted_closure), + bookmark_tracker) {} + +BraveBookmarkModelObserverImpl::~BraveBookmarkModelObserverImpl() = default; + +void BraveBookmarkModelObserverImpl::BookmarkMetaInfoChanged( + bookmarks::BookmarkModel* model, + const bookmarks::BookmarkNode* node) {} + +void BraveBookmarkModelObserverImpl::BookmarkNodeFaviconChanged( + bookmarks::BookmarkModel* model, + const bookmarks::BookmarkNode* node) {} + +} // namespace sync_bookmarks diff --git a/components/sync_bookmarks/brave_bookmark_model_observer_impl.h b/components/sync_bookmarks/brave_bookmark_model_observer_impl.h new file mode 100644 index 000000000000..63de9390c6d9 --- /dev/null +++ b/components/sync_bookmarks/brave_bookmark_model_observer_impl.h @@ -0,0 +1,33 @@ +/* Copyright 2020 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_COMPONENTS_SYNC_BOOKMARKS_BRAVE_BOOKMARK_MODEL_OBSERVER_IMPL_H_ +#define BRAVE_COMPONENTS_SYNC_BOOKMARKS_BRAVE_BOOKMARK_MODEL_OBSERVER_IMPL_H_ + +#include "components/sync_bookmarks/bookmark_model_observer_impl.h" + +namespace sync_bookmarks { + +class BraveBookmarkModelObserverImpl : public BookmarkModelObserverImpl { + public: + // |bookmark_tracker_| must not be null and must outlive this object. + BraveBookmarkModelObserverImpl( + const base::RepeatingClosure& nudge_for_commit_closure, + base::OnceClosure on_bookmark_model_being_deleted_closure, + SyncedBookmarkTracker* bookmark_tracker); + ~BraveBookmarkModelObserverImpl() override; + + // BookmarkModelObserver: + void BookmarkMetaInfoChanged(bookmarks::BookmarkModel* model, + const bookmarks::BookmarkNode* node) override; + void BookmarkNodeFaviconChanged(bookmarks::BookmarkModel* model, + const bookmarks::BookmarkNode* node) override; + private: + DISALLOW_COPY_AND_ASSIGN(BraveBookmarkModelObserverImpl); +}; + +} // namespace sync_bookmarks + +#endif // BRAVE_COMPONENTS_SYNC_BOOKMARKS_BRAVE_BOOKMARK_MODEL_OBSERVER_IMPL_H_ diff --git a/patches/components-sync_bookmarks-BUILD.gn.patch b/patches/components-sync_bookmarks-BUILD.gn.patch new file mode 100644 index 000000000000..13b3ae31bc67 --- /dev/null +++ b/patches/components-sync_bookmarks-BUILD.gn.patch @@ -0,0 +1,13 @@ +diff --git a/components/sync_bookmarks/BUILD.gn b/components/sync_bookmarks/BUILD.gn +index 40ee9b5de6eb017facd08aa639e29d72e787562c..efdf37f4691e6de19fb9cd4474a435dafd1bf2e1 100644 +--- a/components/sync_bookmarks/BUILD.gn ++++ b/components/sync_bookmarks/BUILD.gn +@@ -28,6 +28,8 @@ static_library("sync_bookmarks") { + "bookmark_sync_service.h", + "synced_bookmark_tracker.cc", + "synced_bookmark_tracker.h", ++ "//brave/components/sync_bookmarks/brave_bookmark_model_observer_impl.cc", ++ "//brave/components/sync_bookmarks/brave_bookmark_model_observer_impl.h", + ] + + deps = [ diff --git a/patches/components-sync_bookmarks-bookmark_change_processor.cc.patch b/patches/components-sync_bookmarks-bookmark_change_processor.cc.patch deleted file mode 100644 index 379d50261539..000000000000 --- a/patches/components-sync_bookmarks-bookmark_change_processor.cc.patch +++ /dev/null @@ -1,52 +0,0 @@ -diff --git a/components/sync_bookmarks/bookmark_change_processor.cc b/components/sync_bookmarks/bookmark_change_processor.cc -index 4c50233634aaef7c3d86cffd1465351dbcc388b8..47b5e7b42adf4dd28f0f2dc7ea1ccd92f9615e04 100644 ---- a/components/sync_bookmarks/bookmark_change_processor.cc -+++ b/components/sync_bookmarks/bookmark_change_processor.cc -@@ -87,6 +87,7 @@ void BookmarkChangeProcessor::UpdateSyncNodeProperties( - bookmark_specifics.set_creation_time_us(src->date_added().ToInternalValue()); - dst->SetBookmarkSpecifics(bookmark_specifics); - SetSyncNodeFavicon(src, model, dst); -+ BRAVE_BOOKMARK_CHANGE_PROCESSOR_UPDATE_SYNC_NODE_PROPERTIES - SetSyncNodeMetaInfo(src, dst); - } - -@@ -380,7 +381,6 @@ int64_t BookmarkChangeProcessor::UpdateSyncNode( - - void BookmarkChangeProcessor::BookmarkMetaInfoChanged( - BookmarkModel* model, const BookmarkNode* node) { -- BookmarkNodeChanged(model, node); - } - - void BookmarkChangeProcessor::BookmarkNodeMoved(BookmarkModel* model, -@@ -414,6 +414,7 @@ void BookmarkChangeProcessor::BookmarkNodeMoved(BookmarkModel* model, - error_handler()->OnUnrecoverableError(error); - return; - } -+ BRAVE_BOOKMARK_CHANGE_PROCESSOR_BOOKMARK_NODE_MOVED - - if (!PlaceSyncNode(MOVE, new_parent, new_index, &trans, &sync_node, - model_associator_)) { -@@ -457,6 +458,7 @@ void BookmarkChangeProcessor::BookmarkNodeFaviconChanged( - return; - } - -+ BRAVE_BOOKMARK_CHANGE_PROCESSOR_BOOKMARK_NODE_FAVICON_CHANGED - CreateOrUpdateSyncNode(node); - } - -@@ -489,6 +491,7 @@ void BookmarkChangeProcessor::BookmarkNodeChildrenReordered( - DCHECK_EQ(sync_child.GetParentId(), - model_associator_->GetSyncIdFromChromeId(node->id())); - -+ BRAVE_BOOKMARK_CHANGE_PROCESSOR_CHILDREN_REORDERED - if (!PlaceSyncNode(MOVE, node, i, &trans, &sync_child, - model_associator_)) { - syncer::SyncError error(FROM_HERE, -@@ -732,6 +735,7 @@ void BookmarkChangeProcessor::ApplyChangesFromSyncModel( - // sync order, left to right, moving them into their proper positions. - for (auto it = to_reposition.begin(); it != to_reposition.end(); ++it) { - const BookmarkNode* parent = it->second->parent(); -+ BRAVE_BOOKMARK_CHANGE_PROCESSOR_APPLY_CHANGES_FROM_SYNC_MODEL - model->Move(it->second, parent, size_t{it->first}); - } - diff --git a/patches/components-sync_bookmarks-bookmark_remote_updates_handler.cc.patch b/patches/components-sync_bookmarks-bookmark_remote_updates_handler.cc.patch new file mode 100644 index 000000000000..2c8dc2bf7d1b --- /dev/null +++ b/patches/components-sync_bookmarks-bookmark_remote_updates_handler.cc.patch @@ -0,0 +1,27 @@ +diff --git a/components/sync_bookmarks/bookmark_remote_updates_handler.cc b/components/sync_bookmarks/bookmark_remote_updates_handler.cc +index 8b81f03c143f4f036d56d92c62f2c15ded7ec81d..85c6bcaf6d4c14c71b7f17426f415e2d1aa32f87 100644 +--- a/components/sync_bookmarks/bookmark_remote_updates_handler.cc ++++ b/components/sync_bookmarks/bookmark_remote_updates_handler.cc +@@ -144,7 +144,7 @@ void ApplyRemoteUpdate( + // Compute index information before updating the |tracker|. + const size_t old_index = size_t{old_parent->GetIndexOf(node)}; + const size_t new_index = +- ComputeChildNodeIndex(new_parent, update_entity.unique_position, tracker); ++ ComputeChildNodeIndex(new_parent, update_entity.unique_position, tracker) BRAVE_APPLY_REMOTE_UPDATE; + tracker->Update(update_entity.id, update.response_version, + update_entity.modification_time, + update_entity.unique_position, update_entity.specifics); +@@ -428,11 +428,12 @@ bool BookmarkRemoteUpdatesHandler::ProcessCreate( + LogProblematicBookmark(RemoteBookmarkUpdateError::kMissingParentNode); + return false; + } ++ BRAVE_PROCESS_CREATE_1 + const bookmarks::BookmarkNode* bookmark_node = + CreateBookmarkNodeFromSpecifics( + update_entity.specifics.bookmark(), parent_node, + ComputeChildNodeIndex(parent_node, update_entity.unique_position, +- bookmark_tracker_), ++ bookmark_tracker_) BRAVE_PROCESS_CREATE_2, + update_entity.is_folder, bookmark_model_, favicon_service_); + if (!bookmark_node) { + // We ignore bookmarks we can't add. From 73df1881feaf13e64ee748cc37a607d838116929 Mon Sep 17 00:00:00 2001 From: Anthony Tseng Date: Tue, 7 Jan 2020 23:04:36 -0800 Subject: [PATCH 2/5] fix sync helper unittest --- .../brave_sync/syncer_helper_unittest.cc | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/components/brave_sync/syncer_helper_unittest.cc b/components/brave_sync/syncer_helper_unittest.cc index cd48e6bf557c..381f652da284 100644 --- a/components/brave_sync/syncer_helper_unittest.cc +++ b/components/brave_sync/syncer_helper_unittest.cc @@ -40,7 +40,7 @@ namespace { void RepositionRespectOrder(bookmarks::BookmarkModel* bookmark_model, const bookmarks::BookmarkNode* node) { const bookmarks::BookmarkNode* parent = node->parent(); - int index = GetIndexByCompareOrderStartFrom(parent, node, 0); + int index = GetIndex(parent, node); bookmark_model->Move(node, parent, index); } @@ -276,6 +276,7 @@ TEST_F(SyncerHelperTest, AddBraveMetaInfoNodeMovedReordered) { TEST_F(SyncerHelperTest, GetIndexInPermanentNodes) { BookmarkNode node(/*id=*/0, base::GenerateGUID(), GURL("https://brave.com")); + node.SetMetaInfo("object_id", "notused"); node.SetMetaInfo("order", "1.0.1.1"); EXPECT_EQ(GetIndex(model()->bookmark_bar_node(), &node), 0u); @@ -285,6 +286,7 @@ TEST_F(SyncerHelperTest, GetIndexInPermanentNodes) { const auto* node_a = model()->AddURL(model()->bookmark_bar_node(), 0, base::ASCIIToUTF16("a.com"), GURL("https://a.com/")); + model()->SetNodeMetaInfo(node_a, "object_id", "notused"); // compare device id model()->SetNodeMetaInfo(node_a, "order", "1.1.1.1"); node.SetMetaInfo("order", "1.0.1.1"); @@ -307,15 +309,18 @@ TEST_F(SyncerHelperTest, GetIndexMoreChildren) { const auto* node_a = model()->AddURL(model()->bookmark_bar_node(), i, base::ASCIIToUTF16("a.com"), GURL("https://a.com/")); - std::string order = "1.1.1." + base::NumberToString(i); + std::string order = "1.1.1." + base::NumberToString(i == 9 ? i + 2 : i + 1); model()->SetNodeMetaInfo(node_a, "order", order); + model()->SetNodeMetaInfo(node_a, "object_id", "notused"); } - // inserted as 10th child + // inserted as first child BookmarkNode node(/*id=*/9, base::GenerateGUID(), GURL("https://brave.com")); + node.SetMetaInfo("object_id", "notused"); node.SetMetaInfo("order", "1.0.1.10"); - EXPECT_EQ(GetIndex(model()->bookmark_bar_node(), &node), 9u); + EXPECT_EQ(GetIndex(model()->bookmark_bar_node(), &node), 0u); + // inserted as 10th child node.SetMetaInfo("order", "1.1.1.10"); - EXPECT_EQ(GetIndex(model()->bookmark_bar_node(), &node), 10u); + EXPECT_EQ(GetIndex(model()->bookmark_bar_node(), &node), 9u); } TEST_F(SyncerHelperTest, GetIndexInFolder) { @@ -323,6 +328,7 @@ TEST_F(SyncerHelperTest, GetIndexInFolder) { base::ASCIIToUTF16("Folder1")); model()->SetNodeMetaInfo(folder1, "order", "1.0.1.1"); BookmarkNode node(/*id=*/1, base::GenerateGUID(), GURL("https://brave.com")); + node.SetMetaInfo("object_id", "notused"); node.SetMetaInfo("order", "1.0.1.1.1"); EXPECT_EQ(GetIndex(folder1, &node), 0u); @@ -330,6 +336,7 @@ TEST_F(SyncerHelperTest, GetIndexInFolder) { const auto* node_a = model()->AddURL(folder1, 0, base::ASCIIToUTF16("a.com"), GURL("https://a.com/")); model()->SetNodeMetaInfo(node_a, "order", "1.0.1.1.1"); + model()->SetNodeMetaInfo(node_a, "object_id", "notused"); node.SetMetaInfo("order", "1.0.1.1.2"); EXPECT_EQ(GetIndex(folder1, &node), 1u); } @@ -364,11 +371,11 @@ TEST_F(SyncerHelperTest, SameOrderBookmarksSordetByObjectIdFull3) { // Expect b1 and b2 no need to move uint64_t index_to_move_b1 = - GetIndexByCompareOrderStartFrom(model()->bookmark_bar_node(), node_b1, 0); + GetIndex(model()->bookmark_bar_node(), node_b1); EXPECT_EQ(index_to_move_b1, 2u); uint64_t index_to_move_b2 = - GetIndexByCompareOrderStartFrom(model()->bookmark_bar_node(), node_b2, 0); + GetIndex(model()->bookmark_bar_node(), node_b2); EXPECT_EQ(index_to_move_b2, 3u); const auto* node_a3 = From 9b84b78a4497e259b5303d3420c81be09d6f1ef9 Mon Sep 17 00:00:00 2001 From: Anthony Tseng Date: Wed, 8 Jan 2020 15:41:55 -0800 Subject: [PATCH 3/5] Simplify bookmark_remote_updates_handler.cc overrides --- .../bookmark_remote_updates_handler.cc | 10 ++++------ ...rks-bookmark_remote_updates_handler.cc.patch | 17 +++++++---------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/chromium_src/components/sync_bookmarks/bookmark_remote_updates_handler.cc b/chromium_src/components/sync_bookmarks/bookmark_remote_updates_handler.cc index de8b2dddb3d6..153358b7b5b3 100644 --- a/chromium_src/components/sync_bookmarks/bookmark_remote_updates_handler.cc +++ b/chromium_src/components/sync_bookmarks/bookmark_remote_updates_handler.cc @@ -5,9 +5,7 @@ #include "brave/components/brave_sync/syncer_helper.h" -#define BRAVE_APPLY_REMOTE_UPDATE \ - ? brave_sync::GetIndex(new_parent, node) \ - : brave_sync::GetIndex(new_parent, node) +#define BRAVE_APPLY_REMOTE_UPDATE true ? brave_sync::GetIndex(new_parent, node): #define BRAVE_PROCESS_CREATE_1 \ std::string order; \ @@ -22,9 +20,9 @@ } \ } -#define BRAVE_PROCESS_CREATE_2 \ - ? brave_sync::GetIndex(parent_node, order, object_id) \ - : brave_sync::GetIndex(parent_node, order, object_id) +#define BRAVE_PROCESS_CREATE_2 \ + true ? brave_sync::GetIndex(parent_node, order, object_id): #include "../../../../components/sync_bookmarks/bookmark_remote_updates_handler.cc" +#undef BRAVE_APPLY_REMOTE_UPDATE #undef BRAVE_PROCESS_CREATE_1 #undef BRAVE_PROCESS_CREATE_2 diff --git a/patches/components-sync_bookmarks-bookmark_remote_updates_handler.cc.patch b/patches/components-sync_bookmarks-bookmark_remote_updates_handler.cc.patch index 2c8dc2bf7d1b..5907d3f48c0c 100644 --- a/patches/components-sync_bookmarks-bookmark_remote_updates_handler.cc.patch +++ b/patches/components-sync_bookmarks-bookmark_remote_updates_handler.cc.patch @@ -1,17 +1,16 @@ diff --git a/components/sync_bookmarks/bookmark_remote_updates_handler.cc b/components/sync_bookmarks/bookmark_remote_updates_handler.cc -index 8b81f03c143f4f036d56d92c62f2c15ded7ec81d..85c6bcaf6d4c14c71b7f17426f415e2d1aa32f87 100644 +index 8b81f03c143f4f036d56d92c62f2c15ded7ec81d..b0a4daf7354d7ac7e3505745a884131ae55f1658 100644 --- a/components/sync_bookmarks/bookmark_remote_updates_handler.cc +++ b/components/sync_bookmarks/bookmark_remote_updates_handler.cc -@@ -144,7 +144,7 @@ void ApplyRemoteUpdate( +@@ -144,6 +144,7 @@ void ApplyRemoteUpdate( // Compute index information before updating the |tracker|. const size_t old_index = size_t{old_parent->GetIndexOf(node)}; const size_t new_index = -- ComputeChildNodeIndex(new_parent, update_entity.unique_position, tracker); -+ ComputeChildNodeIndex(new_parent, update_entity.unique_position, tracker) BRAVE_APPLY_REMOTE_UPDATE; ++ BRAVE_APPLY_REMOTE_UPDATE + ComputeChildNodeIndex(new_parent, update_entity.unique_position, tracker); tracker->Update(update_entity.id, update.response_version, update_entity.modification_time, - update_entity.unique_position, update_entity.specifics); -@@ -428,11 +428,12 @@ bool BookmarkRemoteUpdatesHandler::ProcessCreate( +@@ -428,9 +429,11 @@ bool BookmarkRemoteUpdatesHandler::ProcessCreate( LogProblematicBookmark(RemoteBookmarkUpdateError::kMissingParentNode); return false; } @@ -19,9 +18,7 @@ index 8b81f03c143f4f036d56d92c62f2c15ded7ec81d..85c6bcaf6d4c14c71b7f17426f415e2d const bookmarks::BookmarkNode* bookmark_node = CreateBookmarkNodeFromSpecifics( update_entity.specifics.bookmark(), parent_node, ++ BRAVE_PROCESS_CREATE_2 ComputeChildNodeIndex(parent_node, update_entity.unique_position, -- bookmark_tracker_), -+ bookmark_tracker_) BRAVE_PROCESS_CREATE_2, + bookmark_tracker_), update_entity.is_folder, bookmark_model_, favicon_service_); - if (!bookmark_node) { - // We ignore bookmarks we can't add. From a5ca8857f9a5021a7381e7d908a57a8e2a8bc022 Mon Sep 17 00:00:00 2001 From: Anthony Tseng Date: Wed, 8 Jan 2020 19:04:58 -0800 Subject: [PATCH 4/5] fix brave main delegate browsertest --- app/brave_main_delegate_browsertest.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/brave_main_delegate_browsertest.cc b/app/brave_main_delegate_browsertest.cc index f1b2dbde0aee..1681bc8a4ba0 100644 --- a/app/brave_main_delegate_browsertest.cc +++ b/app/brave_main_delegate_browsertest.cc @@ -13,7 +13,6 @@ #include "components/autofill/core/common/autofill_payments_features.h" #include "components/omnibox/common/omnibox_features.h" #include "components/password_manager/core/common/password_manager_features.h" -#include "components/sync/driver/sync_driver_switches.h" #include "components/unified_consent/feature.h" #include "content/public/browser/render_view_host.h" #include "content/public/common/content_features.h" @@ -53,7 +52,6 @@ IN_PROC_BROWSER_TEST_F(BraveMainDelegateBrowserTest, DisabledFeatures) { &features::kWebXrGamepadModule, &unified_consent::kUnifiedConsent, &features::kLookalikeUrlNavigationSuggestionsUI, - &switches::kSyncUSSBookmarks, }; for (const auto* feature : disabled_features) From 7a4de4c9268ce72fc1eb1bb0d41349268dcd82ef Mon Sep 17 00:00:00 2001 From: Anthony Tseng Date: Thu, 9 Jan 2020 10:47:46 -0800 Subject: [PATCH 5/5] Put BraveBookmarkModelObserver in BookmarkModelTypeProcessor override and use AsMutable to set metainfo to prevent triggering BookmarkMetaInfoChanged --- .../bookmark_model_type_processor.cc | 33 +++++++++++++- .../bookmark_specifics_conversions.cc | 2 +- components/brave_sync/syncer_helper.cc | 23 +++++----- components/brave_sync/syncer_helper.h | 4 +- .../brave_sync/syncer_helper_unittest.cc | 44 +++++++++---------- .../brave_bookmark_model_observer_impl.cc | 31 ------------- .../brave_bookmark_model_observer_impl.h | 33 -------------- .../components-sync_bookmarks-BUILD.gn.patch | 13 ------ 8 files changed, 69 insertions(+), 114 deletions(-) delete mode 100644 components/sync_bookmarks/brave_bookmark_model_observer_impl.cc delete mode 100644 components/sync_bookmarks/brave_bookmark_model_observer_impl.h delete mode 100644 patches/components-sync_bookmarks-BUILD.gn.patch diff --git a/chromium_src/components/sync_bookmarks/bookmark_model_type_processor.cc b/chromium_src/components/sync_bookmarks/bookmark_model_type_processor.cc index 99b81d6b6887..2a2febe25d39 100644 --- a/chromium_src/components/sync_bookmarks/bookmark_model_type_processor.cc +++ b/chromium_src/components/sync_bookmarks/bookmark_model_type_processor.cc @@ -3,7 +3,38 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#include "brave/components/sync_bookmarks/brave_bookmark_model_observer_impl.h" +#include "components/sync_bookmarks/bookmark_model_observer_impl.h" + +#include + +namespace sync_bookmarks { + +class BraveBookmarkModelObserverImpl : public BookmarkModelObserverImpl { + public: + // |bookmark_tracker_| must not be null and must outlive this object. + BraveBookmarkModelObserverImpl( + const base::RepeatingClosure& nudge_for_commit_closure, + base::OnceClosure on_bookmark_model_being_deleted_closure, + SyncedBookmarkTracker* bookmark_tracker) + : BookmarkModelObserverImpl( + nudge_for_commit_closure, + std::move(on_bookmark_model_being_deleted_closure), + bookmark_tracker) {} + ~BraveBookmarkModelObserverImpl() override = default; + + // BookmarkModelObserver: + void BookmarkMetaInfoChanged(bookmarks::BookmarkModel* model, + const bookmarks::BookmarkNode* node) override {} + void BookmarkNodeFaviconChanged( + bookmarks::BookmarkModel* model, + const bookmarks::BookmarkNode* node) override {} + + private: + DISALLOW_COPY_AND_ASSIGN(BraveBookmarkModelObserverImpl); +}; + +} // namespace sync_bookmarks + #define BookmarkModelObserverImpl BraveBookmarkModelObserverImpl #include "../../../../components/sync_bookmarks/bookmark_model_type_processor.cc" #undef BookmarkModelObserverImpl diff --git a/chromium_src/components/sync_bookmarks/bookmark_specifics_conversions.cc b/chromium_src/components/sync_bookmarks/bookmark_specifics_conversions.cc index 0ec02ddcebf7..40a8fb1db5e5 100644 --- a/chromium_src/components/sync_bookmarks/bookmark_specifics_conversions.cc +++ b/chromium_src/components/sync_bookmarks/bookmark_specifics_conversions.cc @@ -17,7 +17,7 @@ sync_pb::EntitySpecifics CreateSpecificsFromBookmarkNode( const bookmarks::BookmarkNode* node, bookmarks::BookmarkModel* model, bool force_favicon_load) { - brave_sync::AddBraveMetaInfo(node, model); + brave_sync::AddBraveMetaInfo(node); return CreateSpecificsFromBookmarkNodeChromiumImpl(node, model, force_favicon_load); } diff --git a/components/brave_sync/syncer_helper.cc b/components/brave_sync/syncer_helper.cc index 7962ac48fd25..d60fbe898e55 100644 --- a/components/brave_sync/syncer_helper.cc +++ b/components/brave_sync/syncer_helper.cc @@ -8,13 +8,17 @@ #include "base/strings/string_number_conversions.h" #include "brave/components/brave_sync/bookmark_order_util.h" #include "brave/components/brave_sync/tools.h" -#include "components/bookmarks/browser/bookmark_model.h" +#include "components/bookmarks/browser/bookmark_node.h" namespace brave_sync { namespace { -void SetOrder(bookmarks::BookmarkModel* model, - const bookmarks::BookmarkNode* node, +// Get mutable node to prevent BookmarkMetaInfoChanged from being triggered +bookmarks::BookmarkNode* AsMutable(const bookmarks::BookmarkNode* node) { + return const_cast(node); +} + +void SetOrder(const bookmarks::BookmarkNode* node, const std::string& parent_order) { DCHECK(!parent_order.empty()); int index = node->parent()->GetIndexOf(node); @@ -37,7 +41,7 @@ void SetOrder(bookmarks::BookmarkModel* model, std::string order = brave_sync::GetOrder(prev_order, next_order, parent_order); - model->SetNodeMetaInfo(node, "order", order); + AsMutable(node)->SetMetaInfo("order", order); } } // namespace @@ -75,11 +79,10 @@ size_t GetIndex(const bookmarks::BookmarkNode* parent, return GetIndex(parent, order, object_id); } -void AddBraveMetaInfo(const bookmarks::BookmarkNode* node, - bookmarks::BookmarkModel* model) { +void AddBraveMetaInfo(const bookmarks::BookmarkNode* node) { std::string parent_order; node->parent()->GetMetaInfo("order", &parent_order); - SetOrder(model, node, parent_order); + SetOrder(node, parent_order); std::string object_id; node->GetMetaInfo("object_id", &object_id); @@ -87,17 +90,17 @@ void AddBraveMetaInfo(const bookmarks::BookmarkNode* node, if (object_id.empty()) { object_id = tools::GenerateObjectId(); } - model->SetNodeMetaInfo(node, "object_id", object_id); + AsMutable(node)->SetMetaInfo("object_id", object_id); std::string parent_object_id; node->parent()->GetMetaInfo("object_id", &parent_object_id); - model->SetNodeMetaInfo(node, "parent_object_id", parent_object_id); + AsMutable(node)->SetMetaInfo("parent_object_id", parent_object_id); std::string sync_timestamp; node->GetMetaInfo("sync_timestamp", &sync_timestamp); if (sync_timestamp.empty()) { sync_timestamp = std::to_string(base::Time::Now().ToJsTime()); - model->SetNodeMetaInfo(node, "sync_timestamp", sync_timestamp); + AsMutable(node)->SetMetaInfo("sync_timestamp", sync_timestamp); } DCHECK(!sync_timestamp.empty()); } diff --git a/components/brave_sync/syncer_helper.h b/components/brave_sync/syncer_helper.h index 075f7087f130..6739ef40b950 100644 --- a/components/brave_sync/syncer_helper.h +++ b/components/brave_sync/syncer_helper.h @@ -10,7 +10,6 @@ #include namespace bookmarks { -class BookmarkModel; class BookmarkNode; } // namespace bookmarks @@ -24,8 +23,7 @@ size_t GetIndex(const bookmarks::BookmarkNode* parent, size_t GetIndex(const bookmarks::BookmarkNode* parent, const bookmarks::BookmarkNode* node); -void AddBraveMetaInfo(const bookmarks::BookmarkNode* node, - bookmarks::BookmarkModel* bookmark_model); +void AddBraveMetaInfo(const bookmarks::BookmarkNode* node); } // namespace brave_sync diff --git a/components/brave_sync/syncer_helper_unittest.cc b/components/brave_sync/syncer_helper_unittest.cc index 381f652da284..8bab3ff9714d 100644 --- a/components/brave_sync/syncer_helper_unittest.cc +++ b/components/brave_sync/syncer_helper_unittest.cc @@ -94,7 +94,7 @@ TEST_F(SyncerHelperTest, AddBraveMetaInfoCreateOrUpdate) { std::string sync_timestamp; const auto* folder1 = model()->AddFolder(model()->bookmark_bar_node(), 0, base::ASCIIToUTF16("Folder1")); - AddBraveMetaInfo(folder1, model()); + AddBraveMetaInfo(folder1); folder1->GetMetaInfo("order", &order); EXPECT_EQ(order, "1.0.1.1"); std::string folder1_id; @@ -110,7 +110,7 @@ TEST_F(SyncerHelperTest, AddBraveMetaInfoCreateOrUpdate) { folder1, 0, base::ASCIIToUTF16("A.com - title"), GURL("https://a.com/")); order.clear(); sync_timestamp.clear(); - AddBraveMetaInfo(node_a, model()); + AddBraveMetaInfo(node_a); node_a->GetMetaInfo("order", &order); EXPECT_EQ(order, "1.0.1.1.1"); std::string node_a_id; @@ -127,7 +127,7 @@ TEST_F(SyncerHelperTest, AddBraveMetaInfoCreateOrUpdate) { node_a_id.clear(); node_a_parent_id.clear(); model()->SetURL(node_a, GURL("https://a-m.com/")); - AddBraveMetaInfo(node_a, model()); + AddBraveMetaInfo(node_a); node_a->GetMetaInfo("order", &order); EXPECT_EQ(order, "1.0.1.1.1"); node_a->GetMetaInfo("object_id", &node_a_id); @@ -142,12 +142,12 @@ TEST_F(SyncerHelperTest, AddBraveMetaInfoCreateOrUpdate) { TEST_F(SyncerHelperTest, AddBraveMetaInfoNodeMoved) { const auto* folder1 = model()->AddFolder(model()->bookmark_bar_node(), 0, base::ASCIIToUTF16("Folder1")); - AddBraveMetaInfo(folder1, model()); + AddBraveMetaInfo(folder1); const auto* node_a = model()->AddURL( folder1, 0, base::ASCIIToUTF16("A.com - title"), GURL("https://a.com/")); - AddBraveMetaInfo(node_a, model()); + AddBraveMetaInfo(node_a); model()->Move(node_a, model()->bookmark_bar_node(), 1); - AddBraveMetaInfo(node_a, model()); + AddBraveMetaInfo(node_a); std::string order; node_a->GetMetaInfo("order", &order); @@ -167,15 +167,15 @@ TEST_F(SyncerHelperTest, AddBraveMetaInfoNodeChildrenReordered) { const auto* node_a = model()->AddURL(model()->bookmark_bar_node(), 0, base::ASCIIToUTF16("A.com - title"), GURL("https://a.com/")); - AddBraveMetaInfo(node_a, model()); + AddBraveMetaInfo(node_a); const auto* node_b = model()->AddURL(model()->bookmark_bar_node(), 1, base::ASCIIToUTF16("B.com - title"), GURL("https://b.com/")); - AddBraveMetaInfo(node_b, model()); + AddBraveMetaInfo(node_b); const auto* node_c = model()->AddURL(model()->bookmark_bar_node(), 2, base::ASCIIToUTF16("C.com - title"), GURL("https://c.com/")); - AddBraveMetaInfo(node_c, model()); + AddBraveMetaInfo(node_c); // Expecting to have initially: // 'Bookmarks Bar' 1.0.1 @@ -194,7 +194,7 @@ TEST_F(SyncerHelperTest, AddBraveMetaInfoNodeChildrenReordered) { EXPECT_EQ(order_c, "1.0.1.3"); model()->Move(node_c, model()->bookmark_bar_node(), 0); - AddBraveMetaInfo(node_c, model()); + AddBraveMetaInfo(node_c); // After move to have: // 'Bookmarks Bar' 1.0.1 @@ -218,16 +218,16 @@ TEST_F(SyncerHelperTest, AddBraveMetaInfoNodeMovedReordered) { const auto* node_a = model()->AddURL(model()->bookmark_bar_node(), 0, base::ASCIIToUTF16("A.com - title"), GURL("https://a.com/")); - AddBraveMetaInfo(node_a, model()); + AddBraveMetaInfo(node_a); const auto* folder1 = model()->AddFolder(model()->bookmark_bar_node(), 1, base::ASCIIToUTF16("Folder1")); - AddBraveMetaInfo(folder1, model()); + AddBraveMetaInfo(folder1); const auto* node_b = model()->AddURL( folder1, 0, base::ASCIIToUTF16("B.com - title"), GURL("https://b.com/")); - AddBraveMetaInfo(node_b, model()); + AddBraveMetaInfo(node_b); const auto* node_c = model()->AddURL( folder1, 1, base::ASCIIToUTF16("C.com - title"), GURL("https://c.com/")); - AddBraveMetaInfo(node_c, model()); + AddBraveMetaInfo(node_c); // Expecting here to have: // 'Bookmarks Bar' 1.0.1 @@ -250,7 +250,7 @@ TEST_F(SyncerHelperTest, AddBraveMetaInfoNodeMovedReordered) { EXPECT_EQ(order_folder1, "1.0.1.2"); model()->Move(node_a, folder1, 0); - AddBraveMetaInfo(node_a, model()); + AddBraveMetaInfo(node_a); order_a.clear(); order_b.clear(); @@ -355,19 +355,19 @@ TEST_F(SyncerHelperTest, SameOrderBookmarksSordetByObjectIdFull3) { const auto* node_a1 = model()->AddURL(model()->bookmark_bar_node(), 0, base::ASCIIToUTF16("A1.com"), GURL("https://a1.com/")); - AddBraveMetaInfo(node_a1, model()); + AddBraveMetaInfo(node_a1); const auto* node_a2 = model()->AddURL(model()->bookmark_bar_node(), 1, base::ASCIIToUTF16("A2.com"), GURL("https://a2.com/")); - AddBraveMetaInfo(node_a2, model()); + AddBraveMetaInfo(node_a2); const auto* node_b1 = model()->AddURL(model()->bookmark_bar_node(), 2, base::ASCIIToUTF16("B1.com"), GURL("https://b1.com/")); - AddBraveMetaInfo(node_b1, model()); + AddBraveMetaInfo(node_b1); const auto* node_b2 = model()->AddURL(model()->bookmark_bar_node(), 3, base::ASCIIToUTF16("B2.com"), GURL("https://b2.com/")); - AddBraveMetaInfo(node_b2, model()); + AddBraveMetaInfo(node_b2); // Expect b1 and b2 no need to move uint64_t index_to_move_b1 = @@ -381,15 +381,15 @@ TEST_F(SyncerHelperTest, SameOrderBookmarksSordetByObjectIdFull3) { const auto* node_a3 = model()->AddURL(model()->bookmark_bar_node(), 4, base::ASCIIToUTF16("A3.com"), GURL("https://a3.com/")); - AddBraveMetaInfo(node_a3, model()); + AddBraveMetaInfo(node_a3); const auto* node_b3 = model()->AddURL(model()->bookmark_bar_node(), 5, base::ASCIIToUTF16("B3.com"), GURL("https://b3.com/")); - AddBraveMetaInfo(node_b3, model()); + AddBraveMetaInfo(node_b3); const auto* node_c3 = model()->AddURL(model()->bookmark_bar_node(), 6, base::ASCIIToUTF16("C3.com"), GURL("https://c3.com/")); - AddBraveMetaInfo(node_c3, model()); + AddBraveMetaInfo(node_c3); std::string a3_order; node_a3->GetMetaInfo("order", &a3_order); diff --git a/components/sync_bookmarks/brave_bookmark_model_observer_impl.cc b/components/sync_bookmarks/brave_bookmark_model_observer_impl.cc deleted file mode 100644 index 3da7a03736ca..000000000000 --- a/components/sync_bookmarks/brave_bookmark_model_observer_impl.cc +++ /dev/null @@ -1,31 +0,0 @@ -/* Copyright 2020 The Brave Authors. All rights reserved. - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include "brave/components/sync_bookmarks/brave_bookmark_model_observer_impl.h" - -#include - -namespace sync_bookmarks { - -BraveBookmarkModelObserverImpl::BraveBookmarkModelObserverImpl( - const base::RepeatingClosure& nudge_for_commit_closure, - base::OnceClosure on_bookmark_model_being_deleted_closure, - SyncedBookmarkTracker* bookmark_tracker) - : BookmarkModelObserverImpl( - nudge_for_commit_closure, - std::move(on_bookmark_model_being_deleted_closure), - bookmark_tracker) {} - -BraveBookmarkModelObserverImpl::~BraveBookmarkModelObserverImpl() = default; - -void BraveBookmarkModelObserverImpl::BookmarkMetaInfoChanged( - bookmarks::BookmarkModel* model, - const bookmarks::BookmarkNode* node) {} - -void BraveBookmarkModelObserverImpl::BookmarkNodeFaviconChanged( - bookmarks::BookmarkModel* model, - const bookmarks::BookmarkNode* node) {} - -} // namespace sync_bookmarks diff --git a/components/sync_bookmarks/brave_bookmark_model_observer_impl.h b/components/sync_bookmarks/brave_bookmark_model_observer_impl.h deleted file mode 100644 index 63de9390c6d9..000000000000 --- a/components/sync_bookmarks/brave_bookmark_model_observer_impl.h +++ /dev/null @@ -1,33 +0,0 @@ -/* Copyright 2020 The Brave Authors. All rights reserved. - * This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#ifndef BRAVE_COMPONENTS_SYNC_BOOKMARKS_BRAVE_BOOKMARK_MODEL_OBSERVER_IMPL_H_ -#define BRAVE_COMPONENTS_SYNC_BOOKMARKS_BRAVE_BOOKMARK_MODEL_OBSERVER_IMPL_H_ - -#include "components/sync_bookmarks/bookmark_model_observer_impl.h" - -namespace sync_bookmarks { - -class BraveBookmarkModelObserverImpl : public BookmarkModelObserverImpl { - public: - // |bookmark_tracker_| must not be null and must outlive this object. - BraveBookmarkModelObserverImpl( - const base::RepeatingClosure& nudge_for_commit_closure, - base::OnceClosure on_bookmark_model_being_deleted_closure, - SyncedBookmarkTracker* bookmark_tracker); - ~BraveBookmarkModelObserverImpl() override; - - // BookmarkModelObserver: - void BookmarkMetaInfoChanged(bookmarks::BookmarkModel* model, - const bookmarks::BookmarkNode* node) override; - void BookmarkNodeFaviconChanged(bookmarks::BookmarkModel* model, - const bookmarks::BookmarkNode* node) override; - private: - DISALLOW_COPY_AND_ASSIGN(BraveBookmarkModelObserverImpl); -}; - -} // namespace sync_bookmarks - -#endif // BRAVE_COMPONENTS_SYNC_BOOKMARKS_BRAVE_BOOKMARK_MODEL_OBSERVER_IMPL_H_ diff --git a/patches/components-sync_bookmarks-BUILD.gn.patch b/patches/components-sync_bookmarks-BUILD.gn.patch deleted file mode 100644 index 13b3ae31bc67..000000000000 --- a/patches/components-sync_bookmarks-BUILD.gn.patch +++ /dev/null @@ -1,13 +0,0 @@ -diff --git a/components/sync_bookmarks/BUILD.gn b/components/sync_bookmarks/BUILD.gn -index 40ee9b5de6eb017facd08aa639e29d72e787562c..efdf37f4691e6de19fb9cd4474a435dafd1bf2e1 100644 ---- a/components/sync_bookmarks/BUILD.gn -+++ b/components/sync_bookmarks/BUILD.gn -@@ -28,6 +28,8 @@ static_library("sync_bookmarks") { - "bookmark_sync_service.h", - "synced_bookmark_tracker.cc", - "synced_bookmark_tracker.h", -+ "//brave/components/sync_bookmarks/brave_bookmark_model_observer_impl.cc", -+ "//brave/components/sync_bookmarks/brave_bookmark_model_observer_impl.h", - ] - - deps = [