Implement serialization for NoVarySearchCache

Add a specialization of net::PickleTraits for net::NoVarySearchCache so
that it can be serialized and deserialized to/from a base::Pickle. This
will be used to implement persistence.

Since we need to be able to serialize base::Time objects for this, add a
new header file "net/base/pickle_base_types.h" which adds serialization
support for types from //base that we need. This uses the deprecated
FromInternalValue()/ToInternalValue() methods to allow migrating existing
serialization code in //net to use PickleTraits without changing the
serialization format.

BUG=399562754,382394774

Change-Id: Ic81083995b7661749fcfd84e83841f5944ec3818
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6341087
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1435857}

net/BUILD.gn

@@ -251,6 +251,7 @@ component("net") {
     "base/parse_number.cc",
     "base/parse_number.h",
     "base/pickle.h",
+    "base/pickle_base_types.h",
     "base/pickle_traits.h",
     "base/platform_mime_util.h",
     "base/port_util.cc",
@@ -2674,6 +2675,7 @@ target(_test_target_type, "net_unittests") {
     "base/network_interfaces_unittest.cc",
     "base/network_isolation_key_unittest.cc",
     "base/parse_number_unittest.cc",
+    "base/pickle_base_types_unittest.cc",
     "base/pickle_unittest.cc",
     "base/port_util_unittest.cc",
     "base/prioritized_dispatcher_unittest.cc",

net/base/pickle_base_types.h

@@ -0,0 +1,43 @@
+// Copyright 2025 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Serialization and deserialization code for some //base types that are used in
+// //net. This can't be put in //base because //base can't depend on //net.
+
+#ifndef NET_BASE_PICKLE_BASE_TYPES_H_
+#define NET_BASE_PICKLE_BASE_TYPES_H_
+
+#include <stdint.h>
+
+#include <optional>
+
+#include "base/time/time.h"
+#include "net/base/pickle_traits.h"
+
+namespace net {
+
+template <>
+struct PickleTraits<base::Time> {
+  static void Serialize(base::Pickle& pickle, const base::Time& time) {
+    // For compatibility with existing serialization code in //net, use the
+    // deprecated `ToInternalValue()` method.
+    pickle.WriteInt64(time.ToInternalValue());
+  }
+
+  static std::optional<base::Time> Deserialize(base::PickleIterator& iter) {
+    int64_t time_as_int64;
+    if (!iter.ReadInt64(&time_as_int64)) {
+      return std::nullopt;
+    }
+    return base::Time::FromInternalValue(time_as_int64);
+  }
+
+  static size_t PickleSize(const base::Time& time) {
+    return EstimatePickleSize(int64_t{0});
+  }
+};
+
+}  // namespace net
+
+#endif  // NET_BASE_PICKLE_BASE_TYPES_H_

net/base/pickle_base_types_unittest.cc

@@ -0,0 +1,38 @@
+// Copyright 2025 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/base/pickle_base_types.h"
+
+#include <array>
+
+#include "base/pickle.h"
+#include "base/time/time.h"
+#include "net/base/pickle.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+
+namespace {
+
+using ::testing::Optional;
+
+TEST(PickleBaseTypesTest, Time) {
+  base::Time nov1994;
+  ASSERT_TRUE(
+      base::Time::FromUTCString("Tue, 15 Nov 1994 12:45:26 GMT", &nov1994));
+  static const auto kCases = std::to_array<base::Time>(
+      {base::Time(), base::Time::Max(), base::Time::UnixEpoch(), nov1994});
+  for (base::Time test_case : kCases) {
+    SCOPED_TRACE(test_case);
+    base::Pickle pickle;
+    WriteToPickle(pickle, test_case);
+    EXPECT_EQ(EstimatePickleSize(test_case), pickle.payload_size());
+    EXPECT_THAT(ReadValueFromPickle<base::Time>(pickle), Optional(test_case));
+  }
+}
+
+}  // namespace
+
+}  // namespace net

net/http/no_vary_search_cache.cc

@@ -4,8 +4,10 @@
 
 #include "net/http/no_vary_search_cache.h"
 
+#include <algorithm>
 #include <compare>
 #include <iostream>
+#include <limits>
 #include <tuple>
 #include <type_traits>
 #include <utility>
@@ -15,7 +17,10 @@
 #include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/notreached.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/time/time.h"
+#include "net/base/pickle.h"
+#include "net/base/pickle_base_types.h"
 #include "net/http/http_cache.h"
 #include "net/http/http_no_vary_search_data.h"
 
@@ -184,7 +189,13 @@ class NoVarySearchCache::QueryString final
 
   // Removes this object from both lists and deletes it.
   void RemoveAndDelete() {
-    LruNode::RemoveFromList();
+    // During deserialization a QueryString is not inserted into the `lru_` list
+    // until the end. If deserialization fails before then, it can be deleted
+    // without ever being inserted into the `lru_` list.
+    if (LruNode::next()) {
+      CHECK(LruNode::previous());
+      LruNode::RemoveFromList();
+    }
     QueryStringListNode::RemoveFromList();
     delete this;
   }
@@ -200,9 +211,7 @@ class NoVarySearchCache::QueryString final
 
   const std::optional<std::string>& query() const { return query_; }
 
-  QueryStringList& query_string_list_ref() {
-    return query_string_list_ref_.get();
-  }
+  QueryStringList& query_string_list_ref() { return *query_string_list_ref_; }
 
   base::Time insertion_time() const { return insertion_time_; }
 
@@ -225,12 +234,21 @@ class NoVarySearchCache::QueryString final
     return EraseHandle(weak_factory_.GetWeakPtr());
   }
 
