Add functionality to truncate WebView saveState.

Actually using said functionality will come in a follow up CL.

Bug: 389076708
Change-Id: Ic7a595b636aa7a133df8cae28a5a3248b069887b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6264446
Commit-Queue: Peter Conn <peconn@chromium.org>
Reviewed-by: Richard (Torne) Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1421439}

android_webview/browser/aw_contents.cc

@@ -1088,12 +1088,16 @@ base::android::ScopedJavaLocalRef<jbyteArray> AwContents::GetOpaqueState(
   if (web_contents_->GetController()
           .GetLastCommittedEntry()
           ->IsInitialEntry()) {
-    return ScopedJavaLocalRef<jbyteArray>();
+    return nullptr;
   }
 
-  base::Pickle pickle;
-  WriteToPickle(*web_contents_, &pickle);
-  return base::android::ToJavaByteArray(env, pickle);
+  std::optional<base::Pickle> pickle = WriteToPickle(*web_contents_);
+
+  if (!pickle.has_value()) {
+    return nullptr;
+  }
+
+  return base::android::ToJavaByteArray(env, *pickle);
 }
 
 jboolean AwContents::RestoreFromOpaqueState(

android_webview/browser/state_serializer.cc

@@ -4,7 +4,9 @@
 
 #include "android_webview/browser/state_serializer.h"
 
+#include <algorithm>
 #include <memory>
+#include <optional>
 #include <string>
 #include <vector>
 
@@ -35,7 +37,7 @@ namespace android_webview {
 
 namespace {
 
-const uint32_t AW_STATE_VERSION = internal::AW_STATE_VERSION_DATA_URL;
+const uint32_t AW_STATE_VERSION = internal::AW_STATE_VERSION_MOST_RECENT_FIRST;
 
 // The production implementation of NavigationHistory and NavigationHistorySink,
 // backed by a NavigationController.
@@ -69,11 +71,55 @@ class NavigationControllerWrapper : public internal::NavigationHistory,
   const raw_ptr<content::NavigationController> controller_;
 };
 
+bool RestoreFromPickleLegacy_VersionDataUrl(
+    uint32_t state_version,
+    base::PickleIterator* iterator,
+    internal::NavigationHistorySink& sink) {
+  int entry_count = -1;
+  int selected_entry = -2;  // -1 is a valid value
+
+  if (!iterator->ReadInt(&entry_count)) {
+    return false;
+  }
+
+  if (!iterator->ReadInt(&selected_entry)) {
+    return false;
+  }
+
+  if (entry_count < 0) {
+    return false;
+  }
+  if (selected_entry < -1) {
+    return false;
+  }
+  if (selected_entry >= entry_count) {
+    return false;
+  }
+
+  std::unique_ptr<content::NavigationEntryRestoreContext> context =
+      content::NavigationEntryRestoreContext::Create();
+  std::vector<std::unique_ptr<content::NavigationEntry>> entries;
+  entries.reserve(entry_count);
+  for (int i = 0; i < entry_count; ++i) {
+    entries.push_back(content::NavigationEntry::Create());
+    if (!internal::RestoreNavigationEntryFromPickle(
+            state_version, iterator, entries[i].get(), context.get())) {
+      return false;
+    }
+  }
+
+  // |web_contents| takes ownership of these entries after this call.
+  sink.Restore(selected_entry, &entries);
+  DCHECK_EQ(0u, entries.size());
+
+  return true;
+}
+
 }  // namespace
 
-void WriteToPickle(content::WebContents& web_contents, base::Pickle* pickle) {
+std::optional<base::Pickle> WriteToPickle(content::WebContents& web_contents) {
   NavigationControllerWrapper wrapper(&web_contents.GetController());
-  internal::WriteToPickle(wrapper, pickle);
+  return internal::WriteToPickle(wrapper);
 }
 
 bool RestoreFromPickle(base::PickleIterator* iterator,
@@ -85,10 +131,12 @@ bool RestoreFromPickle(base::PickleIterator* iterator,
 
 namespace internal {
 
-void WriteToPickle(NavigationHistory& history, base::Pickle* pickle) {
-  DCHECK(pickle);
+std::optional<base::Pickle> WriteToPickle(NavigationHistory& history,
+                                          size_t max_size,
+                                          bool save_forward_history) {
+  base::Pickle pickle;
 
-  internal::WriteHeaderToPickle(pickle);
+  internal::WriteHeaderToPickle(AW_STATE_VERSION, &pickle);
 
   const int entry_count = history.GetEntryCount();
   const int selected_entry = history.GetCurrentEntry();
@@ -98,16 +146,58 @@ void WriteToPickle(NavigationHistory& history, base::Pickle* pickle) {
   DCHECK_GE(selected_entry, 0);
   DCHECK_LT(selected_entry, entry_count);
 
-  pickle->WriteInt(entry_count);
-  pickle->WriteInt(selected_entry);
-  for (int i = 0; i < entry_count; ++i) {
-    internal::WriteNavigationEntryToPickle(*history.GetEntryAtIndex(i), pickle);
+  // Navigations are stored in reverse order, allowing us to prioritise the more
+  // recent history entries and stop writing once we exceed the size limit. To
+  // know the size of a navigation entry we've got to serialize it, so to avoid
+  // doing unnecessary work we write the entry, then check if we've exceeded the
+  // limit
+  bool selected_entry_was_saved = false;
+
+  int start_entry = save_forward_history ? entry_count - 1 : selected_entry;
+  for (int i = start_entry; i >= 0; --i) {
+    // Note the difference between |payload_size|, used here and |size| used in
+    // the conditional below. |size| gives the total size of the Pickle, so is
+    // relevant for the size limit, |payload_size| gives the size of the data
+    // we've written (without the Pickle's internal header), so is relevant when
+    // we're copying the payload.
+    size_t payload_size_before_adding_entry = pickle.payload_size();
+
+    pickle.WriteBool(i == selected_entry);
+    internal::WriteNavigationEntryToPickle(*history.GetEntryAtIndex(i),
+                                           &pickle);
+
+    if (pickle.size() > max_size) {
+      if (i == start_entry) {
+        // If not even a single entry can fit into the max size, return nullopt.
+        return std::nullopt;
+      }
+
+      // This should happen rarely, but it's possible that the selected entry
+      // was far enough back in history that it was cut off. In this case, rerun
+      // with save_forward_history = false, ensuring that the current entry is
+      // the first one written.
+      if (!selected_entry_was_saved) {
+        return WriteToPickle(history, max_size,
+                             /* save_forward_history= */ false);
+      }
+
+      base::Pickle new_pickle;
+      new_pickle.WriteBytes(pickle.payload_bytes().subspan(
+          (size_t)0, payload_size_before_adding_entry));
+      return new_pickle;
+    }
+
+    if (i == selected_entry) {
+      selected_entry_was_saved = true;
+    }
   }
 
   // Please update AW_STATE_VERSION and IsSupportedVersion() if serialization
   // format is changed.
   // Make sure the serialization format is updated in a backwards compatible
   // way.
+
+  return pickle;
 }
 
 void WriteHeaderToPickle(base::Pickle* pickle) {
@@ -120,8 +210,9 @@ void WriteHeaderToPickle(uint32_t state_version, base::Pickle* pickle) {
 
 uint32_t RestoreHeaderFromPickle(base::PickleIterator* iterator) {
   uint32_t state_version = -1;
-  if (!iterator->ReadUInt32(&state_version))
+  if (!iterator->ReadUInt32(&state_version)) {
     return 0;
+  }
 
   if (IsSupportedVersion(state_version)) {
     return state_version;
@@ -132,7 +223,8 @@ uint32_t RestoreHeaderFromPickle(base::PickleIterator* iterator) {
 
 bool IsSupportedVersion(uint32_t state_version) {
   return state_version == internal::AW_STATE_VERSION_INITIAL ||
-         state_version == internal::AW_STATE_VERSION_DATA_URL;
+         state_version == internal::AW_STATE_VERSION_DATA_URL ||
+         state_version == internal::AW_STATE_VERSION_MOST_RECENT_FIRST;
 }
 
 void WriteNavigationEntryToPickle(content::NavigationEntry& entry,
@@ -188,43 +280,45 @@ bool RestoreFromPickle(base::PickleIterator* iterator,
     return false;
   }
 
-  int entry_count = -1;
-  int selected_entry = -2;  // -1 is a valid value
-
-  if (!iterator->ReadInt(&entry_count)) {
-    return false;
-  }
-
-  if (!iterator->ReadInt(&selected_entry)) {
-    return false;
-  }
-
-  if (entry_count < 0) {
-    return false;
-  }
-  if (selected_entry < -1) {
-    return false;
-  }
-  if (selected_entry >= entry_count) {
-    return false;
+  if (state_version < AW_STATE_VERSION_MOST_RECENT_FIRST) {
+    return RestoreFromPickleLegacy_VersionDataUrl(state_version, iterator,
+                                                  sink);
   }
 
   std::unique_ptr<content::NavigationEntryRestoreContext> context =
       content::NavigationEntryRestoreContext::Create();
   std::vector<std::unique_ptr<content::NavigationEntry>> entries;
-  entries.reserve(entry_count);
-  for (int i = 0; i < entry_count; ++i) {
-    entries.push_back(content::NavigationEntry::Create());
-    if (!internal::RestoreNavigationEntryFromPickle(
-            state_version, iterator, entries[i].get(), context.get())) {
+
+  std::optional<int> selected_entry;
+
+  while (!iterator->ReachedEnd()) {
+    bool selected = false;
+    if (!iterator->ReadBool(&selected)) {
       return false;
     }
+
+    entries.push_back(content::NavigationEntry::Create());
+
+    if (!internal::RestoreNavigationEntryFromPickle(
+            state_version, iterator, entries.back().get(), context.get())) {
+      return false;
+    }
+
+    if (selected) {
+      selected_entry = entries.size() - 1;
+    }
   }
 
-  // |web_contents| takes ownership of these entries after this call.
-  sink.Restore(selected_entry, &entries);
-  DCHECK_EQ(0u, entries.size());
+  if (!selected_entry.has_value()) {
+    return false;
+  }
 
+  // The list was stored in reverse order, so flip it back (and update selected
+  // index).
+  std::reverse(entries.begin(), entries.end());
+  selected_entry = entries.size() - selected_entry.value() - 1;
+
+  sink.Restore(selected_entry.value(), &entries);
   return true;
 }
 

android_webview/browser/state_serializer.h

@@ -6,7 +6,9 @@
 #define ANDROID_WEBVIEW_BROWSER_STATE_SERIALIZER_H_
 
 #include <cstdint>
+#include <limits>
 #include <memory>
+#include <optional>
 #include <vector>
 
 namespace base {
@@ -27,7 +29,7 @@ class WebContents;
 namespace android_webview {
 
 // Write and restore a WebContents to and from a pickle.
-void WriteToPickle(content::WebContents& web_contents, base::Pickle* pickle);
+std::optional<base::Pickle> WriteToPickle(content::WebContents& web_contents);
 
 // |web_contents| will not be modified if function returns false.
 [[nodiscard]] bool RestoreFromPickle(base::PickleIterator* iterator,
@@ -37,6 +39,7 @@ namespace internal {
 
 const uint32_t AW_STATE_VERSION_INITIAL = 20130814;
 const uint32_t AW_STATE_VERSION_DATA_URL = 20151204;
+const uint32_t AW_STATE_VERSION_MOST_RECENT_FIRST = 20250213;
 
 // The navigation history to be saved. Primarily exists for testing.
 class NavigationHistory {
@@ -59,7 +62,16 @@ class NavigationHistorySink {
 // Functions below are individual helper functions called by functions above.
 // They are broken up for unit testing, and should not be called out side of
 // tests.
-void WriteToPickle(NavigationHistory& history, base::Pickle* pickle);
+
+// Writes the navigation history to a Pickle. If max_size is provided, older
+// entries will be dropped to ensure the returned Pickle is within the limit.
+// If save_forward_history is false, only entries before the selected entry
+// are saved. This is useful for embedders who only have a Back button (not
+// a Forward one).
+std::optional<base::Pickle> WriteToPickle(
+    NavigationHistory& history,
+    size_t max_size = std::numeric_limits<size_t>::max(),
+    bool save_forward_history = true);
 void WriteHeaderToPickle(base::Pickle* pickle);
 void WriteHeaderToPickle(uint32_t state_version, base::Pickle* pickle);
 [[nodiscard]] uint32_t RestoreHeaderFromPickle(base::PickleIterator* iterator);

android_webview/browser/state_serializer_unittest.cc

@@ -9,6 +9,7 @@
 #include <string>
 #include <vector>
 
+#include "base/check_op.h"
 #include "base/pickle.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
@@ -123,6 +124,11 @@ class TestNavigationController : public internal::NavigationHistory,
     current_entry_ = entries_.size() - 1;
   }
 
+  void SetCurrentEntry(unsigned int index) {
+    DCHECK_LT(index, entries_.size());
+    current_entry_ = index;
+  }
+
   void Restore(int selected_entry,
                std::vector<std::unique_ptr<content::NavigationEntry>>* entries)
       override {
@@ -137,6 +143,43 @@ class TestNavigationController : public internal::NavigationHistory,
   std::vector<std::unique_ptr<content::NavigationEntry>> entries_;
 };
 
+void AssertHistoriesEqual(TestNavigationController& lhs,
+                          TestNavigationController& rhs) {
+  EXPECT_EQ(lhs.GetEntryCount(), rhs.GetEntryCount());
+  EXPECT_EQ(lhs.GetCurrentEntry(), rhs.GetCurrentEntry());
+  for (int i = 0; i < lhs.GetEntryCount(); i++) {
+    AssertEntriesEqual(lhs.GetEntryAtIndex(i), rhs.GetEntryAtIndex(i));
+  }
+}
+
+void WriteToPickleLegacy_VersionDataUrl(internal::NavigationHistory& history,
+                                        base::Pickle* pickle) {
+  DCHECK(pickle);
+  int state_version = internal::AW_STATE_VERSION_DATA_URL;
+
+  internal::WriteHeaderToPickle(state_version, pickle);
+
+  const int entry_count = history.GetEntryCount();
+  const int selected_entry = history.GetCurrentEntry();
+  // A NavigationEntry will always exist, so there will always be at least 1
+  // entry.
+  DCHECK_GE(entry_count, 1);
+  DCHECK_GE(selected_entry, 0);
+  DCHECK_LT(selected_entry, entry_count);
+
+  pickle->WriteInt(entry_count);
+  pickle->WriteInt(selected_entry);
+  for (int i = 0; i < entry_count; ++i) {
+    internal::WriteNavigationEntryToPickle(state_version,
+                                           *history.GetEntryAtIndex(i), pickle);
+  }
+
+  // Please update AW_STATE_VERSION and IsSupportedVersion() if serialization
+  // format is changed.
+  // Make sure the serialization format is updated in a backwards compatible
+  // way.
+}
+
 }  // namespace
 
 TEST_F(AndroidWebViewStateSerializerTest, TestHeaderSerialization) {
@@ -336,7 +379,8 @@ TEST_F(AndroidWebViewStateSerializerTest, TestHugeDataURLSerialization) {
   EXPECT_EQ(huge_data_url, copy->GetDataURLAsString()->as_string());
 }
 
-TEST_F(AndroidWebViewStateSerializerTest, TestSerializeMultipleEntries) {
+TEST_F(AndroidWebViewStateSerializerTest,
+       TestDeserializeLegacy_VersionDataUrl) {
   TestNavigationController controller;
 
   controller.Add(CreateNavigationEntry("http://url1"));
@@ -344,17 +388,127 @@ TEST_F(AndroidWebViewStateSerializerTest, TestSerializeMultipleEntries) {
   controller.Add(CreateNavigationEntry("http://url3"));
 
   base::Pickle pickle;
-  internal::WriteToPickle(controller, &pickle);
+  WriteToPickleLegacy_VersionDataUrl(controller, &pickle);
 
   TestNavigationController copy;
   base::PickleIterator iterator(pickle);
   internal::RestoreFromPickle(&iterator, copy);
 
-  EXPECT_EQ(controller.GetEntryCount(), copy.GetEntryCount());
-  EXPECT_EQ(controller.GetCurrentEntry(), controller.GetCurrentEntry());
-  for (int i = 0; i < controller.GetEntryCount(); i++) {
-    AssertEntriesEqual(controller.GetEntryAtIndex(i), copy.GetEntryAtIndex(i));
+  AssertHistoriesEqual(controller, copy);
+}
+
+TEST_F(AndroidWebViewStateSerializerTest, TestHistorySerialization) {
+  TestNavigationController controller;
+
+  controller.Add(CreateNavigationEntry("http://url1"));
+  controller.Add(CreateNavigationEntry("http://url2"));
+  controller.Add(CreateNavigationEntry("http://url3"));
+
+  base::Pickle pickle = internal::WriteToPickle(controller).value();
+
+  TestNavigationController copy;
+  base::PickleIterator iterator(pickle);
+  internal::RestoreFromPickle(&iterator, copy);
+
+  AssertHistoriesEqual(controller, copy);
+}
+
+TEST_F(AndroidWebViewStateSerializerTest, TestHistoryTruncation) {
+  // Create the expected result first, so we can measure it and determine what
+  // to set the max size to.
+  TestNavigationController expected;
+  expected.Add(CreateNavigationEntry("http://url2"));
+  expected.Add(CreateNavigationEntry("http://url3"));
+  size_t max_size = internal::WriteToPickle(expected)->size();
+
+  TestNavigationController controller;
+  controller.Add(CreateNavigationEntry("http://url1"));
+  controller.Add(CreateNavigationEntry("http://url2"));
+  controller.Add(CreateNavigationEntry("http://url3"));
+
+  base::Pickle pickle = internal::WriteToPickle(controller, max_size).value();
+
+  TestNavigationController copy;
+  base::PickleIterator iterator(pickle);
+  EXPECT_TRUE(internal::RestoreFromPickle(&iterator, copy));
+
+  AssertHistoriesEqual(expected, copy);
+}
+
+TEST_F(AndroidWebViewStateSerializerTest,
+       TestHistoryTruncation_MaxSizeTooSmall) {
+  size_t max_size = 0;
+
+  TestNavigationController controller;
+  controller.Add(CreateNavigationEntry("http://url1"));
+  controller.Add(CreateNavigationEntry("http://url2"));
+  controller.Add(CreateNavigationEntry("http://url3"));
+
+  std::optional<base::Pickle> maybe_pickle =
+      internal::WriteToPickle(controller, max_size);
+
+  EXPECT_FALSE(maybe_pickle.has_value());
+}
+
+TEST_F(AndroidWebViewStateSerializerTest,
+       TestHistoryTruncation_NoForwardHistory) {
+  // In this test we expect url3 to be cut, because url2 is selected and we pass
+  // save_forward_history as false.
+  TestNavigationController expected;
+  expected.Add(CreateNavigationEntry("http://url1"));
+  expected.Add(CreateNavigationEntry("http://url2"));
+  size_t max_size = internal::WriteToPickle(expected)->size();
+
+  TestNavigationController controller;
+  controller.Add(CreateNavigationEntry("http://url1"));
+  controller.Add(CreateNavigationEntry("http://url2"));
+  controller.Add(CreateNavigationEntry("http://url3"));
+  controller.SetCurrentEntry(1);
+
+  base::Pickle pickle =
+      internal::WriteToPickle(controller, max_size,
+                              /* save_forward_history= */ false)
+          .value();
+
+  TestNavigationController copy;
+  base::PickleIterator iterator(pickle);
+  EXPECT_TRUE(internal::RestoreFromPickle(&iterator, copy));
+
+  AssertHistoriesEqual(expected, copy);
+}
+
+TEST_F(AndroidWebViewStateSerializerTest,
+       TestHistoryTruncation_SelectedEntryFarBack) {
+  // The selected entry is far back in history (more than max_size) back. Make
+  // sure that we don't drop it.
+
+  // The current implementation falls back to save_forward_history = false, but
+  // the key requirement is that the selected entry is saved.
+
+  size_t max_size;
+  {
+    TestNavigationController controller;
+    controller.Add(CreateNavigationEntry("http://url2"));
+    controller.Add(CreateNavigationEntry("http://url3"));
+    max_size = internal::WriteToPickle(controller)->size();
   }
+
+  TestNavigationController expected;
+  expected.Add(CreateNavigationEntry("http://url1"));
+
+  TestNavigationController controller;
+  controller.Add(CreateNavigationEntry("http://url1"));
+  controller.Add(CreateNavigationEntry("http://url2"));
+  controller.Add(CreateNavigationEntry("http://url3"));
+  controller.SetCurrentEntry(0);
+
+  base::Pickle pickle = internal::WriteToPickle(controller, max_size).value();
+
+  TestNavigationController copy;
+  base::PickleIterator iterator(pickle);
+  EXPECT_TRUE(internal::RestoreFromPickle(&iterator, copy));
+
+  AssertHistoriesEqual(expected, copy);
 }
 
 }  // namespace android_webview