Bug 1682612 - Check for ArrayBuffer Detach/Transfer in texImage2D(ImageData). r=gfx-reviewers,aosmond
authorKelsey Gilbert <kelsey.gilbert@mozilla.com>
Mon, 21 Mar 2022 23:21:42 +0000
changeset 611408 e17ff252de565e5a8c37b47ddae64068c6a44d33
parent 611407 63b6391367f9b2d762505329151b9880596b54ca
child 611409 24bca595e5e6d695959cb07b42b120e4325e3755
push id39453
push userctuns@mozilla.com
push dateTue, 22 Mar 2022 04:27:58 +0000
treeherdermozilla-central@eee8f4778c79 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfx-reviewers, aosmond
bugs1682612
milestone100.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1682612 - Check for ArrayBuffer Detach/Transfer in texImage2D(ImageData). r=gfx-reviewers,aosmond Differential Revision: https://phabricator.services.mozilla.com/D141689
dom/canvas/ClientWebGLContext.cpp
dom/canvas/ImageData.h
dom/canvas/WebGLTextureUpload.cpp
dom/canvas/test/webgl-mochitest/mochitest.ini
dom/canvas/test/webgl-mochitest/test_imagedata_transfered_arraybuffer.html
--- a/dom/canvas/ClientWebGLContext.cpp
+++ b/dom/canvas/ClientWebGLContext.cpp
@@ -3950,20 +3950,16 @@ void ClientWebGLContext::TexStorage(uint
 }
 
 namespace webgl {
 // TODO: Move these definitions into statics here.
 Maybe<webgl::TexUnpackBlobDesc> FromImageBitmap(
     GLenum target, Maybe<uvec3> size, const dom::ImageBitmap& imageBitmap,
     ErrorResult* const out_rv);
 
-webgl::TexUnpackBlobDesc FromImageData(GLenum target, Maybe<uvec3> size,
-                                       const dom::ImageData& imageData,
-                                       dom::Uint8ClampedArray* const scopedArr);
-
 Maybe<webgl::TexUnpackBlobDesc> FromOffscreenCanvas(
     const ClientWebGLContext&, GLenum target, Maybe<uvec3> size,
     const dom::OffscreenCanvas& src, ErrorResult* const out_error);
 
 Maybe<webgl::TexUnpackBlobDesc> FromDomElem(const ClientWebGLContext&,
                                             GLenum target, Maybe<uvec3> size,
                                             const dom::Element& src,
                                             ErrorResult* const out_error);
@@ -4070,18 +4066,61 @@ void ClientWebGLContext::TexImage(uint8_
     }
 
     if (src.mImageBitmap) {
       return webgl::FromImageBitmap(imageTarget, size, *(src.mImageBitmap),
                                     src.mOut_error);
     }
 
     if (src.mImageData) {
-      return Some(webgl::FromImageData(imageTarget, size, *(src.mImageData),
-                                       &scopedArr));
+      const auto& imageData = *src.mImageData;
+      MOZ_RELEASE_ASSERT(scopedArr.Init(imageData.GetDataObject()));
+      scopedArr.ComputeState();
+      const auto dataSize = scopedArr.Length();
+      const auto data = reinterpret_cast<uint8_t*>(scopedArr.Data());
+      if (!data) {
+        // Neutered, e.g. via Transfer
+        EnqueueError(LOCAL_GL_INVALID_VALUE,
+                     "ImageData.data.buffer is Detached. (Maybe you Transfered "
+                     "it to a Worker?");
+        return {};
+      }
+
+      // -
+
+      const gfx::IntSize imageSize(imageData.Width(), imageData.Height());
+      const auto sizeFromDims =
+          CheckedInt<size_t>(imageSize.width) * imageSize.height * 4;
+      MOZ_RELEASE_ASSERT(sizeFromDims.isValid() &&
+                         sizeFromDims.value() == dataSize);
+
+      const RefPtr<gfx::DataSourceSurface> surf =
+          gfx::Factory::CreateWrappingDataSourceSurface(
+              data, imageSize.width * 4, imageSize,
+              gfx::SurfaceFormat::R8G8B8A8);
+      MOZ_ASSERT(surf);
+
+      // -
+
+      const auto imageUSize = *uvec2::FromSize(imageSize);
+      const auto concreteSize =
+          size.valueOr(uvec3{imageUSize.x, imageUSize.y, 1});
+
+      // WhatWG "HTML Living Standard" (30 October 2015):
+      // "The getImageData(sx, sy, sw, sh) method [...] Pixels must be returned
+      // as non-premultiplied alpha values."
+      return Some(webgl::TexUnpackBlobDesc{imageTarget,
+                                           concreteSize,
+                                           gfxAlphaType::NonPremult,
+                                           {},
+                                           {},
+                                           Some(imageUSize),
+                                           nullptr,
+                                           {},
+                                           surf});
     }
 
     if (src.mOffscreenCanvas) {
       return webgl::FromOffscreenCanvas(
           *this, imageTarget, size, *(src.mOffscreenCanvas), src.mOut_error);
     }
 
     if (src.mDomElem) {
--- a/dom/canvas/ImageData.h
+++ b/dom/canvas/ImageData.h
@@ -27,27 +27,32 @@ class ErrorResult;
 
 namespace dom {
 
 class GlobalObject;
 template <typename T>
 class Optional;
 
 class ImageData final : public nsISupports {
-  ~ImageData() { DropData(); }
+ public:
+  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
+  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(ImageData)
+
+  const uint32_t mWidth;
+  const uint32_t mHeight;
+
+ private:
+  JS::Heap<JSObject*> mData;
 
  public:
   ImageData(uint32_t aWidth, uint32_t aHeight, JSObject& aData)
       : mWidth(aWidth), mHeight(aHeight), mData(&aData) {
     HoldData();
   }
 
-  NS_DECL_CYCLE_COLLECTING_ISUPPORTS
-  NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(ImageData)
-
   static already_AddRefed<ImageData> Constructor(const GlobalObject& aGlobal,
                                                  const uint32_t aWidth,
                                                  const uint32_t aHeight,
                                                  ErrorResult& aRv);
 
   static already_AddRefed<ImageData> Constructor(
       const GlobalObject& aGlobal, const Uint8ClampedArray& aData,
       const uint32_t aWidth, const Optional<uint32_t>& aHeight,
@@ -70,17 +75,15 @@ class ImageData final : public nsISuppor
   bool WriteStructuredClone(JSContext* aCx,
                             JSStructuredCloneWriter* aWriter) const;
 
  private:
   void HoldData();
   void DropData();
 
   ImageData() = delete;
-
-  uint32_t mWidth, mHeight;
-  JS::Heap<JSObject*> mData;
+  ~ImageData() { DropData(); }
 };
 
 }  // namespace dom
 }  // namespace mozilla
 
 #endif  // mozilla_dom_ImageData_h
--- a/dom/canvas/WebGLTextureUpload.cpp
+++ b/dom/canvas/WebGLTextureUpload.cpp
@@ -67,51 +67,16 @@ Maybe<TexUnpackBlobDesc> FromImageBitmap
                                 Some(imageSize),
                                 nullptr,
                                 {},
                                 surf,
                                 {},
                                 false});
 }
 
