From 2d1097c27f31cb15f7588b043d691cda9710d14b Mon Sep 17 00:00:00 2001 From: Marian-Vasile Laza Date: Wed, 26 Oct 2022 23:00:01 +0300 Subject: [PATCH] Backed out 6 changesets (bug 1774302) for causing bustages on VideoFrame.cpp. CLOSED TREE Backed out changeset 6ff613ee0977 (bug 1774302) Backed out changeset b94e43f1e91f (bug 1774302) Backed out changeset 5a2fc97cac78 (bug 1774302) Backed out changeset 3db9d390aa0d (bug 1774302) Backed out changeset 0009cdcc3e11 (bug 1774302) Backed out changeset 7328aadd86e9 (bug 1774302) --- dom/base/StructuredCloneHolder.cpp | 21 -- dom/base/StructuredCloneHolder.h | 11 +- dom/base/StructuredCloneTags.h | 2 - dom/media/webcodecs/VideoFrame.cpp | 194 +----------------- dom/media/webcodecs/VideoFrame.h | 15 -- dom/webidl/VideoFrame.webidl | 3 +- .../video-frame-serialization.any.js.ini | 31 ++- ...e-serialization.crossAgentCluster.html.ini | 5 - ...erialization.crossAgentCluster.helper.html | 23 --- ...Frame-serialization.crossAgentCluster.html | 76 ------- 10 files changed, 31 insertions(+), 350 deletions(-) delete mode 100644 testing/web-platform/meta/webcodecs/videoFrame-serialization.crossAgentCluster.html.ini delete mode 100644 testing/web-platform/tests/webcodecs/videoFrame-serialization.crossAgentCluster.helper.html delete mode 100644 testing/web-platform/tests/webcodecs/videoFrame-serialization.crossAgentCluster.html diff --git a/dom/base/StructuredCloneHolder.cpp b/dom/base/StructuredCloneHolder.cpp index 1496f8d31c8e..d305b7ae7eeb 100644 --- a/dom/base/StructuredCloneHolder.cpp +++ b/dom/base/StructuredCloneHolder.cpp @@ -55,8 +55,6 @@ #include "mozilla/dom/ToJSValue.h" #include "mozilla/dom/TransformStream.h" #include "mozilla/dom/TransformStreamBinding.h" -#include "mozilla/dom/VideoFrame.h" -#include "mozilla/dom/VideoFrameBinding.h" #include "mozilla/dom/WebIDLSerializable.h" #include "mozilla/dom/WritableStream.h" #include "mozilla/dom/WritableStreamBinding.h" @@ -399,7 +397,6 @@ void StructuredCloneHolder::Read(nsIGlobalObject* aGlobal, JSContext* aCx, mWasmModuleArray.Clear(); mClonedSurfaces.Clear(); mInputStreamArray.Clear(); - mVideoFrameImages.Clear(); Clear(); } } @@ -1025,13 +1022,6 @@ JSObject* StructuredCloneHolder::CustomReadHandler( return ClonedErrorHolder::ReadStructuredClone(aCx, aReader, this); } - if (StaticPrefs::dom_media_webcodecs_enabled() && - aTag == SCTAG_DOM_VIDEOFRAME && - CloneScope() == StructuredCloneScope::SameProcess) { - return VideoFrame::ReadStructuredClone(aCx, mGlobal, aReader, - VideoFrameImages()[aIndex]); - } - return ReadFullySerializableObjects(aCx, aReader, aTag); } @@ -1128,17 +1118,6 @@ bool StructuredCloneHolder::CustomWriteHandler( return false; } - // See if this is a VideoFrame object. - if (StaticPrefs::dom_media_webcodecs_enabled()) { - VideoFrame* videoFrame = nullptr; - if (NS_SUCCEEDED(UNWRAP_OBJECT(VideoFrame, &obj, videoFrame))) { - SameProcessScopeRequired(aSameProcessScopeRequired); - return CloneScope() == StructuredCloneScope::SameProcess - ? videoFrame->WriteStructuredClone(aWriter, this) - : false; - } - } - { // We only care about streams, so ReflectorToISupportsStatic is fine. nsCOMPtr base = xpc::ReflectorToISupportsStatic(aObj); diff --git a/dom/base/StructuredCloneHolder.h b/dom/base/StructuredCloneHolder.h index 6f00c44ebd37..05fb3effd11e 100644 --- a/dom/base/StructuredCloneHolder.h +++ b/dom/base/StructuredCloneHolder.h @@ -166,7 +166,6 @@ class StructuredCloneHolderBase { class BlobImpl; class MessagePort; class MessagePortIdentifier; -struct VideoFrameImageData; class StructuredCloneHolder : public StructuredCloneHolderBase { public: @@ -209,8 +208,7 @@ class StructuredCloneHolder : public StructuredCloneHolderBase { // Call this method to know if this object is keeping some DOM object alive. bool HasClonedDOMObjects() const { return !mBlobImplArray.IsEmpty() || !mWasmModuleArray.IsEmpty() || - !mClonedSurfaces.IsEmpty() || !mInputStreamArray.IsEmpty() || - !mVideoFrameImages.IsEmpty(); + !mClonedSurfaces.IsEmpty() || !mInputStreamArray.IsEmpty(); } nsTArray>& BlobImpls() { @@ -266,10 +264,6 @@ class StructuredCloneHolder : public StructuredCloneHolderBase { return mClonedSurfaces; } - nsTArray& VideoFrameImages() { - return mVideoFrameImages; - } - // Implementations of the virtual methods to allow cloning of objects which // JS engine itself doesn't clone. @@ -367,9 +361,6 @@ class StructuredCloneHolder : public StructuredCloneHolderBase { // instance, so no race condition will occur. nsTArray> mClonedSurfaces; - // Used for cloning VideoFrame in the structured cloning algorithm. - nsTArray mVideoFrameImages; - // This raw pointer is only set within ::Read() and is unset by the end. nsIGlobalObject* MOZ_NON_OWNING_REF mGlobal; diff --git a/dom/base/StructuredCloneTags.h b/dom/base/StructuredCloneTags.h index 42f2e4e1d139..041a0859b34c 100644 --- a/dom/base/StructuredCloneTags.h +++ b/dom/base/StructuredCloneTags.h @@ -158,8 +158,6 @@ enum StructuredCloneTags : uint32_t { SCTAG_DOM_TRANSFORMSTREAM, - SCTAG_DOM_VIDEOFRAME, - // IMPORTANT: If you plan to add an new IDB tag, it _must_ be add before the // "less stable" tags! }; diff --git a/dom/media/webcodecs/VideoFrame.cpp b/dom/media/webcodecs/VideoFrame.cpp index 1e4febf7309a..58913f7fe8bc 100644 --- a/dom/media/webcodecs/VideoFrame.cpp +++ b/dom/media/webcodecs/VideoFrame.cpp @@ -12,7 +12,6 @@ #include "ImageContainer.h" #include "VideoColorSpace.h" -#include "js/StructuredClone.h" #include "mozilla/Maybe.h" #include "mozilla/Result.h" #include "mozilla/ResultVariant.h" @@ -29,14 +28,11 @@ #include "mozilla/dom/OffscreenCanvas.h" #include "mozilla/dom/Promise.h" #include "mozilla/dom/SVGImageElement.h" -#include "mozilla/dom/StructuredCloneHolder.h" -#include "mozilla/dom/StructuredCloneTags.h" #include "mozilla/dom/UnionTypes.h" #include "mozilla/gfx/2D.h" #include "mozilla/gfx/Swizzle.h" #include "nsLayoutUtils.h" #include "nsIPrincipal.h" -#include "nsIURI.h" namespace mozilla::dom { @@ -256,15 +252,6 @@ class NV12BufferReader final : public YUVBufferReaderBase { * The followings are helpers defined in * https://w3c.github.io/webcodecs/#videoframe-algorithms */ -static bool IsSameOrigin(nsIGlobalObject* aGlobal, nsIURI* aURI) { - MOZ_ASSERT(aGlobal); - - nsIPrincipal* principal = aGlobal->PrincipalOrNull(); - // If VideoFrames is created in worker, then it's from the same origin. In - // this case, principal or aURI is null. Otherwise, check the origin. - return !principal || !aURI || principal->IsSameOrigin(aURI); -} - static bool IsSameOrigin(nsIGlobalObject* aGlobal, const VideoFrame& aFrame) { MOZ_ASSERT(aGlobal); MOZ_ASSERT(aFrame.GetParentObject()); @@ -1192,16 +1179,10 @@ VideoFrame::VideoFrame(const VideoFrame& aOther) MOZ_ASSERT(mParent); } -nsIGlobalObject* VideoFrame::GetParentObject() const { - AssertIsOnOwningThread(); - - return mParent.get(); -} +nsIGlobalObject* VideoFrame::GetParentObject() const { return mParent.get(); } JSObject* VideoFrame::WrapObject(JSContext* aCx, JS::Handle aGivenProto) { - AssertIsOnOwningThread(); - return VideoFrame_Binding::Wrap(aCx, this, aGivenProto); } @@ -1806,179 +1787,6 @@ void VideoFrame::Close() { mTimestamp.reset(); } -// https://w3c.github.io/webcodecs/#ref-for-deserialization-steps%E2%91%A0 -/* static */ -JSObject* VideoFrame::ReadStructuredClone(JSContext* aCx, - nsIGlobalObject* aGlobal, - JSStructuredCloneReader* aReader, - const VideoFrameImageData& aData) { - 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 duration = - hasDuration ? Some(uint64_t(durationHigh) << 32 | durationLow) - : Nothing(); - - uint8_t hasTimestamp = 0; - uint32_t timestampLow = 0; - uint32_t timestampHigh = 0; - if (NS_WARN_IF(!JS_ReadBytes(aReader, &hasTimestamp, 1)) || - NS_WARN_IF(!JS_ReadUint32Pair(aReader, ×tampLow, ×tampHigh))) { - return nullptr; - } - Maybe timestamp = - hasTimestamp ? Some(uint64_t(timestampHigh) << 32 | timestampLow) - : Nothing(); - - 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(VideoMatrixCoefficients::EndGuard_)) { - colorSpace.mMatrix.Construct( - static_cast(colorSpaceMatrix)); - } - if (colorSpacePrimaries < - static_cast(VideoColorPrimaries::EndGuard_)) { - colorSpace.mPrimaries.Construct( - static_cast(colorSpacePrimaries)); - } - if (colorSpaceTransfer < - static_cast(VideoTransferCharacteristics::EndGuard_)) { - colorSpace.mTransfer.Construct( - static_cast(colorSpaceTransfer)); - } - - RefPtr frame = MakeAndAddRef( - aGlobal, aData.mImage, format, gfx::IntSize(codedWidth, codedHeight), - gfx::IntRect(visibleX, visibleY, visibleWidth, visibleHeight), - gfx::IntSize(displayWidth, displayHeight), std::move(duration), - std::move(timestamp), colorSpace); - - JS::Rooted value(aCx, JS::NullValue()); - if (!GetOrCreateDOMReflector(aCx, frame, &value)) { - return nullptr; - } - return value.toObjectOrNull(); -} - -// https://w3c.github.io/webcodecs/#ref-for-serialization-steps%E2%91%A0 -bool VideoFrame::WriteStructuredClone(JSStructuredCloneWriter* aWriter, - StructuredCloneHolder* aHolder) const { - AssertIsOnOwningThread(); - - // TODO: Throw error if this is _detached_ instead of checking resource (bug - // 1774306). - if (!mResource) { - return false; - } - - const uint8_t format = BitwiseCast(mResource->mFormat.PixelFormat()); - - const uint32_t codedWidth = BitwiseCast(mCodedSize.Width()); - const uint32_t codedHeight = BitwiseCast(mCodedSize.Height()); - - const uint32_t visibleX = BitwiseCast(mVisibleRect.X()); - const uint32_t visibleY = BitwiseCast(mVisibleRect.Y()); - const uint32_t visibleWidth = BitwiseCast(mVisibleRect.Width()); - const uint32_t visibleHeight = BitwiseCast(mVisibleRect.Height()); - - const uint32_t displayWidth = BitwiseCast(mDisplaySize.Width()); - const uint32_t displayHeight = BitwiseCast(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 uint8_t hasTimestamp = mTimestamp ? 1 : 0; - const uint32_t timestampLow = mTimestamp ? uint32_t(*mTimestamp) : 0; - const uint32_t timestampHigh = mTimestamp ? uint32_t(*mTimestamp >> 32) : 0; - - const uint8_t colorSpaceFullRange = - mColorSpace.mFullRange.WasPassed() ? mColorSpace.mFullRange.Value() : 2; - const uint8_t colorSpaceMatrix = BitwiseCast( - mColorSpace.mMatrix.WasPassed() ? mColorSpace.mMatrix.Value() - : VideoMatrixCoefficients::EndGuard_); - const uint8_t colorSpacePrimaries = BitwiseCast( - mColorSpace.mPrimaries.WasPassed() ? mColorSpace.mPrimaries.Value() - : VideoColorPrimaries::EndGuard_); - const uint8_t colorSpaceTransfer = - BitwiseCast(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(); - RefPtr 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. - nsIPrincipal* principal = mParent->PrincipalOrNull(); - nsCOMPtr uri = principal ? principal->GetURI() : nullptr; - aHolder->VideoFrameImages().AppendElement( - VideoFrameImageData{image.forget(), uri}); - - return !( - 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_WriteBytes(aWriter, &hasTimestamp, 1)) || - 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))); -} - /* * VideoFrame::Format * diff --git a/dom/media/webcodecs/VideoFrame.h b/dom/media/webcodecs/VideoFrame.h index 50628abf5109..8f0f0f0c8d57 100644 --- a/dom/media/webcodecs/VideoFrame.h +++ b/dom/media/webcodecs/VideoFrame.h @@ -21,7 +21,6 @@ #include "nsWrapperCache.h" class nsIGlobalObject; -class nsIURI; namespace mozilla { @@ -41,7 +40,6 @@ class OffscreenCanvas; class OwningMaybeSharedArrayBufferViewOrMaybeSharedArrayBuffer; class Promise; class SVGImageElement; -class StructuredCloneHolder; class VideoColorSpace; class VideoFrame; enum class VideoPixelFormat : uint8_t; @@ -54,11 +52,6 @@ struct VideoFrameCopyToOptions; namespace mozilla::dom { -struct VideoFrameImageData { - const RefPtr mImage; - const nsCOMPtr mURI; -}; - class VideoFrame final : public nsISupports, public nsWrapperCache { public: NS_DECL_CYCLE_COLLECTING_ISUPPORTS @@ -143,14 +136,6 @@ class VideoFrame final : public nsISupports, public nsWrapperCache { void Close(); - // [Serializable] implementations: {Read, Write}StructuredClone - static JSObject* ReadStructuredClone(JSContext* aCx, nsIGlobalObject* aGlobal, - JSStructuredCloneReader* aReader, - const VideoFrameImageData& aImage); - - bool WriteStructuredClone(JSStructuredCloneWriter* aWriter, - StructuredCloneHolder* aHolder) const; - public: // A VideoPixelFormat wrapper providing utilities for VideoFrame. class Format final { diff --git a/dom/webidl/VideoFrame.webidl b/dom/webidl/VideoFrame.webidl index 72c86ac1f8f9..9d23f165fd26 100644 --- a/dom/webidl/VideoFrame.webidl +++ b/dom/webidl/VideoFrame.webidl @@ -12,8 +12,7 @@ enum AlphaOption { "discard", }; -// [Serializable] is implemented without adding attribute here. -[Exposed=(Window,DedicatedWorker) /*, Transferable (bug 1774306) */, Pref="dom.media.webcodecs.enabled"] +[Exposed=(Window,DedicatedWorker) /* , Serializable (bug 1774302), Transferable (bug 1774306) */, Pref="dom.media.webcodecs.enabled"] interface VideoFrame { // The constructors should be shorten to: // ``` diff --git a/testing/web-platform/meta/webcodecs/video-frame-serialization.any.js.ini b/testing/web-platform/meta/webcodecs/video-frame-serialization.any.js.ini index 8d420c9f452f..4bd29f488a7f 100644 --- a/testing/web-platform/meta/webcodecs/video-frame-serialization.any.js.ini +++ b/testing/web-platform/meta/webcodecs/video-frame-serialization.any.js.ini @@ -1,15 +1,40 @@ [video-frame-serialization.any.worker.html] - prefs: [dom.media.webcodecs.enabled:true] + [Test we can clone a VideoFrame.] + expected: FAIL + + [Verify closing a frame doesn't affect its clones.] + expected: FAIL + + [Verify cloning a closed frame throws.] + expected: FAIL + + [Verify posting closed frames throws.] + expected: FAIL + + [Verify closing frames does not propagate accross contexts.] + expected: FAIL [Verify transferring frames closes them.] expected: FAIL [video-frame-serialization.any.html] - prefs: [dom.media.webcodecs.enabled:true] expected: if (os == "android") and fission: [OK, TIMEOUT] + [Test we can clone a VideoFrame.] + expected: FAIL + + [Verify closing a frame doesn't affect its clones.] + expected: FAIL + + [Verify cloning a closed frame throws.] + expected: FAIL + + [Verify posting closed frames throws.] + expected: FAIL + + [Verify closing frames does not propagate accross contexts.] + expected: FAIL [Verify transferring frames closes them.] expected: FAIL - diff --git a/testing/web-platform/meta/webcodecs/videoFrame-serialization.crossAgentCluster.html.ini b/testing/web-platform/meta/webcodecs/videoFrame-serialization.crossAgentCluster.html.ini deleted file mode 100644 index 90e242c4bbc9..000000000000 --- a/testing/web-platform/meta/webcodecs/videoFrame-serialization.crossAgentCluster.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[videoFrame-serialization.crossAgentCluster.html] - prefs: [dom.media.webcodecs.enabled:true] - expected: - if (os == "android") and fission: [OK, TIMEOUT] - diff --git a/testing/web-platform/tests/webcodecs/videoFrame-serialization.crossAgentCluster.helper.html b/testing/web-platform/tests/webcodecs/videoFrame-serialization.crossAgentCluster.helper.html deleted file mode 100644 index 8e751632a1f2..000000000000 --- a/testing/web-platform/tests/webcodecs/videoFrame-serialization.crossAgentCluster.helper.html +++ /dev/null @@ -1,23 +0,0 @@ - - - -

-
- - - diff --git a/testing/web-platform/tests/webcodecs/videoFrame-serialization.crossAgentCluster.html b/testing/web-platform/tests/webcodecs/videoFrame-serialization.crossAgentCluster.html deleted file mode 100644 index c2ab29850e88..000000000000 --- a/testing/web-platform/tests/webcodecs/videoFrame-serialization.crossAgentCluster.html +++ /dev/null @@ -1,76 +0,0 @@ - - - - - - - - - - - -