USSBookmarks migration

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);
 

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

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

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

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

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
 

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,29 +309,34 @@ 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) {
   const auto* folder1 = model()->AddFolder(model()->bookmark_bar_node(), 0,
                                            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);
 
   // appended at the end
   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 =