chromium/chromium-80.0.3987.106-move-RemoteTreeNode-declaration.patch

From cdf3e81ff49b200213d67d65558f2919222b60ab Mon Sep 17 00:00:00 2001
From: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Date: Mon, 16 Dec 2019 11:39:11 +0000
Subject: [PATCH] BookmarkModelMerger: Move RemoteTreeNode declaration to
 header.

This fixes the build with libstdc++ after commit 8f5dad93e58 ("Fix CHECK
failure due to untracked local nodes"):

/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/stl_pair.h:215:11: error: field has incomplete type 'sync_bookmarks::BookmarkModelMerger::RemoteTreeNode'
      _T2 second;                /// @c second is a copy of the second object
          ^
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/ext/aligned_buffer.h:91:28: note: in instantiation of template class 'std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode>' requested here
    : std::aligned_storage<sizeof(_Tp), __alignof__(_Tp)>
                           ^
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/hashtable_policy.h:233:43: note: in instantiation of template class '__gnu_cxx::__aligned_buffer<std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode> >' requested here
      __gnu_cxx::__aligned_buffer<_Value> _M_storage;
                                          ^
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/hashtable_policy.h:264:39: note: in instantiation of template class 'std::__detail::_Hash_node_value_base<std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode> >' requested here
    struct _Hash_node<_Value, true> : _Hash_node_value_base<_Value>
                                      ^
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/hashtable_policy.h:2028:25: note: in instantiation of template class 'std::__detail::_Hash_node<std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode>, true>' requested here
        rebind_traits<typename __node_type::value_type>;
                               ^
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/hashtable.h:184:15: note: in instantiation of template class 'std::__detail::_Hashtable_alloc<std::allocator<std::__detail::_Hash_node<std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode>, true> > >
' requested here
      private __detail::_Hashtable_alloc<
              ^
/usr/lib/gcc/x86_64-redhat-linux/9/../../../../include/c++/9/bits/unordered_map.h:105:18: note: in instantiation of template class 'std::_Hashtable<std::__cxx11::basic_string<char>, std::pair<const std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode>, std::allocator<std::pair<con
st std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode> >, std::__detail::_Select1st, std::equal_to<std::__cxx11::basic_string<char> >, std::hash<std::string>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__deta
il::_Hashtable_traits<true, false, true> >' requested here
      _Hashtable _M_h;
                 ^
../../components/sync_bookmarks/bookmark_model_merger.h:146:22: note: in instantiation of template class 'std::unordered_map<std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode, std::hash<std::string>, std::equal_to<std::__cxx11::basic_string<char> >, std::allocator<std::pair<con
st std::__cxx11::basic_string<char>, sync_bookmarks::BookmarkModelMerger::RemoteTreeNode> > >' requested here
  const RemoteForest remote_forest_;
                     ^
../../components/sync_bookmarks/bookmark_model_merger.h:53:9: note: forward declaration of 'sync_bookmarks::BookmarkModelMerger::RemoteTreeNode'
  class RemoteTreeNode;
        ^

Essentially, the problem is that libstdc++'s std::unordered_map<T, U>
implementation requires both T and U to be fully declared. I raised the
problem in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92770, and GCC's
position is that we are relying on undefined behavior according to the C++
standard (https://eel.is/c++draft/requirements#res.on.functions-2.5).

Bug: 957519
Change-Id: Ife7e435e516932a795bfbe05b2c910c3272878f0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1960156
Commit-Queue: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Auto-Submit: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
Cr-Commit-Position: refs/heads/master@{#725070}
---
 .../sync_bookmarks/bookmark_model_merger.cc   | 89 +++++++------------
 .../sync_bookmarks/bookmark_model_merger.h    | 48 +++++++++-
 2 files changed, 80 insertions(+), 57 deletions(-)

diff --git a/components/sync_bookmarks/bookmark_model_merger.cc b/components/sync_bookmarks/bookmark_model_merger.cc
index eae153eff95..579848ee664 100644
--- a/components/sync_bookmarks/bookmark_model_merger.cc
+++ b/components/sync_bookmarks/bookmark_model_merger.cc
@@ -5,7 +5,6 @@
 #include "components/sync_bookmarks/bookmark_model_merger.h"
 
 #include <algorithm>
-#include <memory>
 #include <set>
 #include <string>
 #include <utility>
@@ -205,66 +204,44 @@ UpdatesPerParentId GroupValidUpdatesByParentId(
 
 }  // namespace
 
-class BookmarkModelMerger::RemoteTreeNode final {
- public:
-  // Constructs a tree given |update| as root and recursively all descendants by
-  // traversing |*updates_per_parent_id|. |update| and |updates_per_parent_id|
-  // must not be null. All updates |*updates_per_parent_id| must represent valid
-  // updates. Updates corresponding from descendant nodes are moved away from
-  // |*updates_per_parent_id|.
-  static RemoteTreeNode BuildTree(
-      std::unique_ptr<syncer::UpdateResponseData> update,
-      UpdatesPerParentId* updates_per_parent_id);
-
-  ~RemoteTreeNode() = default;
-
-  // Allow moves, useful during construction.
-  RemoteTreeNode(RemoteTreeNode&&) = default;
-  RemoteTreeNode& operator=(RemoteTreeNode&&) = default;
-
-  const syncer::EntityData& entity() const { return *update_->entity; }
-  int64_t response_version() const { return update_->response_version; }
-
-  // Direct children nodes, sorted by ascending unique position. These are
-  // guaranteed to be valid updates (e.g. IsValidBookmarkSpecifics()).
-  const std::vector<RemoteTreeNode>& children() const { return children_; }
-
-  // Recursively emplaces all GUIDs (this node and descendants) into
-  // |*guid_to_remote_node_map|, which must not be null.
-  void EmplaceSelfAndDescendantsByGUID(
-      std::unordered_map<std::string, const RemoteTreeNode*>*
-          guid_to_remote_node_map) const {
-    DCHECK(guid_to_remote_node_map);
-
-    const std::string& guid = entity().specifics.bookmark().guid();
-    if (!guid.empty()) {
-      DCHECK(base::IsValidGUID(guid));
-
-      // Duplicate GUIDs have been sorted out before.
-      bool success = guid_to_remote_node_map->emplace(guid, this).second;
-      DCHECK(success);
-    }
+BookmarkModelMerger::RemoteTreeNode::RemoteTreeNode() = default;
 
-    for (const RemoteTreeNode& child : children_) {
-      child.EmplaceSelfAndDescendantsByGUID(guid_to_remote_node_map);
-    }
-  }
+BookmarkModelMerger::RemoteTreeNode::~RemoteTreeNode() = default;
+
+BookmarkModelMerger::RemoteTreeNode::RemoteTreeNode(
+    BookmarkModelMerger::RemoteTreeNode&&) = default;
+BookmarkModelMerger::RemoteTreeNode& BookmarkModelMerger::RemoteTreeNode::
+operator=(BookmarkModelMerger::RemoteTreeNode&&) = default;
+
+void BookmarkModelMerger::RemoteTreeNode::EmplaceSelfAndDescendantsByGUID(
+    std::unordered_map<std::string, const RemoteTreeNode*>*
+        guid_to_remote_node_map) const {
+  DCHECK(guid_to_remote_node_map);
+
+  const std::string& guid = entity().specifics.bookmark().guid();
+  if (!guid.empty()) {
+    DCHECK(base::IsValidGUID(guid));
 
- private:
-  static bool UniquePositionLessThan(const RemoteTreeNode& lhs,
-                                     const RemoteTreeNode& rhs) {
-    const syncer::UniquePosition a_pos =
-        syncer::UniquePosition::FromProto(lhs.entity().unique_position);
-    const syncer::UniquePosition b_pos =
-        syncer::UniquePosition::FromProto(rhs.entity().unique_position);
-    return a_pos.LessThan(b_pos);
+    // Duplicate GUIDs have been sorted out before.
+    bool success = guid_to_remote_node_map->emplace(guid, this).second;
+    DCHECK(success);
   }
 
-  RemoteTreeNode() = default;
+  for (const RemoteTreeNode& child : children_) {
+    child.EmplaceSelfAndDescendantsByGUID(guid_to_remote_node_map);
+  }
+}
 
-  std::unique_ptr<syncer::UpdateResponseData> update_;
-  std::vector<RemoteTreeNode> children_;
-};
+// static
+bool BookmarkModelMerger::RemoteTreeNode::UniquePositionLessThan(
+    const RemoteTreeNode& lhs,
+    const RemoteTreeNode& rhs) {
+  const syncer::UniquePosition a_pos =
+      syncer::UniquePosition::FromProto(lhs.entity().unique_position);
+  const syncer::UniquePosition b_pos =
+      syncer::UniquePosition::FromProto(rhs.entity().unique_position);
+  return a_pos.LessThan(b_pos);
+}
 
 // static
 BookmarkModelMerger::RemoteTreeNode
diff --git a/components/sync_bookmarks/bookmark_model_merger.h b/components/sync_bookmarks/bookmark_model_merger.h
index 9b592000dc5..bf0783ecf8e 100644
--- a/components/sync_bookmarks/bookmark_model_merger.h
+++ b/components/sync_bookmarks/bookmark_model_merger.h
@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_SYNC_BOOKMARKS_BOOKMARK_MODEL_MERGER_H_
 #define COMPONENTS_SYNC_BOOKMARKS_BOOKMARK_MODEL_MERGER_H_
 
+#include <memory>
 #include <string>
 #include <unordered_map>
 #include <vector>
@@ -50,7 +51,52 @@ class BookmarkModelMerger {
 
  private:
   // Internal representation of a remote tree, composed of nodes.
-  class RemoteTreeNode;
+  class RemoteTreeNode final {
+   private:
+    using UpdatesPerParentId =
+        std::unordered_map<base::StringPiece,
+                           syncer::UpdateResponseDataList,
+                           base::StringPieceHash>;
+
+   public:
+    // Constructs a tree given |update| as root and recursively all descendants
+    // by traversing |*updates_per_parent_id|. |update| and
+    // |updates_per_parent_id| must not be null. All updates
+    // |*updates_per_parent_id| must represent valid updates. Updates
+    // corresponding from descendant nodes are moved away from
+    // |*updates_per_parent_id|.
+    static RemoteTreeNode BuildTree(
+        std::unique_ptr<syncer::UpdateResponseData> update,
+        UpdatesPerParentId* updates_per_parent_id);
+
+    ~RemoteTreeNode();
+
+    // Allow moves, useful during construction.
+    RemoteTreeNode(RemoteTreeNode&&);
+    RemoteTreeNode& operator=(RemoteTreeNode&&);
+
+    const syncer::EntityData& entity() const { return *update_->entity; }
+    int64_t response_version() const { return update_->response_version; }
+
+    // Direct children nodes, sorted by ascending unique position. These are
+    // guaranteed to be valid updates (e.g. IsValidBookmarkSpecifics()).
+    const std::vector<RemoteTreeNode>& children() const { return children_; }
+
+    // Recursively emplaces all GUIDs (this node and descendants) into
+    // |*guid_to_remote_node_map|, which must not be null.
+    void EmplaceSelfAndDescendantsByGUID(
+        std::unordered_map<std::string, const RemoteTreeNode*>*
+            guid_to_remote_node_map) const;
+
+   private:
+    static bool UniquePositionLessThan(const RemoteTreeNode& lhs,
+                                       const RemoteTreeNode& rhs);
+
+    RemoteTreeNode();
+
+    std::unique_ptr<syncer::UpdateResponseData> update_;
+    std::vector<RemoteTreeNode> children_;
+  };
 
   // A forest composed of multiple trees where the root of each tree represents
   // a permanent node, keyed by server-defined unique tag of the root.