+  void set_query_string_list_ref(
+      base::PassKey<NoVarySearchCache::QueryStringList>,
+      QueryStringList* query_string_list) {
+    query_string_list_ref_ = query_string_list;
+  }
+
  private:
+  friend struct PickleTraits<NoVarySearchCache::QueryStringList>;
+
   QueryString(std::optional<std::string_view> query,
-              QueryStringList& query_string_list)
+              QueryStringList& query_string_list,
+              base::Time insertion_time = base::Time::Now())
       : query_(query),
-        query_string_list_ref_(query_string_list),
-        insertion_time_(base::Time::Now()) {}
+        query_string_list_ref_(&query_string_list),
+        insertion_time_(insertion_time) {}
 
   // Must only be called from RemoveAndDelete().
   ~QueryString() = default;
@@ -254,8 +272,9 @@ class NoVarySearchCache::QueryString final
   const std::optional<std::string> query_;
 
   // `query_string_list_ref_` allows the keys for this entry to be located in
-  // the cache so that it can be erased efficiently.
-  const raw_ref<QueryStringList> query_string_list_ref_;
+  // the cache so that it can be erased efficiently. It is modified when a
+  // QueryStringList object is moved.
+  raw_ptr<QueryStringList> query_string_list_ref_ = nullptr;
 
   // `insertion_time_` breaks ties when there are multiple possible matches. The
   // most recent entry will be used as it is most likely to still exist in the
@@ -300,9 +319,17 @@ bool NoVarySearchCache::EraseHandle::IsGoneForTesting() const {
 }
 
 NoVarySearchCache::NoVarySearchCache(size_t max_size) : max_size_(max_size) {
-  CHECK_GT(max_size_, 0u);
+  CHECK_GE(max_size_, 1u);
+  // We can't serialize if `max_size` won't fit in an int.
+  CHECK(base::IsValueInRangeForNumericType<int>(max_size));
 }
 
