Replace O(n log n) base::Value::Dict::Clone with O(n)

The previous implementation called Set for each key-value pair to be
cloned, resulting in an unnecessary O(log n) lookup for each of them,
even though we know that the map being cloned is already sorted and
unique.

Fortunately, those same guarantees ensured that the to-be-Set key was
appended to the end of the underlying vector, preventing the previous
implementation from being an even worse O(n^2).

I plan to eliminate the need to call `replace` in a followup CL that
exposes a `base::MakeFlatMap(base::sorted_unique_t, ...)` helper.

Change-Id: Ia0b4e48155be9af28d08eeee42bb90607bdae375
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6023411
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1383284}

base/values.cc

@@ -412,7 +412,19 @@ Value::Dict::iterator Value::Dict::erase(const_iterator pos) {
 }
 
 Value::Dict Value::Dict::Clone() const {
-  return Dict(storage_);
+  std::vector<std::pair<std::string, std::unique_ptr<Value>>> storage;
+  storage.reserve(storage_.size());
+
+  for (const auto& [key, value] : storage_) {
+    storage.emplace_back(key, std::make_unique<Value>(value->Clone()));
+  }
+
+  Dict result;
+  // `storage` is already sorted and unique by construction, which allows us to
+  // avoid an additional O(n log n) step.
+  result.storage_ = flat_map<std::string, std::unique_ptr<Value>>(
+      sorted_unique, std::move(storage));
+  return result;
 }
 
 void Value::Dict::Merge(Dict dict) {
@@ -924,14 +936,6 @@ void Value::Dict::WriteIntoTrace(perfetto::TracedValue context) const {
 }
 #endif  // BUILDFLAG(ENABLE_BASE_TRACING)
 
-Value::Dict::Dict(
-    const flat_map<std::string, std::unique_ptr<Value>>& storage) {
-  storage_.reserve(storage.size());
-  for (const auto& [key, value] : storage) {
-    Set(key, value->Clone());
-  }
-}
-
 bool operator==(const Value::Dict& lhs, const Value::Dict& rhs) {
   auto deref_2nd = [](const auto& p) { return std::tie(p.first, *p.second); };
   return ranges::equal(lhs.storage_, rhs.storage_, {}, deref_2nd, deref_2nd);

base/values.h

@@ -617,8 +617,6 @@ class BASE_EXPORT GSL_OWNER Value {
     BASE_EXPORT friend bool operator<=(const Dict& lhs, const Dict& rhs);
     BASE_EXPORT friend bool operator>=(const Dict& lhs, const Dict& rhs);
 
-    explicit Dict(const flat_map<std::string, std::unique_ptr<Value>>& storage);
-
     // TODO(dcheng): Replace with `flat_map<std::string, Value>` once no caller
     // relies on stability of pointers anymore.
     flat_map<std::string, std::unique_ptr<Value>> storage_;