Bug 1774306 - Avoid manual JS Read/Writes in VideoFrame serialization r=padenot

Depends on D163195

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

dom/base/StructuredCloneHolder.cpp

@@ -395,7 +395,7 @@ void StructuredCloneHolder::Read(nsIGlobalObject* aGlobal, JSContext* aCx,
     mWasmModuleArray.Clear();
     mClonedSurfaces.Clear();
     mInputStreamArray.Clear();
-    mVideoFrameImages.Clear();
+    mVideoFrames.Clear();
     Clear();
   }
 }
@@ -1025,7 +1025,7 @@ JSObject* StructuredCloneHolder::CustomReadHandler(
       aTag == SCTAG_DOM_VIDEOFRAME &&
       CloneScope() == StructuredCloneScope::SameProcess) {
     return VideoFrame::ReadStructuredClone(aCx, mGlobal, aReader,
-                                           VideoFrameImages()[aIndex]);
+                                           VideoFrames()[aIndex]);
   }
 
   return ReadFullySerializableObjects(aCx, aReader, aTag);

dom/base/StructuredCloneHolder.h

@@ -166,7 +166,7 @@ class StructuredCloneHolderBase {
 class BlobImpl;
 class MessagePort;
 class MessagePortIdentifier;
-struct VideoFrameImageData;
+struct VideoFrameSerializedData;
 
 class StructuredCloneHolder : public StructuredCloneHolderBase {
  public:
@@ -210,7 +210,7 @@ class StructuredCloneHolder : public StructuredCloneHolderBase {
   bool HasClonedDOMObjects() const {
     return !mBlobImplArray.IsEmpty() || !mWasmModuleArray.IsEmpty() ||
            !mClonedSurfaces.IsEmpty() || !mInputStreamArray.IsEmpty() ||
-           !mVideoFrameImages.IsEmpty();
+           !mVideoFrames.IsEmpty();
   }
 
   nsTArray<RefPtr<BlobImpl>>& BlobImpls() {
@@ -266,9 +266,7 @@ class StructuredCloneHolder : public StructuredCloneHolderBase {
     return mClonedSurfaces;
   }
 
-  nsTArray<VideoFrameImageData>& VideoFrameImages() {
+  nsTArray<VideoFrameSerializedData>& VideoFrames() { return mVideoFrames; }
-    return mVideoFrameImages;
-  }
 
   // Implementations of the virtual methods to allow cloning of objects which
   // JS engine itself doesn't clone.
@@ -368,7 +366,7 @@ class StructuredCloneHolder : public StructuredCloneHolderBase {
   nsTArray<RefPtr<gfx::DataSourceSurface>> mClonedSurfaces;
 
   // Used for cloning VideoFrame in the structured cloning algorithm.
-  nsTArray<VideoFrameImageData> mVideoFrameImages;
+  nsTArray<VideoFrameSerializedData> mVideoFrames;
 
   // This raw pointer is only set within ::Read() and is unset by the end.
   nsIGlobalObject* MOZ_NON_OWNING_REF mGlobal;

dom/media/webcodecs/VideoFrame.cpp

@@ -1051,10 +1051,10 @@ static Result<RefPtr<VideoFrame>, nsCString> CreateVideoFrameFromBuffer(
   // TODO: Spec should assign aInit.mFormat to inner format value:
   // https://github.com/w3c/webcodecs/issues/509.
   // This comment should be removed once the issue is resolved.
-  return MakeRefPtr<VideoFrame>(
+  return MakeRefPtr<VideoFrame>(aGlobal, data, aInit.mFormat, codedSize,
-      aGlobal, data, aInit.mFormat, codedSize, parsedRect,
+                                parsedRect,
-      displaySize ? *displaySize : parsedRect.Size(), std::move(duration),
+                                displaySize ? *displaySize : parsedRect.Size(),
-      aInit.mTimestamp, colorSpace);
+                                duration, aInit.mTimestamp, colorSpace);
 }
 
 template <class T>
@@ -1137,7 +1137,7 @@ InitializeFrameWithResourceAndSize(
   const VideoColorSpaceInit colorSpace{};
   return MakeAndAddRef<VideoFrame>(aGlobal, image, format->PixelFormat(),
                                    image->GetSize(), visibleRect.value(),
-                                   displaySize.value(), std::move(duration),
+                                   displaySize.value(), duration,
                                    aInit.mTimestamp.Value(), colorSpace);
 }
 
@@ -1171,8 +1171,8 @@ InitializeFrameFromOtherFrame(nsIGlobalObject* aGlobal,
 
   return MakeAndAddRef<VideoFrame>(
       aGlobal, aData.mImage, aData.mFormat.PixelFormat(),
-      aData.mImage->GetSize(), *visibleRect, *displaySize, std::move(duration),
+      aData.mImage->GetSize(), *visibleRect, *displaySize, duration, timestamp,
-      timestamp, aData.mColorSpace);
+      aData.mColorSpace);
 }
 
 /*
@@ -1183,14 +1183,14 @@ VideoFrame::VideoFrame(nsIGlobalObject* aParent,
                        const RefPtr<layers::Image>& aImage,
                        const VideoPixelFormat& aFormat, gfx::IntSize aCodedSize,
                        gfx::IntRect aVisibleRect, gfx::IntSize aDisplaySize,
-                       Maybe<uint64_t>&& aDuration, int64_t aTimestamp,
+                       const Maybe<uint64_t>& aDuration, int64_t aTimestamp,
                        const VideoColorSpaceInit& aColorSpace)
     : mParent(aParent),
       mResource(Some(Resource(aImage, VideoFrame::Format(aFormat)))),
       mCodedSize(aCodedSize),
       mVisibleRect(aVisibleRect),
       mDisplaySize(aDisplaySize),
-      mDuration(std::move(aDuration)),
+      mDuration(aDuration),
       mTimestamp(aTimestamp),
       mColorSpace(aColorSpace) {
   MOZ_ASSERT(mParent);
@@ -1827,88 +1827,13 @@ void VideoFrame::Close() {
 
 // https://w3c.github.io/webcodecs/#ref-for-deserialization-steps%E2%91%A0
 /* static */
-JSObject* VideoFrame::ReadStructuredClone(JSContext* aCx,
+JSObject* VideoFrame::ReadStructuredClone(
-                                          nsIGlobalObject* aGlobal,
+    JSContext* aCx, nsIGlobalObject* aGlobal, JSStructuredCloneReader* aReader,
-                                          JSStructuredCloneReader* aReader,
+    const VideoFrameSerializedData& aData) {
-                                          const VideoFrameImageData& aData) {
+  if (!IsSameOrigin(aGlobal, aData.mPrincipalURI.get())) {
-  if (!IsSameOrigin(aGlobal, aData.mURI.get())) {
     return nullptr;
   }
 
-  VideoPixelFormat format;
-  if (NS_WARN_IF(!JS_ReadBytes(aReader, &format, 1))) {
-    return nullptr;
-  }
-
-  uint32_t codedWidth = 0;
-  uint32_t codedHeight = 0;
-  if (NS_WARN_IF(!JS_ReadUint32Pair(aReader, &codedWidth, &codedHeight))) {
-    return nullptr;
-  }
-
-  uint32_t visibleX = 0;
-  uint32_t visibleY = 0;
-  uint32_t visibleWidth = 0;
-  uint32_t visibleHeight = 0;
-  if (NS_WARN_IF(!JS_ReadUint32Pair(aReader, &visibleX, &visibleY)) ||
-      NS_WARN_IF(!JS_ReadUint32Pair(aReader, &visibleWidth, &visibleHeight))) {
-    return nullptr;
-  }
-
-  uint32_t displayWidth = 0;
-  uint32_t displayHeight = 0;
-  if (NS_WARN_IF(!JS_ReadUint32Pair(aReader, &displayWidth, &displayHeight))) {
-    return nullptr;
-  }
-
-  uint8_t hasDuration = 0;
-  uint32_t durationLow = 0;
-  uint32_t durationHigh = 0;
-  if (NS_WARN_IF(!JS_ReadBytes(aReader, &hasDuration, 1)) ||
-      NS_WARN_IF(!JS_ReadUint32Pair(aReader, &durationLow, &durationHigh))) {
-    return nullptr;
-  }
-  Maybe<uint64_t> duration =
-      hasDuration ? Some(uint64_t(durationHigh) << 32 | durationLow)
-                  : Nothing();
-
-  uint32_t timestampLow = 0;
-  uint32_t timestampHigh = 0;
-  if (NS_WARN_IF(!JS_ReadUint32Pair(aReader, &timestampLow, &timestampHigh))) {
-    return nullptr;
-  }
-  int64_t timestamp = int64_t(timestampHigh) << 32 | timestampLow;
-
-  uint8_t colorSpaceFullRange = 0;
-  uint8_t colorSpaceMatrix = 0;
-  uint8_t colorSpacePrimaries = 0;
-  uint8_t colorSpaceTransfer = 0;
-  if (NS_WARN_IF(!JS_ReadBytes(aReader, &colorSpaceFullRange, 1)) ||
-      NS_WARN_IF(!JS_ReadBytes(aReader, &colorSpaceMatrix, 1)) ||
-      NS_WARN_IF(!JS_ReadBytes(aReader, &colorSpacePrimaries, 1)) ||
-      NS_WARN_IF(!JS_ReadBytes(aReader, &colorSpaceTransfer, 1))) {
-    return nullptr;
-  }
-  VideoColorSpaceInit colorSpace{};
-  if (colorSpaceFullRange < 2) {
-    colorSpace.mFullRange.Construct(colorSpaceFullRange > 0);
-  }
-  if (colorSpaceMatrix <
-      static_cast<uint8_t>(VideoMatrixCoefficients::EndGuard_)) {
-    colorSpace.mMatrix.Construct(
-        static_cast<VideoMatrixCoefficients>(colorSpaceMatrix));
-  }
-  if (colorSpacePrimaries <
-      static_cast<uint8_t>(VideoColorPrimaries::EndGuard_)) {
-    colorSpace.mPrimaries.Construct(
-        static_cast<VideoColorPrimaries>(colorSpacePrimaries));
-  }
-  if (colorSpaceTransfer <
-      static_cast<uint8_t>(VideoTransferCharacteristics::EndGuard_)) {
-    colorSpace.mTransfer.Construct(
-        static_cast<VideoTransferCharacteristics>(colorSpaceTransfer));
-  }
-
   JS::Rooted<JS::Value> value(aCx, JS::NullValue());
   // To avoid a rooting hazard error from returning a raw JSObject* before
   // running the RefPtr destructor, RefPtr needs to be destructed before
@@ -1917,10 +1842,9 @@ JSObject* VideoFrame::ReadStructuredClone(JSContext* aCx,
   // be safely destructed while the unrooted return JSObject* is on the stack.
   {
     RefPtr<VideoFrame> frame = MakeAndAddRef<VideoFrame>(
-        aGlobal, aData.mImage, format, gfx::IntSize(codedWidth, codedHeight),
+        aGlobal, aData.mImage, aData.mFormat, aData.mCodedSize,
-        gfx::IntRect(visibleX, visibleY, visibleWidth, visibleHeight),
+        aData.mVisibleRect, aData.mDisplaySize, aData.mDuration,
-        gfx::IntSize(displayWidth, displayHeight), std::move(duration),
+        aData.mTimestamp, aData.mColorSpace);
-        timestamp, colorSpace);
     if (!GetOrCreateDOMReflector(aCx, frame, &value) || !value.isObject()) {
       return nullptr;
     }
@@ -1939,61 +1863,17 @@ bool VideoFrame::WriteStructuredClone(JSStructuredCloneWriter* aWriter,
     return false;
   }
 
-  const uint8_t format = BitwiseCast<uint8_t>(mResource->mFormat.PixelFormat());
-
-  const uint32_t codedWidth = BitwiseCast<uint32_t>(mCodedSize.Width());
-  const uint32_t codedHeight = BitwiseCast<uint32_t>(mCodedSize.Height());
-
-  const uint32_t visibleX = BitwiseCast<uint32_t>(mVisibleRect.X());
-  const uint32_t visibleY = BitwiseCast<uint32_t>(mVisibleRect.Y());
-  const uint32_t visibleWidth = BitwiseCast<uint32_t>(mVisibleRect.Width());
-  const uint32_t visibleHeight = BitwiseCast<uint32_t>(mVisibleRect.Height());
-
-  const uint32_t displayWidth = BitwiseCast<uint32_t>(mDisplaySize.Width());
-  const uint32_t displayHeight = BitwiseCast<uint32_t>(mDisplaySize.Height());
-
-  const uint8_t hasDuration = mDuration ? 1 : 0;
-  const uint32_t durationLow = mDuration ? uint32_t(*mDuration) : 0;
-  const uint32_t durationHigh = mDuration ? uint32_t(*mDuration >> 32) : 0;
-
-  const uint32_t timestampLow = uint32_t(mTimestamp);
-  const uint32_t timestampHigh = uint32_t(mTimestamp >> 32);
-
-  const uint8_t colorSpaceFullRange =
-      mColorSpace.mFullRange.WasPassed() ? mColorSpace.mFullRange.Value() : 2;
-  const uint8_t colorSpaceMatrix = BitwiseCast<uint8_t>(
-      mColorSpace.mMatrix.WasPassed() ? mColorSpace.mMatrix.Value()
-                                      : VideoMatrixCoefficients::EndGuard_);
-  const uint8_t colorSpacePrimaries = BitwiseCast<uint8_t>(
-      mColorSpace.mPrimaries.WasPassed() ? mColorSpace.mPrimaries.Value()
-                                         : VideoColorPrimaries::EndGuard_);
-  const uint8_t colorSpaceTransfer =
-      BitwiseCast<uint8_t>(mColorSpace.mTransfer.WasPassed()
-                               ? mColorSpace.mTransfer.Value()
-                               : VideoTransferCharacteristics::EndGuard_);
-
   // Indexing the image and send the index to the receiver.
-  const uint32_t index = aHolder->VideoFrameImages().Length();
+  const uint32_t index = aHolder->VideoFrames().Length();
   RefPtr<layers::Image> image(mResource->mImage.get());
   // The serialization is limited to the same process scope so it's ok to
   // serialize a reference instead of a copy.
-  aHolder->VideoFrameImages().AppendElement(
+  aHolder->VideoFrames().AppendElement(VideoFrameSerializedData{
-      VideoFrameImageData{image.forget(), GetPrincipalURI()});
+      image.forget(), mResource->mFormat.PixelFormat(), mCodedSize,
+      mVisibleRect, mDisplaySize, mDuration, mTimestamp,
+      VideoColorSpaceInit(mColorSpace), GetPrincipalURI()});
 
-  return !(
+  return !NS_WARN_IF(!JS_WriteUint32Pair(aWriter, SCTAG_DOM_VIDEOFRAME, index));
-      NS_WARN_IF(!JS_WriteUint32Pair(aWriter, SCTAG_DOM_VIDEOFRAME, index)) ||
-      NS_WARN_IF(!JS_WriteBytes(aWriter, &format, 1)) ||
-      NS_WARN_IF(!JS_WriteUint32Pair(aWriter, codedWidth, codedHeight)) ||
-      NS_WARN_IF(!JS_WriteUint32Pair(aWriter, visibleX, visibleY)) ||
-      NS_WARN_IF(!JS_WriteUint32Pair(aWriter, visibleWidth, visibleHeight)) ||
-      NS_WARN_IF(!JS_WriteUint32Pair(aWriter, displayWidth, displayHeight)) ||
-      NS_WARN_IF(!JS_WriteBytes(aWriter, &hasDuration, 1)) ||
-      NS_WARN_IF(!JS_WriteUint32Pair(aWriter, durationLow, durationHigh)) ||
-      NS_WARN_IF(!JS_WriteUint32Pair(aWriter, timestampLow, timestampHigh)) ||
-      NS_WARN_IF(!JS_WriteBytes(aWriter, &colorSpaceFullRange, 1)) ||
-      NS_WARN_IF(!JS_WriteBytes(aWriter, &colorSpaceMatrix, 1)) ||
-      NS_WARN_IF(!JS_WriteBytes(aWriter, &colorSpacePrimaries, 1)) ||
-      NS_WARN_IF(!JS_WriteBytes(aWriter, &colorSpaceTransfer, 1)));
 }
 
 // https://w3c.github.io/webcodecs/#ref-for-transfer-steps%E2%91%A0

dom/media/webcodecs/VideoFrame.h

@@ -54,9 +54,16 @@ struct VideoFrameCopyToOptions;
 
 namespace mozilla::dom {
 
-struct VideoFrameImageData {
+struct VideoFrameSerializedData {
   const RefPtr<layers::Image> mImage;
-  const nsCOMPtr<nsIURI> mURI;
+  const VideoPixelFormat mFormat;
+  const gfx::IntSize mCodedSize;
+  const gfx::IntRect mVisibleRect;
+  const gfx::IntSize mDisplaySize;
+  const Maybe<uint64_t> mDuration;
+  const int64_t mTimestamp;
+  const VideoColorSpaceInit mColorSpace;
+  const nsCOMPtr<nsIURI> mPrincipalURI;
 };
 
 class VideoFrame final : public nsISupports, public nsWrapperCache {
@@ -68,7 +75,7 @@ class VideoFrame final : public nsISupports, public nsWrapperCache {
   VideoFrame(nsIGlobalObject* aParent, const RefPtr<layers::Image>& aImage,
              const VideoPixelFormat& aFormat, gfx::IntSize aCodedSize,
              gfx::IntRect aVisibleRect, gfx::IntSize aDisplaySize,
-             Maybe<uint64_t>&& aDuration, int64_t aTimestamp,
+             const Maybe<uint64_t>& aDuration, int64_t aTimestamp,
              const VideoColorSpaceInit& aColorSpace);
 
   VideoFrame(const VideoFrame& aOther);
@@ -146,7 +153,7 @@ class VideoFrame final : public nsISupports, public nsWrapperCache {
   // [Serializable] implementations: {Read, Write}StructuredClone
   static JSObject* ReadStructuredClone(JSContext* aCx, nsIGlobalObject* aGlobal,
                                        JSStructuredCloneReader* aReader,
-                                       const VideoFrameImageData& aImage);
+                                       const VideoFrameSerializedData& aData);
 
   bool WriteStructuredClone(JSStructuredCloneWriter* aWriter,
                             StructuredCloneHolder* aHolder) const;