-TexUnpackBlobDesc FromImageData(const GLenum target, Maybe<uvec3> size,
-                                const dom::ImageData& imageData,
-                                dom::Uint8ClampedArray* const scopedArr) {
-  MOZ_RELEASE_ASSERT(scopedArr->Init(imageData.GetDataObject()));
-  scopedArr->ComputeState();
-  const size_t dataSize = scopedArr->Length();
-  const auto data = reinterpret_cast<uint8_t*>(scopedArr->Data());
-
-  const gfx::IntSize imageISize(imageData.Width(), imageData.Height());
-  const auto imageUSize = *uvec2::FromSize(imageISize);
-  const size_t stride = imageUSize.x * 4;
-  const gfx::SurfaceFormat surfFormat = gfx::SurfaceFormat::R8G8B8A8;
-  MOZ_ALWAYS_TRUE(dataSize == stride * imageUSize.y);
-
-  const RefPtr<gfx::DataSourceSurface> surf =
-      gfx::Factory::CreateWrappingDataSourceSurface(data, stride, imageISize,
-                                                    surfFormat);
-  MOZ_ASSERT(surf);
-
-  ////
-
-  if (!size) {
-    size.emplace(imageUSize.x, imageUSize.y, 1);
-  }
-
-  ////
-
-  // WhatWG "HTML Living Standard" (30 October 2015):
-  // "The getImageData(sx, sy, sw, sh) method [...] Pixels must be returned as
-  // non-premultiplied alpha values."
-  return {target,  size.value(), gfxAlphaType::NonPremult,
-          {},      {},           Some(imageUSize),
-          nullptr, {},           surf};
-}
-
 static layers::SurfaceDescriptor Flatten(const layers::SurfaceDescriptor& sd) {
   const auto sdType = sd.type();
   if (sdType != layers::SurfaceDescriptor::TSurfaceDescriptorGPUVideo) {
     return sd;
   }
   const auto& sdv = sd.get_SurfaceDescriptorGPUVideo();
   const auto& sdvType = sdv.type();
   if (sdvType !=
--- a/dom/canvas/test/webgl-mochitest/mochitest.ini
+++ b/dom/canvas/test/webgl-mochitest/mochitest.ini
@@ -69,16 +69,17 @@ support-files = ../captureStream_common.
 [test_depth_tex_lazy_clear.html]
 [test_draw.html]
 [test_fb_param.html]
 [test_fb_param_crash.html]
 [test_has_rbab.html]
 fail-if = (os == 'android') || (os == 'linux') || (os == 'mac')
 [test_hidden_alpha.html]
 [test_hidden_depth_stencil.html]
+[test_imagedata_transfered_arraybuffer.html]
 [test_implicit_color_buffer_float.html]
 [test_highp_fs.html]
 [test_no_arr_points.html]
 [test_noprog_draw.html]
 [test_pixel_pack_buffer.html]
 # skip-if = os == "win" && os_version == "10.0" # Bug 1302199
 skip-if =
   (os == "win") # Unofficial DXGL support regressed by bug 1632249
new file mode 100644
--- /dev/null
+++ b/dom/canvas/test/webgl-mochitest/test_imagedata_transfered_arraybuffer.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <meta charset=utf-8>
+    <script src='/tests/SimpleTest/SimpleTest.js'></script>
+    <link rel='stylesheet' href='/tests/SimpleTest/test.css'>
+</head>
+<body>
+    <script>
+'use strict';
+const ab = new ArrayBuffer(4);
+const ta = new Uint8ClampedArray(ab);
+const idata = new ImageData(ta, 1);
+const canvas = document.createElement('canvas');
+const gl = canvas.getContext('webgl2');
+const worker = new Worker('worker.js');
+worker.postMessage([ab], [ab]);
+gl.texImage2D(gl.TEXTURE_CUBE_MAP_NEGATIVE_X, 1, gl.RGB, idata.width, idata.height, 0, gl.RGB, gl.UNSIGNED_SHORT_5_6_5, idata);
+const err = gl.getError();
+window.ok = window.ok || console.log;
+ok(err == gl.INVALID_VALUE, 'texImage2D(ImageData) with Transferred ArrayBuffer is INVALID_VALUE');
+    </script>
+</body>
+</html>