From 55fd3501daa64497b101feea310a52845c771d44 Mon Sep 17 00:00:00 2001 From: Max Karolinskiy Date: Fri, 9 Aug 2019 17:45:25 -0400 Subject: [PATCH] TEMPORARY: bookmarks workaround. #2010 Pending. --- browser/bookmarks/brave_bookmark_client.cc | 9 ++- browser/bookmarks/brave_bookmark_client.h | 2 +- components/brave_sync/brave_sync_service.h | 5 +- .../client/bookmark_change_processor.cc | 79 ++++++++++--------- .../client/bookmark_change_processor.h | 4 +- components/brave_sync/test_util.cc | 38 ++++----- 6 files changed, 71 insertions(+), 66 deletions(-) diff --git a/browser/bookmarks/brave_bookmark_client.cc b/browser/bookmarks/brave_bookmark_client.cc index 4a5e49d88c41..6f9df4f2162b 100644 --- a/browser/bookmarks/brave_bookmark_client.cc +++ b/browser/bookmarks/brave_bookmark_client.cc @@ -22,12 +22,13 @@ BraveBookmarkClient::BraveBookmarkClient( managed_bookmark_service, bookmark_sync_service) {} -bookmarks::LoadExtraCallback BraveBookmarkClient::GetLoadExtraNodesCallback() { +bookmarks::LoadManagedNodeCallback +BraveBookmarkClient::GetLoadManagedNodeCallback() { #if BUILDFLAG(ENABLE_BRAVE_SYNC) - return base::BindOnce(&brave_sync::LoadExtraNodes, - ChromeBookmarkClient::GetLoadExtraNodesCallback()); + return base::BindOnce(&brave_sync::LoadManagedNode, + ChromeBookmarkClient::GetLoadManagedNodeCallback()); #endif - return ChromeBookmarkClient::GetLoadExtraNodesCallback(); + return ChromeBookmarkClient::GetLoadManagedNodeCallback(); } bool BraveBookmarkClient::IsPermanentNodeVisible( diff --git a/browser/bookmarks/brave_bookmark_client.h b/browser/bookmarks/brave_bookmark_client.h index ed3e7a96d99e..7fb8bef91d58 100644 --- a/browser/bookmarks/brave_bookmark_client.h +++ b/browser/bookmarks/brave_bookmark_client.h @@ -14,7 +14,7 @@ class BraveBookmarkClient : public ChromeBookmarkClient { bookmarks::ManagedBookmarkService* managed_bookmark_service, sync_bookmarks::BookmarkSyncService* bookmark_sync_service); - bookmarks::LoadExtraCallback GetLoadExtraNodesCallback() override; + bookmarks::LoadManagedNodeCallback GetLoadManagedNodeCallback() override; bool IsPermanentNodeVisible( const bookmarks::BookmarkPermanentNode* node) override; diff --git a/components/brave_sync/brave_sync_service.h b/components/brave_sync/brave_sync_service.h index a4e7c24dd586..0d2a05eddb97 100644 --- a/components/brave_sync/brave_sync_service.h +++ b/components/brave_sync/brave_sync_service.h @@ -21,8 +21,9 @@ class BraveSyncServiceObserver; class Settings; class SyncDevices; -bookmarks::BookmarkPermanentNodeList -LoadExtraNodes(bookmarks::LoadExtraCallback callback, int64_t* next_node_id); +std::unique_ptr LoadManagedNode( + bookmarks::LoadManagedNodeCallback callback, + int64_t* next_node_id); bool IsSyncManagedNode(const bookmarks::BookmarkPermanentNode* node); class BraveSyncService : public KeyedService { diff --git a/components/brave_sync/client/bookmark_change_processor.cc b/components/brave_sync/client/bookmark_change_processor.cc index bd0b106e2f3c..1c685b2fdc46 100644 --- a/components/brave_sync/client/bookmark_change_processor.cc +++ b/components/brave_sync/client/bookmark_change_processor.cc @@ -13,6 +13,7 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" +#include "brave/components/brave_sync/brave_sync_service.h" #include "brave/components/brave_sync/bookmark_order_util.h" #include "brave/components/brave_sync/client/bookmark_node.h" #include "brave/components/brave_sync/jslib_const.h" @@ -80,21 +81,13 @@ bool IsSyncManagedNode(const bookmarks::BookmarkPermanentNode* node) { return IsSyncManagedNodeDeleted(node) || IsSyncManagedNodePending(node); } -bookmarks::BookmarkPermanentNodeList -LoadExtraNodes(bookmarks::LoadExtraCallback callback, +std::unique_ptr LoadManagedNode( + bookmarks::LoadManagedNodeCallback callback, int64_t* next_node_id) { // TODO(bridiver) - deleted node should not be visible - bookmarks::BookmarkPermanentNodeList extra_nodes; if (callback) - extra_nodes = std::move(callback).Run(next_node_id); - - auto deleted_node = MakePermanentNode(kDeletedBookmarksTitle, next_node_id); - extra_nodes.push_back(std::move(deleted_node)); - - auto pending_node = MakePermanentNode(kPendingBookmarksTitle, next_node_id); - extra_nodes.push_back(std::move(pending_node)); - - return extra_nodes; + return std::move(callback).Run(next_node_id); + return nullptr; } namespace { @@ -797,38 +790,48 @@ void BookmarkChangeProcessor::GetAllSyncData( bookmarks::BookmarkNode* BookmarkChangeProcessor::GetDeletedNodeRoot() { if (!deleted_node_root_) { - ui::TreeNodeIterator - iterator(bookmark_model_->root_node()); - while (iterator.has_next()) { - const bookmarks::BookmarkNode* node = iterator.Next(); - if (node->is_permanent_node() && - IsSyncManagedNodeDeleted( - static_cast(node))) { - deleted_node_root_ = const_cast(node); - return deleted_node_root_; - } - } + int64_t next_node_id = bookmark_model_->next_node_id(); + deleted_node_root_ = + MakePermanentNode(kDeletedBookmarksTitle, &next_node_id); + bookmark_model_->Move(deleted_node_root_.get(), + bookmark_model_->root_node(), 0); + //ui::TreeNodeIterator + // iterator(bookmark_model_->root_node()); + //while (iterator.has_next()) { + // const bookmarks::BookmarkNode* node = iterator.Next(); + // if (node->is_permanent_node() && + // IsSyncManagedNodeDeleted( + // static_cast(node))) { + // deleted_node_root_ = const_cast(node); + // return deleted_node_root_; + // } + //} } - DCHECK(deleted_node_root_); - return deleted_node_root_; + DCHECK(deleted_node_root_.get()); + return deleted_node_root_.get(); } bookmarks::BookmarkNode* BookmarkChangeProcessor::GetPendingNodeRoot() { if (!pending_node_root_) { - ui::TreeNodeIterator - iterator(bookmark_model_->root_node()); - while (iterator.has_next()) { - const bookmarks::BookmarkNode* node = iterator.Next(); - if (node->is_permanent_node() && - IsSyncManagedNodePending( - static_cast(node))) { - pending_node_root_ = const_cast(node); - return pending_node_root_; - } - } + int64_t next_node_id = bookmark_model_->next_node_id(); + pending_node_root_ = + MakePermanentNode(kPendingBookmarksTitle, &next_node_id); + bookmark_model_->Move(pending_node_root_.get(), + bookmark_model_->root_node(), 0); + // ui::TreeNodeIterator + // iterator(bookmark_model_->root_node()); + //while (iterator.has_next()) { + // const bookmarks::BookmarkNode* node = iterator.Next(); + // if (node->is_permanent_node() && + // IsSyncManagedNodePending( + // static_cast(node))) { + // pending_node_root_ = const_cast(node); + // return pending_node_root_; + // } + //} } - DCHECK(pending_node_root_); - return pending_node_root_; + DCHECK(pending_node_root_.get()); + return pending_node_root_.get(); } int BookmarkChangeProcessor::FindMigrateSubOrderLength( diff --git a/components/brave_sync/client/bookmark_change_processor.h b/components/brave_sync/client/bookmark_change_processor.h index 016873f537d8..6a5af7e6e647 100644 --- a/components/brave_sync/client/bookmark_change_processor.h +++ b/components/brave_sync/client/bookmark_change_processor.h @@ -136,8 +136,8 @@ class BookmarkChangeProcessor : public ChangeProcessor, Profile* profile_; // not owned bookmarks::BookmarkModel* bookmark_model_; // not owned - bookmarks::BookmarkNode* deleted_node_root_; - bookmarks::BookmarkNode* pending_node_root_; + std::unique_ptr deleted_node_root_; + std::unique_ptr pending_node_root_; DISALLOW_COPY_AND_ASSIGN(BookmarkChangeProcessor); }; diff --git a/components/brave_sync/test_util.cc b/components/brave_sync/test_util.cc index b7858a045a7e..929ef9bfd5ad 100644 --- a/components/brave_sync/test_util.cc +++ b/components/brave_sync/test_util.cc @@ -18,20 +18,20 @@ #include "components/sync_preferences/pref_service_mock_factory.h" #include "components/sync_preferences/testing_pref_service_syncable.h" -namespace { - -using namespace bookmarks; - -void AddPermanentNode(BookmarkPermanentNodeList* extra_nodes, int64_t id, - const std::string& title) { - auto node = std::make_unique(id); - node->set_type(bookmarks::BookmarkNode::FOLDER); - node->set_visible(false); - node->SetTitle(base::UTF8ToUTF16(title)); - extra_nodes->push_back(std::move(node)); -} - -} // namespace +//namespace { +// +//using namespace bookmarks; +// +//void AddPermanentNode(BookmarkPermanentNodeList* extra_nodes, int64_t id, +// const std::string& title) { +// auto node = std::make_unique(id); +// node->set_type(bookmarks::BookmarkNode::FOLDER); +// node->set_visible(false); +// node->SetTitle(base::UTF8ToUTF16(title)); +// extra_nodes->push_back(std::move(node)); +//} +// +//} // namespace namespace brave_sync { @@ -59,13 +59,13 @@ std::unique_ptr BuildFakeBookmarkModelForTests( // Don't need context, unless we have more than one profile using namespace bookmarks; std::unique_ptr client(new TestBookmarkClient()); - BookmarkPermanentNodeList extra_nodes; + //BookmarkPermanentNodeList extra_nodes; - // These hard-coded titles cannot be changed - AddPermanentNode(&extra_nodes, 0xDE1E7ED40DE, "Deleted Bookmarks"); - AddPermanentNode(&extra_nodes, 0x9E7D17640DE, "Pending Bookmarks"); + //// These hard-coded titles cannot be changed + //AddPermanentNode(&extra_nodes, 0xDE1E7ED40DE, "Deleted Bookmarks"); + //AddPermanentNode(&extra_nodes, 0x9E7D17640DE, "Pending Bookmarks"); - client->SetExtraNodesToLoad(std::move(extra_nodes)); + //client->SetExtraNodesToLoad(std::move(extra_nodes)); std::unique_ptr model( TestBookmarkClient::CreateModelWithClient(std::move(client))); return model;