Ensure serialized size_t can be passed between 32bit and 64bit processes

https://crrev.com/c/4205862 broken some WebView environment which
uses 32bit renderer process and 64bit browser process because size_t
was serialized as the native type uint32_t or uint64_t.

Now partly revert the CL about size_t to always serialize size_t as
uint64_t.

The current PaintOpReader and PaintOpWriter are still fragile because
the caller can use Write/Read() instead of WriteSize()/ReadSize() for
size_t and cause problems. I will follow up to refactor this to prevent
the problem.

Bug: 1414445, 1414686
Change-Id: I9c427950fd3559c5c1f79d019c7a0f8131a01b17
Cq-Include-Trybots: luci.chromium.try:android-webview-pie-arm64-dbg,android-webview-oreo-arm64-dbg
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4237958
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1104172}

cc/BUILD.gn

@@ -754,6 +754,7 @@ cc_test("cc_unittests") {
     "paint/paint_image_unittest.cc",
     "paint/paint_op_buffer_unittest.cc",
     "paint/paint_op_helper_unittest.cc",
+    "paint/paint_op_writer_reader_unittest.cc",
     "paint/paint_shader_unittest.cc",
     "paint/paint_worklet_input_unittest.cc",
     "paint/scoped_raster_flags_unittest.cc",

cc/paint/paint_op_reader.cc

@@ -18,6 +18,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/notreached.h"
+#include "base/numerics/safe_conversions.h"
 #include "base/rand_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/types/optional_util.h"
@@ -179,7 +180,11 @@ void PaintOpReader::ReadData(size_t bytes, void* data) {
 }
 
 void PaintOpReader::ReadSize(size_t* size) {
-  ReadSimple(size);
+  // size_t is always serialized as uint64_t to make the serialized result
+  // portable between 32bit and 64bit processes.
+  uint64_t size64 = 0;
+  ReadSimple(&size64);
+  *size = base::checked_cast<size_t>(size64);
 }
 
 void PaintOpReader::Read(SkScalar* data) {

cc/paint/paint_op_writer.cc

@@ -175,7 +175,7 @@ void PaintOpWriter::WriteFlattenable(const SkFlattenable* val) {
     return;
   }
 
-  size_t* size_memory = WriteSize(0u);
+  uint64_t* size_memory = WriteSize(0u);
   if (!valid_)
     return;
 
@@ -189,9 +189,11 @@ void PaintOpWriter::WriteFlattenable(const SkFlattenable* val) {
   DidWrite(bytes_written);
 }
 
-size_t* PaintOpWriter::WriteSize(size_t size) {
-  size_t* memory = reinterpret_cast<size_t*>(memory_.get());
-  WriteSimple(size);
+uint64_t* PaintOpWriter::WriteSize(size_t size) {
+  // size_t is always serialized as uint64_t to make the serialized result
+  // portable between 32bit and 64bit processes.
+  uint64_t* memory = reinterpret_cast<uint64_t*>(memory_.get());
+  WriteSimple(static_cast<uint64_t>(size));
   return memory;
 }
 
@@ -256,7 +258,7 @@ void PaintOpWriter::Write(const SkPath& path, UsePaintCache use_paint_cache) {
   } else {
     Write(static_cast<uint32_t>(PaintCacheEntryState::kInlinedDoNotCache));
   }
-  size_t* bytes_to_skip = WriteSize(0u);
+  uint64_t* bytes_to_skip = WriteSize(0u);
   if (!valid_)
     return;
 
@@ -349,7 +351,7 @@ void PaintOpWriter::Write(scoped_refptr<SkottieWrapper> skottie) {
   uint32_t id = skottie->id();
   Write(id);
 
-  size_t* bytes_to_skip = WriteSize(0u);
+  uint64_t* bytes_to_skip = WriteSize(0u);
   if (!valid_)
     return;
 
@@ -453,7 +455,7 @@ void PaintOpWriter::Write(const sk_sp<GrSlug>& slug) {
     return;
 
   AssertFieldAlignment();
-  size_t* size_memory = WriteSize(0u);
+  uint64_t* size_memory = WriteSize(0u);
   if (!valid_)
     return;
 
@@ -971,7 +973,7 @@ void PaintOpWriter::Write(const PaintRecord& record,
   // We need to record how many bytes we will serialize, but we don't know this
   // information until we do the serialization. So, write 0 as the size first,
   // and amend it after writing.
-  size_t* size_memory = WriteSize(0u);
+  uint64_t* size_memory = WriteSize(0u);
   if (!valid_)
     return;
 

cc/paint/paint_op_writer.h

@@ -91,19 +91,35 @@ class CC_PAINT_EXPORT PaintOpWriter {
   // A field can also use a larger alignment by calling AlignMemory().
   static constexpr size_t kDefaultAlignment = alignof(uint32_t);
 
+ private:
+  template <typename T>
+  static constexpr size_t SerializedSizeSimple() {
+    static_assert(!std::is_pointer_v<T>);
+    return base::bits::AlignUp(sizeof(T), kDefaultAlignment);
+  }
+  // size_t is always serialized as uint64_t to make the serialized result
+  // portable between 32bit and 64bit processes.
+  template <>
+  constexpr size_t SerializedSizeSimple<size_t>() {
+    return base::bits::AlignUp(sizeof(uint64_t), kDefaultAlignment);
+  }
+
+ public:
   // SerializedSize() returns the maximum serialized size of the given type or
   // the given parameter. For a buffer to contain serialization of multiple
   // data, the size can be the accumulated results of SerializedSize() of each
-  // data.
+  // data. When possible, the parameterized version should be used to make it
+  // easier to keep serialized size calculation in sync with serialization and
+  // deserialization, and make it possible to allow dynamic sizing for some
+  // data types (see the specialized/overloaded functions).
   template <typename T>
   static constexpr size_t SerializedSize() {
     static_assert(std::is_arithmetic_v<T> || std::is_enum_v<T>);
-    return base::bits::AlignUp(sizeof(T), kDefaultAlignment);
+    return SerializedSizeSimple<T>();
   }
   template <typename T>
   static constexpr size_t SerializedSize(const T& data) {
-    static_assert(!std::is_pointer_v<T>);
-    return base::bits::AlignUp(sizeof(T), kDefaultAlignment);
+    return SerializedSizeSimple<T>();
   }
   static size_t SerializedSize(const PaintImage& image);
   static size_t SerializedSize(const PaintRecord& record);
@@ -170,7 +186,14 @@ class CC_PAINT_EXPORT PaintOpWriter {
   // alignment.
   size_t size() const { return valid_ ? size_ - remaining_bytes_ : 0u; }
 
-  size_t* WriteSize(size_t size);
+  // Writes a size_t. The return value can be used when the size is unknown
+  // before writing some data:
+  //   uint64_t* memory = WriteSize(0u);
+  //   size_t data_size = WriteSomeData();
+  //   *memory = data_size;
+  // Note that size_t is always serialized as uint64_t to make the serialized
+  // result portable between 32bit and 64bit processes.
+  uint64_t* WriteSize(size_t size);
 
   void Write(SkScalar data);
   void Write(SkMatrix data);

cc/paint/paint_op_writer_reader_unittest.cc

@@ -0,0 +1,50 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "cc/paint/paint_op_reader.h"
+#include "cc/paint/paint_op_writer.h"
+
+#include "cc/test/test_options_provider.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace cc {
+
+TEST(PaintOpWriterReaderTest, SizeT) {
+  static_assert(PaintOpWriter::SerializedSize<int>() == 4u);
+  static_assert(PaintOpWriter::SerializedSize<size_t>() == 8u);
+  static_assert(PaintOpWriter::SerializedSize(static_cast<size_t>(0u)) == 8u);
+
+  char buffer[128];
+  TestOptionsProvider options_provider;
+  memset(buffer, 0xa5, std::size(buffer));
+  PaintOpWriter writer(buffer, std::size(buffer),
+                       options_provider.serialize_options(),
+                       /*enable_security_constraints*/ true);
+  int i = 0x5555aaaa;
+  size_t s1 = static_cast<size_t>(0x123456789abcdef0L);
+  size_t s2 = static_cast<size_t>(0xfedcba9876543210L);
+  writer.WriteSize(s1);
+  writer.Write(i);
+  writer.WriteSize(s2);
+  EXPECT_EQ(20u, writer.size());
+
+  PaintOpReader reader(buffer, writer.size(),
+                       options_provider.deserialize_options(),
+                       /*enable_security_constraints*/ true);
+  int read_i;
+  size_t read_s1, read_s2;
+  reader.ReadSize(&read_s1);
+  reader.Read(&read_i);
+  reader.ReadSize(&read_s2);
+  EXPECT_EQ(i, read_i);
+  EXPECT_EQ(s1, read_s1);
+  EXPECT_EQ(s2, read_s2);
+  EXPECT_TRUE(reader.valid());
+  EXPECT_EQ(0u, reader.remaining_bytes());
+
+  reader.ReadSize(&read_s2);
+  EXPECT_FALSE(reader.valid());
+}
+
+}  // namespace cc