+NoVarySearchCache::NoVarySearchCache(NoVarySearchCache&& rhs)
+    : map_(std::move(rhs.map_)),
+      lru_(std::move(rhs.lru_)),
+      size_(std::exchange(rhs.size_, 0u)),
+      max_size_(rhs.max_size_) {}
+
 NoVarySearchCache::~NoVarySearchCache() {
   map_.clear();
   // Clearing the map should have freed all the QueryString objects.
@@ -423,7 +450,7 @@ bool NoVarySearchCache::ClearData(UrlFilterType filter_type,
   // then erase them.
   // TODO(https://crbug.com/382394774): Make this algorithm more efficient.
   std::vector<QueryString*> pending_erase;
-  for (const auto& [cache_key, data_map] : map_) {
+  for (auto& [cache_key, data_map] : map_) {
     const std::string base_url_string =
         HttpCache::GetResourceURLFromHttpCacheKey(cache_key.value());
     const GURL base_url(base_url_string);
@@ -458,9 +485,27 @@ bool NoVarySearchCache::IsTopLevelMapEmptyForTesting() const {
 }
 
 NoVarySearchCache::QueryStringList::QueryStringList(const BaseURLCacheKey& key)
-    : key_ref(key) {}
+    : key_ref(&key) {}
+
+NoVarySearchCache::QueryStringList::QueryStringList() = default;
+
+NoVarySearchCache::QueryStringList::QueryStringList(QueryStringList&& rhs)
+    : list(std::move(rhs.list)) {
+  // We should not move a list after the key references have been assigned.
+  CHECK(!rhs.nvs_data_ref);
+  CHECK(!rhs.key_ref);
+  // We have to patch up all the references to `rhs` in our QueryString objects
+  // to point to us instead.
+  ForEachQueryString(list, [&](QueryString* query_string) {
+    query_string->set_query_string_list_ref(base::PassKey<QueryStringList>(),
+                                            this);
+  });
+}
 
 NoVarySearchCache::QueryStringList::~QueryStringList() {
+  // The `list.head()` check works around the unfortunate fact that moving from
+  // a base::LinkedList leaves it in an invalid state where `list.empty()` is
+  // false.
   while (!list.empty()) {
     list.head()->value()->ToQueryString()->RemoveAndDelete();
   }
@@ -481,11 +526,11 @@ void NoVarySearchCache::EraseQuery(QueryString* query_string) {
   const QueryStringList& query_strings = query_string->query_string_list_ref();
   query_string->RemoveAndDelete();
   if (query_strings.list.empty()) {
-    const HttpNoVarySearchData* nvs_data_ref = query_strings.nvs_data_ref.get();
-    const BaseURLCacheKey& key_ref = query_strings.key_ref.get();
+    const HttpNoVarySearchData& nvs_data_ref = *query_strings.nvs_data_ref;
+    const BaseURLCacheKey& key_ref = *query_strings.key_ref;
     const auto map_it = map_.find(key_ref);
     CHECK(map_it != map_.end());
-    const size_t removed_count = map_it->second.erase(*nvs_data_ref);
+    const size_t removed_count = map_it->second.erase(nvs_data_ref);
     CHECK_EQ(removed_count, 1u);
     if (map_it->second.empty()) {
       map_.erase(map_it);
@@ -495,20 +540,18 @@ void NoVarySearchCache::EraseQuery(QueryString* query_string) {
 
 // static
 void NoVarySearchCache::FindQueryStringsInTimeRange(
-    const DataMapType& data_map,
+    DataMapType& data_map,
     base::Time delete_begin,
     base::Time delete_end,
     std::vector<QueryString*>& matches) {
-  for (const auto& [_, query_string_list] : data_map) {
-    for (auto* node = query_string_list.list.head();
-         node != query_string_list.list.end(); node = node->next()) {
-      QueryString* query_string = node->value()->ToQueryString();
+  for (auto& [_, query_string_list] : data_map) {
+    ForEachQueryString(query_string_list.list, [&](QueryString* query_string) {
       const base::Time insertion_time = query_string->insertion_time();
       if ((delete_begin.is_null() || delete_begin <= insertion_time) &&
           (delete_end.is_max() || delete_end > insertion_time)) {
         matches.push_back(query_string);
       }
-    }
+    });
   }
 }
 
@@ -531,4 +574,189 @@ NoVarySearchCache::FindQueryStringInList(QueryStringList& query_strings,
   return std::nullopt;
 }
 
+// static
+void NoVarySearchCache::ForEachQueryString(
+    base::LinkedList<QueryStringListNode>& list,
+    base::FunctionRef<void(QueryString*)> f) {
+  for (auto* node = list.head(); node != list.end(); node = node->next()) {
+    QueryString* query_string = node->value()->ToQueryString();
+    f(query_string);
+  }
+}
+
+// static
+void NoVarySearchCache::ForEachQueryString(
+    const base::LinkedList<QueryStringListNode>& list,
+    base::FunctionRef<void(const QueryString*)> f) {
+  for (auto* node = list.head(); node != list.end(); node = node->next()) {
+    const QueryString* query_string = node->value()->ToQueryString();
+    f(query_string);
+  }
+}
+
+template <>
+struct PickleTraits<NoVarySearchCache::QueryStringList> {
+  static void Serialize(
+      base::Pickle& pickle,
+      const NoVarySearchCache::QueryStringList& query_strings) {
+    // base::LinkedList doesn't keep an element count, so we need to count them
+    // ourselves.
+    size_t size = 0u;
+    for (auto* node = query_strings.list.head();
+         node != query_strings.list.end(); node = node->next()) {
+      ++size;
+    }
+    WriteToPickle(pickle, base::checked_cast<int>(size));
+    NoVarySearchCache::ForEachQueryString(
+        query_strings.list,
+        [&](const NoVarySearchCache::QueryString* query_string) {
+          WriteToPickle(pickle, query_string->query_,
+                        query_string->insertion_time_);
+        });
+  }
+
+  static std::optional<NoVarySearchCache::QueryStringList> Deserialize(
+      base::PickleIterator& iter) {
+    NoVarySearchCache::QueryStringList query_string_list;
+    size_t size = 0;
+    if (!iter.ReadLength(&size)) {
+      return std::nullopt;
+    }
+    for (size_t i = 0; i < size; ++i) {
+      // QueryString is not movable or copyable, so it won't work well with
+      // PickleTraits. Deserialize it inline instead.
+      auto result =
+          ReadValuesFromPickle<std::optional<std::string>, base::Time>(iter);
+      if (!result) {
+        return std::nullopt;
+      }
+      auto [query, insertion_time] = std::move(result).value();
+      if (query && query->find('#') != std::string_view::npos) {
+        // A '#' character must not appear in the query.
+        return std::nullopt;
+      }
+      auto* query_string = new NoVarySearchCache::QueryString(
+          std::move(query), query_string_list, insertion_time);
+      // Serialization happens from head to tail, so to deserialize in the same
+      // order, we add elements at the tail of the list.
+      query_string_list.list.Append(query_string);
+    }
+    return query_string_list;
+  }
+
+  static size_t PickleSize(
+      const NoVarySearchCache::QueryStringList& query_strings) {
+    size_t estimate = EstimatePickleSize(int{});
+    NoVarySearchCache::ForEachQueryString(
+        query_strings.list,
+        [&](const NoVarySearchCache::QueryString* query_string) {
+          estimate += EstimatePickleSize(query_string->query_,
+                                         query_string->insertion_time_);
+        });
+    return estimate;
+  }
+};
+
+template <>
+struct PickleTraits<NoVarySearchCache::BaseURLCacheKey> {
+  static void Serialize(base::Pickle& pickle,
+                        const NoVarySearchCache::BaseURLCacheKey& key) {
+    WriteToPickle(pickle, *key);
+  }
+
+  static std::optional<NoVarySearchCache::BaseURLCacheKey> Deserialize(
+      base::PickleIterator& iter) {
+    NoVarySearchCache::BaseURLCacheKey key;
+    if (!ReadPickleInto(iter, *key)) {
+      return std::nullopt;
+    }
+    return key;
+  }
+
+  static size_t PickleSize(const NoVarySearchCache::BaseURLCacheKey& key) {
+    return EstimatePickleSize(*key);
+  }
+};
+
+// static
+void PickleTraits<NoVarySearchCache>::Serialize(
+    base::Pickle& pickle,
+    const NoVarySearchCache& cache) {
+  // `size_t` is different sizes on 32-bit and 64-bit platforms. For a
+  // consistent format, serialize as int. This will crash if someone creates a
+  // NoVarySearchCache which supports over 2 billion entries, which would be a
+  // terrible idea anyway.
+  int max_size_as_int = base::checked_cast<int>(cache.max_size_);
+  int size_as_int = base::checked_cast<int>(cache.size_);
+
+  // `lru_` is reconstructed during deserialization and so doesn't need to be
+  // stored explicitly.
+  WriteToPickle(pickle, size_as_int, max_size_as_int, cache.map_);
+}
+
+// static
+std::optional<NoVarySearchCache> PickleTraits<NoVarySearchCache>::Deserialize(
+    base::PickleIterator& iter) {
+  const std::optional<int> maybe_size = ReadValueFromPickle<int>(iter);
+  if (!maybe_size || *maybe_size < 0) {
+    return std::nullopt;
+  }
+  const size_t size = static_cast<size_t>(*maybe_size);
+  const std::optional<int> maybe_max_size = ReadValueFromPickle<int>(iter);
+  if (!maybe_max_size || *maybe_max_size < 1) {
+    return std::nullopt;
+  }
+  const size_t max_size = static_cast<size_t>(*maybe_max_size);
+
+  if (size > max_size) {
+    return std::nullopt;
+  }
+
+  NoVarySearchCache cache(max_size);
+  cache.size_ = size;
+  if (!ReadPickleInto(iter, cache.map_)) {
+    return std::nullopt;
+  }
+
+  using QueryString = NoVarySearchCache::QueryString;
+  // Get a list of every QueryString object in the map so that we can sort
+  // them to reconstruct the `lru_` list.
+  std::vector<QueryString*> all_query_strings;
+  all_query_strings.reserve(size);
+  for (auto& [base_url_cache_key, data_map] : cache.map_) {
+    for (auto& [nvs_data, query_string_list] : data_map) {
+      query_string_list.nvs_data_ref = &nvs_data;
+      query_string_list.key_ref = &base_url_cache_key;
+      NoVarySearchCache::ForEachQueryString(
+          query_string_list.list, [&](QueryString* query_string) {
+            all_query_strings.push_back(query_string);
+          });
+    }
+  }
+  if (size != all_query_strings.size()) {
+    return std::nullopt;
+  }
+
+  // Sort by `insertion_time`, which we use as an approximation of `use_time`
+  // during deserialization on the assumption that it won't make much
+  // difference.
+  std::ranges::sort(all_query_strings, std::less<base::Time>(),
+                    [](QueryString* qs) { return qs->insertion_time(); });
+
+  // Insert each entry at the head of the list, so that the oldest entry ends
+  // up at the tail.
+  for (QueryString* qs : all_query_strings) {
+    qs->LruNode::InsertBefore(cache.lru_.head());
+  }
+
+  return cache;
+}
+
+// static
+size_t PickleTraits<NoVarySearchCache>::PickleSize(
+    const NoVarySearchCache& cache) {
+  // `size_` and `max_size_` are pickled as ints.
+  return EstimatePickleSize(int{}, int{}, cache.map_);
+}
+
 }  // namespace net

net/http/no_vary_search_cache.h

@@ -17,6 +17,7 @@
 #include <vector>
 
 #include "base/containers/linked_list.h"
+#include "base/functional/function_ref.h"
 #include "base/memory/raw_ptr.h"
 #include "base/memory/raw_ref.h"
 #include "base/memory/stack_allocated.h"
@@ -24,6 +25,7 @@
 #include "base/types/strong_alias.h"
 #include "net/base/does_url_match_filter.h"
 #include "net/base/net_export.h"
+#include "net/base/pickle_traits.h"
 #include "net/http/http_no_vary_search_data.h"
 #include "net/http/http_request_info.h"
 #include "url/gurl.h"
@@ -90,9 +92,13 @@ class NET_EXPORT_PRIVATE NoVarySearchCache {
   // bytes.
   explicit NoVarySearchCache(size_t max_size);
 
-  // Not copyable, assignable or movable.
+  // Move-constructible to permit deserialization and passing between threads.
+  NoVarySearchCache(NoVarySearchCache&&);
+
+  // Not copyable or assignable.
   NoVarySearchCache(const NoVarySearchCache&) = delete;
   NoVarySearchCache& operator=(const NoVarySearchCache&) = delete;
+  NoVarySearchCache& operator=(NoVarySearchCache&&) = delete;
 
   ~NoVarySearchCache();
 
@@ -137,23 +143,39 @@ class NET_EXPORT_PRIVATE NoVarySearchCache {
   bool IsTopLevelMapEmptyForTesting() const;
 
  private:
-  class LruNode;
-  class QueryStringListNode;
+  friend struct PickleTraits<NoVarySearchCache>;
+
+  struct QueryStringList;
+  friend struct PickleTraits<NoVarySearchCache::QueryStringList>;
 
   using BaseURLCacheKey =
       base::StrongAlias<struct BaseURLCacheKeyTagType, std::string>;
+  friend struct PickleTraits<NoVarySearchCache::BaseURLCacheKey>;
+
+  class LruNode;
+  class QueryStringListNode;
 
   struct QueryStringList {
     base::LinkedList<QueryStringListNode> list;
     // nvs_data_ref can't be raw_ref because it needs to be lazily initialized
     // after the QueryStringList has been added to the map.
-    raw_ptr<const HttpNoVarySearchData> nvs_data_ref;
-    raw_ref<const BaseURLCacheKey> key_ref;
+    raw_ptr<const HttpNoVarySearchData> nvs_data_ref = nullptr;
+
+    // key_ref can't be raw_ref because it needs to be added in a second pass
+    // during deserialization.
+    raw_ptr<const BaseURLCacheKey> key_ref = nullptr;
 
     // The referent of this reference has to be the actual key in the map. It is
     // not sufficient for the value to match, because the lifetime has to be the
     // same.
     explicit QueryStringList(const BaseURLCacheKey& key);
+
+    // Needed during deserialization.
+    QueryStringList();
+
+    // Only used during deserialization. This is O(N) in the size of `list`.
+    QueryStringList(QueryStringList&&);
+
     // base::LinkedList<> does not do memory management, so make sure the
     // contents of `list` are deleted on destruction.
     ~QueryStringList();
@@ -178,8 +200,11 @@ class NET_EXPORT_PRIVATE NoVarySearchCache {
   void EraseQuery(QueryString* query_string);
 
   // Scans all the QueryStrings in `data_map` to find ones in the range
-  // [delete_begin, delete_end) and appends them to `matches`.
-  static void FindQueryStringsInTimeRange(const DataMapType& data_map,
+  // [delete_begin, delete_end) and appends them to `matches`. `data_map` is
+  // mutable to reflect that it is returning mutable pointers to QueryString
+  // objects that it owns. The returned QueryString objects are mutable so the
+  // caller can erase them.
+  static void FindQueryStringsInTimeRange(DataMapType& data_map,
                                           base::Time delete_begin,
                                           base::Time delete_end,
                                           std::vector<QueryString*>& matches);
@@ -190,6 +215,15 @@ class NET_EXPORT_PRIVATE NoVarySearchCache {
       const GURL& url,
       const HttpNoVarySearchData& nvs_data);
 
+  // Calls f(query_string_ptr) for every QueryString in `list`.
+  static void ForEachQueryString(base::LinkedList<QueryStringListNode>& list,
+                                 base::FunctionRef<void(QueryString*)> f);
+
+  // Calls f(const_query_string_ptr) for every QueryString in `list`.
+  static void ForEachQueryString(
+      const base::LinkedList<QueryStringListNode>& list,
+      base::FunctionRef<void(const QueryString*)> f);
+
   // The main cache data structure.
   OuterMapType map_;
 
@@ -203,6 +237,16 @@ class NET_EXPORT_PRIVATE NoVarySearchCache {
   const size_t max_size_;
 };
 
+template <>
+struct NET_EXPORT_PRIVATE PickleTraits<NoVarySearchCache> {
+  static void Serialize(base::Pickle& pickle, const NoVarySearchCache& cache);
+
+  static std::optional<NoVarySearchCache> Deserialize(
+      base::PickleIterator& iter);
+
+  static size_t PickleSize(const NoVarySearchCache& cache);
+};
+
 }  // namespace net
 
 #endif  // NET_HTTP_NO_VARY_SEARCH_CACHE_H_

net/http/no_vary_search_cache_unittest.cc

@@ -10,6 +10,7 @@
 #include <string_view>
 #include <utility>
 
+#include "base/pickle.h"
 #include "base/strings/strcat.h"
 #include "base/test/scoped_feature_list.h"
 #include "base/test/task_environment.h"
@@ -18,6 +19,8 @@
 #include "net/base/features.h"
 #include "net/base/load_flags.h"
 #include "net/base/network_isolation_key.h"
+#include "net/base/pickle.h"
+#include "net/base/pickle_traits.h"
 #include "net/base/schemeful_site.h"
 #include "net/http/http_cache.h"
 #include "net/http/http_response_headers.h"
@@ -171,6 +174,19 @@ TEST_P(NoVarySearchCacheTest, InsertLookupErase) {
   EXPECT_TRUE(cache().IsTopLevelMapEmptyForTesting());
 }
 
+TEST_P(NoVarySearchCacheTest, MoveConstruct) {
+  Insert("a=b", "key-order");
+
+  NoVarySearchCache new_cache = std::move(cache());
+
+  EXPECT_TRUE(new_cache.Lookup(TestRequest("a=b")));
+
+  // NOLINTNEXTLINE(bugprone-use-after-move)
+  EXPECT_EQ(cache().GetSizeForTesting(), 0u);
+  // NOLINTNEXTLINE(bugprone-use-after-move)
+  EXPECT_TRUE(cache().IsTopLevelMapEmptyForTesting());
+}
+
 // An asan build will find leaks, but this test works on any build.
 TEST_P(NoVarySearchCacheTest, QueryNotLeaked) {
   std::optional<NoVarySearchCache::LookupResult> result;
@@ -185,10 +201,16 @@ TEST_P(NoVarySearchCacheTest, QueryNotLeaked) {
   EXPECT_TRUE(result->erase_handle.IsGoneForTesting());
 }
 
+std::string QueryWithIParameter(size_t i) {
+  return "i=" + base::NumberToString(i);
+}
+
+constexpr std::string_view kVaryOnIParameter = "params, except=(\"i\")";
+
 TEST_P(NoVarySearchCacheTest, OldestItemIsEvicted) {
   for (size_t i = 0; i < kMaxSize + 1; ++i) {
-    std::string query = "i=" + base::NumberToString(i);
-    Insert(query, "params, except=(\"i\")");
+    std::string query = QueryWithIParameter(i);
+    Insert(query, kVaryOnIParameter);
     EXPECT_TRUE(Exists(query));
   }
 
@@ -199,8 +221,8 @@ TEST_P(NoVarySearchCacheTest, OldestItemIsEvicted) {
 
 TEST_P(NoVarySearchCacheTest, RecentlyUsedItemIsNotEvicted) {
   for (size_t i = 0; i < kMaxSize + 1; ++i) {
-    std::string query = "i=" + base::NumberToString(i);
-    Insert(query, "params, except=(\"i\")");
+    std::string query = QueryWithIParameter(i);
+    Insert(query, kVaryOnIParameter);
     EXPECT_TRUE(Exists(query));
     // Exists() calls Lookup(), which makes an entry "used".
     EXPECT_TRUE(Exists("i=0"));
@@ -213,11 +235,10 @@ TEST_P(NoVarySearchCacheTest, RecentlyUsedItemIsNotEvicted) {
 }
 
 TEST_P(NoVarySearchCacheTest, MostRecentlyUsedItemIsNotEvicted) {
-  static constexpr char kNoVarySearchValue[] = "params, except=(\"i\")";
-  const auto query = [](int i) { return "i=" + base::NumberToString(i); };
+  const auto query = QueryWithIParameter;
   // Fill the cache.
   for (size_t i = 0; i < kMaxSize; ++i) {
-    Insert(query(i), kNoVarySearchValue);
+    Insert(query(i), kVaryOnIParameter);
   }
   EXPECT_EQ(cache().GetSizeForTesting(), kMaxSize);
 
@@ -226,7 +247,7 @@ TEST_P(NoVarySearchCacheTest, MostRecentlyUsedItemIsNotEvicted) {
 
   // Evict kMaxSize - 1 items.
   for (size_t i = kMaxSize; i < kMaxSize * 2 - 1; ++i) {
-    Insert(query(i), kNoVarySearchValue);
+    Insert(query(i), kVaryOnIParameter);
     EXPECT_TRUE(Exists(query(i)));
   }
 
@@ -236,11 +257,10 @@ TEST_P(NoVarySearchCacheTest, MostRecentlyUsedItemIsNotEvicted) {
 }
 
 TEST_P(NoVarySearchCacheTest, LeastRecentlyUsedItemIsEvicted) {
-  static constexpr char kNoVarySearchValue[] = "params, except=(\"i\")";
-  const auto query = [](int i) { return "i=" + base::NumberToString(i); };
+  const auto query = QueryWithIParameter;
   // Fill the cache.
   for (size_t i = 0; i < kMaxSize; ++i) {
-    Insert(query(i), kNoVarySearchValue);
+    Insert(query(i), kVaryOnIParameter);
   }
   EXPECT_EQ(cache().GetSizeForTesting(), kMaxSize);
 
@@ -250,7 +270,7 @@ TEST_P(NoVarySearchCacheTest, LeastRecentlyUsedItemIsEvicted) {
   }
 
   // Evict one item.
-  Insert(query(kMaxSize), kNoVarySearchValue);
+  Insert(query(kMaxSize), kVaryOnIParameter);
 
   // Verify it was the least-recently-used item.
   EXPECT_FALSE(Exists(query(kMaxSize - 1)));
@@ -319,8 +339,7 @@ TEST_P(NoVarySearchCacheTest, InsertWithBaseURLMatchingEvicted) {
   cache().MaybeInsert(my_test_request("will-be-evicted"),
                       TestHeaders("key-order"));
   for (size_t i = 1; i < kMaxSize; ++i) {
-    std::string query = "i=" + base::NumberToString(i);
-    Insert(query, "params, except=(\"i\")");
+    Insert(QueryWithIParameter(i), kVaryOnIParameter);
   }
   EXPECT_EQ(cache().GetSizeForTesting(), kMaxSize);
 
@@ -336,8 +355,7 @@ TEST_P(NoVarySearchCacheTest, InsertWithBaseURLMatchingEvicted) {
 TEST_P(NoVarySearchCacheTest, InsertWithNoVarySearchValueMatchingEvicted) {
   Insert("will-be-evicted", "params=(\"ignored\")");
   for (size_t i = 1; i < kMaxSize; ++i) {
-    std::string query = "i=" + base::NumberToString(i);
-    Insert(query, "params, except=(\"i\")");
+    Insert(QueryWithIParameter(i), kVaryOnIParameter);
   }
   EXPECT_EQ(cache().GetSizeForTesting(), kMaxSize);
 
@@ -746,6 +764,114 @@ TEST_P(NoVarySearchCacheTest, ClearDataNoMatch) {
   EXPECT_TRUE(Exists("a=1"));
 }
 
+std::optional<NoVarySearchCache> TestPickleRoundTrip(
+    const NoVarySearchCache& cache) {
+  base::Pickle pickle;
+  WriteToPickle(pickle, cache);
+  // The estimate of PickeSize should always be correct.
+  EXPECT_EQ(EstimatePickleSize(cache), pickle.payload_size());
+  auto maybe_cache = ReadValueFromPickle<NoVarySearchCache>(pickle);
+  if (!maybe_cache) {
+    return std::nullopt;
+  }
+
+  EXPECT_EQ(cache.GetSizeForTesting(), maybe_cache->GetSizeForTesting());
+  return maybe_cache;
+}
+
+TEST_P(NoVarySearchCacheTest, SerializeDeserializeEmpty) {
+  EXPECT_TRUE(TestPickleRoundTrip(cache()));
+}
+
+TEST_P(NoVarySearchCacheTest, SerializeDeserializeSimple) {
+  Insert("b=1", "key-order");
+  Insert("c&d", "key-order");
+  Insert("f=3", "params=(\"a\")");
+
+  auto new_cache = TestPickleRoundTrip(cache());
+  ASSERT_TRUE(new_cache);
+
+  const auto lookup = [&](std::string_view params) {
+    return new_cache->Lookup(TestRequest(params));
+  };
+
+  auto maybe_handle1 = lookup("b=1");
+  auto maybe_handle2 = lookup("d&c");
+  auto maybe_handle3 = lookup("f=3&a=7");
+
+  ASSERT_TRUE(maybe_handle1);
+  ASSERT_TRUE(maybe_handle2);
+  ASSERT_TRUE(maybe_handle3);
+
+  new_cache->Erase(std::move(maybe_handle1->erase_handle));
+  new_cache->Erase(std::move(maybe_handle2->erase_handle));
+  new_cache->Erase(std::move(maybe_handle3->erase_handle));
+
+  EXPECT_EQ(new_cache->GetSizeForTesting(), 0u);
+  EXPECT_TRUE(new_cache->IsTopLevelMapEmptyForTesting());
+}
+
+TEST_P(NoVarySearchCacheTest, SerializeDeserializeFull) {
+  for (size_t i = 0; i < kMaxSize; ++i) {
+    Insert(QueryWithIParameter(i), kVaryOnIParameter);
+  }
+
+  auto new_cache = TestPickleRoundTrip(cache());
+  ASSERT_TRUE(new_cache);
+
+  for (size_t i = 0; i < kMaxSize; ++i) {
+    EXPECT_TRUE(new_cache->Lookup(TestRequest(QueryWithIParameter(i))));
+  }
+}
+
+TEST_P(NoVarySearchCacheTest, DeserializeBadSizes) {
+  struct TestCase {
+    std::string_view test_description;
+    int size;
+    int max_size;
+    int map_size;
+  };
+  static constexpr auto kTestCases = std::to_array<TestCase>({
+      {"Negative size", -1, 1, 0},
+      {"Size larger than max_size", 2, 1, 0},
+      {"Size bigger than map contents", 1, 1, 0},
+      {"Negative max_size", 0, -1, 0},
+      {"Zero max_size", 0, 0, 0},
+      {"Negative map size", 0, 1, -1},
+      {"Map size larger than map contents", 0, 1, 1},
+  });
+
+  for (const auto& test_case : kTestCases) {
+    SCOPED_TRACE(test_case.test_description);
+    base::Pickle pickle;
+    // This uses the fact that containers use an integer for size.
+    WriteToPickle(pickle, test_case.size, test_case.max_size,
+                  test_case.map_size);
+    EXPECT_FALSE(ReadValueFromPickle<NoVarySearchCache>(pickle));
+  }
+}
+
+// A truncated Pickle should never deserialize to a NoVarySearchCache object.
+// This tests covers many different checks for bad data during deserialization.
+TEST_P(NoVarySearchCacheTest, TruncatedPickle) {
+  Insert("a=9&b=1", "params=(\"a\")");
+  Insert("a=8&b=2", "params=(\"a\")");
+  Insert("f=3", "params, except=(\"f\")");
+  Insert("", "params, except=(\"f\")");
+
+  base::Pickle pickle;
+  WriteToPickle(pickle, cache());
+
+  // Go up in increments of 4 bytes because a Pickle with a size that is not a
+  // multiple of 4 is invalid in a way that is not interesting to this test.
+  for (size_t bytes = 4u; bytes < pickle.payload_size(); bytes += 4) {
+    SCOPED_TRACE(bytes);
+    base::Pickle truncated;
+    truncated.WriteBytes(pickle.payload_bytes().first(bytes));
+    EXPECT_FALSE(ReadValueFromPickle<NoVarySearchCache>(truncated));
+  }
+}
+
 INSTANTIATE_TEST_SUITE_P(All,
                          NoVarySearchCacheTest,
                          ::testing::Bool(),