Bug 1885246 - Replace PlainOldDataSerializer with ParamTraits_TiedFields, except for gfx/vr and gfx/layers/wr. r=mccr8

Differential Revision: https://phabricator.services.mozilla.com/D204567

dom/canvas/QueueParamTraits.h

@@ -261,15 +261,6 @@ struct QueueParamTraits<bool> {
 
 // ---------------------------------------------------------------
 
-template <class T>
-Maybe<T> AsValidEnum(const std::underlying_type_t<T> raw_val) {
-  const auto raw_enum = T{raw_val};  // This is the risk we prevent!
-  if (!IsEnumCase(raw_enum)) return {};
-  return Some(raw_enum);
-}
-
-// -
-
 template <class T>
 struct QueueParamTraits_IsEnumCase {
   template <typename ProducerView>

dom/canvas/TiedFields.h

@@ -131,42 +131,34 @@ constexpr bool AssertTiedFieldsAreExhaustive() {
 // -
 
 /**
- * Padding<T> can be used to pad out a struct so that it's not implicitly
- * padded by struct rules.
- * You can also just add your padding to TiedFields, but by explicitly typing
- * padding like this, serialization can make a choice whether to copy Padding,
- * or instead to omit the copy.
- *
- * Omitting the copy isn't always faster.
- * struct Entry {
- *   uint16_t key;
- *   Padding<uint16_t> padding;
- *   uint32_t val;
- *   auto MutTiedFields() { return std::tie(key, padding, val); }
- * };
- * If you serialize Padding, the serialized size is 8, and the compiler will
- * optimize serialization to a single 8-byte memcpy.
- * If your serialization omits Padding, the serialized size of Entry shrinks
- * by 25%. If you have a big list of Entrys, maybe this is a big savings!
- * However, by optimizing for size here you sacrifice speed, because this splits
- * the single memcpy into two: a 2-byte memcpy and a 4-byte memcpy.
- *
- * Explicitly marking padding gives callers the option of choosing.
+ * PaddingField<T,N=1> can be used to pad out a struct so that it's not
+ * implicitly padded by struct rules, but also can't be accidentally initialized
+ * via Aggregate Initialization. (TiedFields serialization checks rely on object
+ * fields leaving no implicit padding bytes, but explicit padding fields are
+ * fine) While you can use e.g. `uint8_t _padding[3];`, consider instead
+ * `PaddingField<uint8_t,3> _padding;` for clarity and to move the `3` nearer
+ * to the `uint8_t`.
  */
-template <class T>
-struct Padding {
-  T ignored;
+template <class T, size_t N = 1>
+struct PaddingField {
+  static_assert(!std::is_array_v<T>, "Use PaddingField<T,N> not <T[N]>.");
 
-  friend constexpr bool operator==(const Padding&, const Padding&) {
+  std::array<T, N> ignored = {};
+
+  PaddingField() {}
+
+  friend constexpr bool operator==(const PaddingField&, const PaddingField&) {
     return true;
   }
-  friend constexpr bool operator<(const Padding&, const Padding&) {
+  friend constexpr bool operator<(const PaddingField&, const PaddingField&) {
     return false;
   }
+
+  auto MutTiedFields() { return std::tie(ignored); }
 };
-static_assert(sizeof(Padding<bool>) == 1);
-static_assert(sizeof(Padding<bool[2]>) == 2);
-static_assert(sizeof(Padding<int>) == 4);
+static_assert(sizeof(PaddingField<bool>) == 1);
+static_assert(sizeof(PaddingField<bool, 2>) == 2);
+static_assert(sizeof(PaddingField<int>) == 4);
 
 // -
 
@@ -202,7 +194,7 @@ static_assert(AreAllBytesTiedFields<Fish>());
 
 struct Eel {  // Like a Fish, but you can skip serializing the padding.
   bool b;
-  Padding<bool> padding[3];
+  PaddingField<bool, 3> padding;
   int i;
 
   constexpr auto MutTiedFields() { return std::tie(i, b, padding); }

dom/canvas/WebGLIpdl.h

@@ -244,6 +244,45 @@ struct ParamTraits<mozilla::dom::PredefinedColorSpace> final
     : public mozilla::dom::WebIDLEnumSerializer<
           mozilla::dom::PredefinedColorSpace> {};
 
+// -
+// ParamTraits_IsEnumCase
+
+/*
+`IsEnumCase(T) -> bool` guarantees that we never have false negatives or false
+positives due to adding or removing enum cases to enums, and forgetting to
+update their serializations. Also, it allows enums to be non-continguous, unlike
+ContiguousEnumSerializer.
+*/
+
+template <class T>
+struct ParamTraits_IsEnumCase {
+  static bool Write(MessageWriter* const writer, const T& in) {
+    MOZ_ASSERT(IsEnumCase(in));
+    const auto shadow = static_cast<std::underlying_type_t<T>>(in);
+    WriteParam(writer, shadow);
+    return true;
+  }
+
+  static bool Read(MessageReader* const reader, T* const out) {
+    auto shadow = std::underlying_type_t<T>{};
+    if (!ReadParam(reader, &shadow)) return false;
+    const auto e = mozilla::AsValidEnum<T>(shadow);
+    if (!e) return false;
+    *out = *e;
+    return true;
+  }
+};
+
+// -
+
+#define USE_IS_ENUM_CASE(T) \
+  template <>               \
+  struct ParamTraits<T> : public ParamTraits_IsEnumCase<T> {};
+
+USE_IS_ENUM_CASE(mozilla::webgl::OptionalRenderableFormatBits)
+
+#undef USE_IS_ENUM_CASE
+
 // -
 // ParamTraits_TiedFields
 
@@ -272,6 +311,10 @@ struct ParamTraits_TiedFields {
   }
 };
 
+template <class U, size_t N>
+struct ParamTraits<mozilla::PaddingField<U, N>> final
+    : public ParamTraits_TiedFields<mozilla::PaddingField<U, N>> {};
+
 // -
 
 template <>
@@ -609,35 +652,6 @@ struct ParamTraits<mozilla::avec3<U>> final {
   }
 };
 
-// -
-
-template <class TT>
-struct ParamTraits_IsEnumCase {
-  using T = TT;
-
-  static void Write(IPC::MessageWriter* const writer, const T& in) {
-    MOZ_RELEASE_ASSERT(IsEnumCase(in));
-    WriteParam(writer, mozilla::UnderlyingValue(in));
-  }
-
-  static bool Read(IPC::MessageReader* const reader, T* const out) {
-    std::underlying_type_t<T> rawVal;
-    if (!ReadParam(reader, &rawVal)) return false;
-    *out = static_cast<T>(rawVal);
-    return IsEnumCase(*out);
-  }
-};
-
-// -
-
-#define USE_IS_ENUM_CASE(T) \
-  template <>               \
-  struct ParamTraits<T> : public ParamTraits_IsEnumCase<T> {};
-
-USE_IS_ENUM_CASE(mozilla::webgl::OptionalRenderableFormatBits)
-
-#undef USE_IS_ENUM_CASE
-
 }  // namespace IPC
 
 #endif

dom/canvas/WebGLTypes.h

@@ -1224,6 +1224,15 @@ inline bool StartsWith(const std::string_view str,
 
 // -
 
+template <class T>
+Maybe<T> AsValidEnum(const std::underlying_type_t<T> raw_val) {
+  const auto raw_enum = T{raw_val};  // This is the risk we prevent!
+  if (!IsEnumCase(raw_enum)) return {};
+  return Some(raw_enum);
+}
+
+// -
+
 namespace webgl {
 
 // In theory, this number can be unbounded based on the driver. However, no

dom/ipc/ContentParent.cpp

@@ -644,6 +644,10 @@ static const char* sObserverTopics[] = {
     DEFAULT_TIMEZONE_CHANGED_OBSERVER_TOPIC,
 };
 
+void ContentParent_NotifyUpdatedDictionaries() {
+  ContentParent::NotifyUpdatedDictionaries();
+}
+
 // PreallocateProcess is called by the PreallocatedProcessManager.
 // ContentParent then takes this process back within GetNewOrUsedBrowserProcess.
 /*static*/ already_AddRefed<ContentParent>

dom/ipc/ContentParent_NotifyUpdatedDictionaries.h

@@ -0,0 +1,17 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_dom_ContentParent_NotifyUpdatedDictionaries_h
+#define mozilla_dom_ContentParent_NotifyUpdatedDictionaries_h
+
+// Avoid including ContentParent.h because it ends up including WebGLTypes.h,
+// where we have our mozilla::malloc(ForbidNarrowing<size_t>) overrides, which
+// cause issues with the way hunspell overrides malloc for itself.
+namespace mozilla::dom {
+void ContentParent_NotifyUpdatedDictionaries();
+}  // namespace mozilla::dom
+
+#endif  // mozilla_dom_ContentParent_NotifyUpdatedDictionaries_h

dom/ipc/moz.build

@@ -52,6 +52,7 @@ EXPORTS.mozilla.dom += [
     "CoalescedWheelData.h",
     "ContentChild.h",
     "ContentParent.h",
+    "ContentParent_NotifyUpdatedDictionaries.h",
     "ContentProcess.h",
     "ContentProcessManager.h",
     "CSPMessageUtils.h",

dom/media/MediaInfo.h

@@ -109,12 +109,16 @@ struct FlacCodecSpecificData {
   RefPtr<MediaByteBuffer> mStreamInfoBinaryBlob{new MediaByteBuffer};
 };
 
-struct Mp3CodecSpecificData {
+struct Mp3CodecSpecificData final {
   bool operator==(const Mp3CodecSpecificData& rhs) const {
     return mEncoderDelayFrames == rhs.mEncoderDelayFrames &&
            mEncoderPaddingFrames == rhs.mEncoderPaddingFrames;
   }
 
+  auto MutTiedFields() {
+    return std::tie(mEncoderDelayFrames, mEncoderPaddingFrames);
+  }
+
   // The number of frames that should be skipped from the beginning of the
   // decoded stream.
   // See https://bugzilla.mozilla.org/show_bug.cgi?id=1566389 for more info.

dom/media/ipc/MediaIPCUtils.h

@@ -15,6 +15,7 @@
 #include "ipc/EnumSerializer.h"
 #include "mozilla/EnumSet.h"
 #include "mozilla/GfxMessageUtils.h"
+#include "mozilla/dom/WebGLIpdl.h"
 #include "mozilla/gfx/Rect.h"
 #include "mozilla/dom/MFCDMSerializers.h"
 
@@ -139,7 +140,7 @@ struct ParamTraits<mozilla::FlacCodecSpecificData> {
 
 template <>
 struct ParamTraits<mozilla::Mp3CodecSpecificData>
-    : public PlainOldDataSerializer<mozilla::Mp3CodecSpecificData> {};
+    : public ParamTraits_TiedFields<mozilla::Mp3CodecSpecificData> {};
 
 template <>
 struct ParamTraits<mozilla::OpusCodecSpecificData> {

dom/network/TCPSocket.cpp

@@ -49,9 +49,6 @@
 #include "secerr.h"
 #include "sslerr.h"
 
-#define BUFFER_SIZE 65536
-#define NETWORK_STATS_THRESHOLD 65536
-
 using namespace mozilla::dom;
 
 NS_IMPL_CYCLE_COLLECTION(LegacyMozTCPSocket, mGlobal)

extensions/spellcheck/hunspell/glue/mozHunspell.cpp

@@ -69,14 +69,14 @@
 #include "nsIPrefBranch.h"
 #include "nsIPrefService.h"
 #include "nsNetUtil.h"
-#include "mozilla/dom/ContentParent.h"
+#include "prenv.h"
 #include "mozilla/Components.h"
 #include "mozilla/Services.h"
+#include "mozilla/dom/ContentParent_NotifyUpdatedDictionaries.h"
 
 #include <stdlib.h>
 #include <tuple>
 
-using mozilla::dom::ContentParent;
 using namespace mozilla;
 
 NS_IMPL_CYCLE_COLLECTING_ADDREF(mozHunspell)
@@ -294,7 +294,7 @@ void mozHunspell::DictionariesChanged(bool aNotifyChildProcesses) {
   mozInlineSpellChecker::UpdateCanEnableInlineSpellChecking();
 
   if (aNotifyChildProcesses) {
-    ContentParent::NotifyUpdatedDictionaries();
+    mozilla::dom::ContentParent_NotifyUpdatedDictionaries();
   }
 
   // Check if the current dictionaries are still available.

gfx/layers/FrameMetrics.h

@@ -700,7 +700,7 @@ MOZ_DEFINE_ENUM_CLASS_WITH_BASE(
 std::ostream& operator<<(std::ostream& aStream,
                          const OverscrollBehavior& aBehavior);
 
-struct OverscrollBehaviorInfo {
+struct OverscrollBehaviorInfo final {
   OverscrollBehaviorInfo();
 
   // Construct from StyleOverscrollBehavior values.
@@ -711,6 +711,8 @@ struct OverscrollBehaviorInfo {
   friend std::ostream& operator<<(std::ostream& aStream,
                                   const OverscrollBehaviorInfo& aInfo);
 
+  auto MutTiedFields() { return std::tie(mBehaviorX, mBehaviorY); }
+
   OverscrollBehavior mBehaviorX;
   OverscrollBehavior mBehaviorY;
 };

gfx/layers/LayersTypes.h

@@ -46,9 +46,11 @@ class TextureHost;
 #undef NONE
 #undef OPAQUE
 
-struct LayersId {
+struct LayersId final {
   uint64_t mId = 0;
 
+  auto MutTiedFields() { return std::tie(mId); }
+
   bool IsValid() const { return mId != 0; }
 
   // Allow explicit cast to a uint64_t for now
@@ -75,9 +77,11 @@ struct LayersId {
 };
 
 template <typename T>
-struct BaseTransactionId {
+struct BaseTransactionId final {
   uint64_t mId = 0;
 
+  auto MutTiedFields() { return std::tie(mId); }
+
   bool IsValid() const { return mId != 0; }
 
   [[nodiscard]] BaseTransactionId<T> Next() const {

gfx/layers/ipc/LayersMessageUtils.h

@@ -18,6 +18,7 @@
 #include "ipc/IPCMessageUtils.h"
 #include "mozilla/ScrollSnapInfo.h"
 #include "mozilla/ServoBindings.h"
+#include "mozilla/dom/WebGLIpdl.h"
 #include "mozilla/ipc/ByteBuf.h"
 #include "mozilla/ipc/ProtocolMessageUtils.h"
 #include "mozilla/layers/APZInputBridge.h"
@@ -48,15 +49,11 @@ namespace IPC {
 
 template <>
 struct ParamTraits<mozilla::layers::LayersId>
-    : public PlainOldDataSerializer<mozilla::layers::LayersId> {};
+    : public ParamTraits_TiedFields<mozilla::layers::LayersId> {};
 
 template <typename T>
 struct ParamTraits<mozilla::layers::BaseTransactionId<T>>
-    : public PlainOldDataSerializer<mozilla::layers::BaseTransactionId<T>> {};
-
-template <>
-struct ParamTraits<mozilla::VsyncId>
-    : public PlainOldDataSerializer<mozilla::VsyncId> {};
+    : public ParamTraits_TiedFields<mozilla::layers::BaseTransactionId<T>> {};
 
 template <>
 struct ParamTraits<mozilla::VsyncEvent> {
@@ -419,7 +416,7 @@ struct ParamTraits<mozilla::StyleScrollSnapStop>
 
 template <>
 struct ParamTraits<mozilla::ScrollSnapTargetId>
-    : public PlainOldDataSerializer<mozilla::ScrollSnapTargetId> {};
+    : public ParamTraits_IsEnumCase<mozilla::ScrollSnapTargetId> {};
 
 template <>
 struct ParamTraits<mozilla::SnapPoint> {
@@ -495,26 +492,12 @@ struct ParamTraits<mozilla::ScrollSnapInfo> {
 };
 
 template <>
-struct ParamTraits<mozilla::layers::OverscrollBehaviorInfo> {
-  // Not using PlainOldDataSerializer so we get enum validation
-  // for the members.
-
-  typedef mozilla::layers::OverscrollBehaviorInfo paramType;
-
-  static void Write(MessageWriter* aWriter, const paramType& aParam) {
-    WriteParam(aWriter, aParam.mBehaviorX);
-    WriteParam(aWriter, aParam.mBehaviorY);
-  }
-
-  static bool Read(MessageReader* aReader, paramType* aResult) {
-    return (ReadParam(aReader, &aResult->mBehaviorX) &&
-            ReadParam(aReader, &aResult->mBehaviorY));
-  }
-};
+struct ParamTraits<mozilla::layers::OverscrollBehaviorInfo>
+    : public ParamTraits_TiedFields<mozilla::layers::OverscrollBehaviorInfo> {};
 
 template <typename T>
 struct ParamTraits<mozilla::ScrollGeneration<T>>
-    : PlainOldDataSerializer<mozilla::ScrollGeneration<T>> {};
+    : public ParamTraits_TiedFields<mozilla::ScrollGeneration<T>> {};
 
 template <>
 struct ParamTraits<mozilla::ScrollUpdateType>

gfx/layers/ipc/SharedSurfacesMemoryReport.h

@@ -12,6 +12,7 @@
 #include "base/process.h"
 #include "ipc/IPCMessageUtils.h"
 #include "ipc/IPCMessageUtilsSpecializations.h"
+#include "mozilla/dom/WebGLIpdl.h"
 #include "mozilla/gfx/Point.h"  // for IntSize
 
 namespace mozilla {
@@ -26,8 +27,16 @@ class SharedSurfacesMemoryReport final {
     int32_t mStride;
     uint32_t mConsumers;
     bool mCreatorRef;
+    PaddingField<bool, 3> _padding;
+
+    auto MutTiedFields() {
+      return std::tie(mCreatorPid, mSize, mStride, mConsumers, mCreatorRef,
+                      _padding);
+    }
   };
 
+  auto MutTiedFields() { return std::tie(mSurfaces); }
+
   std::unordered_map<uint64_t, SurfaceEntry> mSurfaces;
 };
 
@@ -37,21 +46,13 @@ class SharedSurfacesMemoryReport final {
 namespace IPC {
 
 template <>
-struct ParamTraits<mozilla::layers::SharedSurfacesMemoryReport> {
-  typedef mozilla::layers::SharedSurfacesMemoryReport paramType;
-
-  static void Write(MessageWriter* aWriter, const paramType& aParam) {
-    WriteParam(aWriter, aParam.mSurfaces);
-  }
-
-  static bool Read(MessageReader* aReader, paramType* aResult) {
-    return ReadParam(aReader, &aResult->mSurfaces);
-  }
-};
+struct ParamTraits<mozilla::layers::SharedSurfacesMemoryReport>
+    : public ParamTraits_TiedFields<
+          mozilla::layers::SharedSurfacesMemoryReport> {};
 
 template <>
 struct ParamTraits<mozilla::layers::SharedSurfacesMemoryReport::SurfaceEntry>
-    : public PlainOldDataSerializer<
+    : public ParamTraits_TiedFields<
           mozilla::layers::SharedSurfacesMemoryReport::SurfaceEntry> {};
 
 }  // namespace IPC

layout/generic/ScrollGeneration.h

@@ -7,6 +7,7 @@
 
 #include <cstdint>
 #include <iosfwd>
+#include <tuple>
 
 namespace mozilla {
 
@@ -23,7 +24,7 @@ std::ostream& operator<<(std::ostream& aStream,
                          const ScrollGeneration<Tag>& aGen);
 
 template <typename Tag>
-struct ScrollGeneration {
+struct ScrollGeneration final {
   friend struct ScrollGenerationCounter;
 
  private:
@@ -43,6 +44,8 @@ struct ScrollGeneration {
   friend std::ostream& operator<< <>(std::ostream& aStream,
                                      const ScrollGeneration<Tag>& aGen);
 
+  auto MutTiedFields() { return std::tie(mValue); }
+
  private:
   uint64_t mValue;
 };

layout/generic/ScrollSnapTargetId.h

@@ -17,6 +17,7 @@ namespace mozilla {
 enum class ScrollSnapTargetId : uintptr_t {
   None = 0,
 };
+inline constexpr bool IsEnumCase(ScrollSnapTargetId) { return true; }
 
 struct ScrollSnapTargetIds {
   CopyableTArray<ScrollSnapTargetId> mIdsOnX;

toolkit/components/telemetry/core/ipc/TelemetryComms.h

@@ -12,6 +12,7 @@
 #include "mozilla/TelemetryProcessEnums.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/Variant.h"
+#include "mozilla/dom/WebGLIpdl.h"
 #include "nsITelemetry.h"
 
 namespace mozilla {
@@ -96,6 +97,13 @@ struct DiscardedData {
   uint32_t mDiscardedScalarActions;
   uint32_t mDiscardedKeyedScalarActions;
   uint32_t mDiscardedChildEvents;
+
+  auto MutTiedFields() {
+    return std::tie(mDiscardedHistogramAccumulations,
+                    mDiscardedKeyedHistogramAccumulations,
+                    mDiscardedScalarActions, mDiscardedKeyedScalarActions,
+                    mDiscardedChildEvents);
+  }
 };
 
 }  // namespace Telemetry
@@ -393,7 +401,7 @@ struct ParamTraits<mozilla::Telemetry::EventExtraEntry> {
 
 template <>
 struct ParamTraits<mozilla::Telemetry::DiscardedData>
-    : public PlainOldDataSerializer<mozilla::Telemetry::DiscardedData> {};
+    : public ParamTraits_TiedFields<mozilla::Telemetry::DiscardedData> {};
 
 }  // namespace IPC