Make ImageBuffer::GetImageData to be a static function in StaticBitmapImage

As a prepartory step to remove ImageBuffer, this CL moves the GetImageData
function (which has nothing to do with ImageData) from ImageBuffer to
StaticBitmapImage. Some unnecessary checks (e.g., IsSurfaceValid() which has
been checked over and over again) in the function have been removed.

Bug: 776806
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Iab7a346644027901bb10c96cc9c62a0e4336748d
Reviewed-on: https://chromium-review.googlesource.com/820016
Commit-Queue: Olivia Lai <xlai@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523501}

third_party/WebKit/Source/core/layout/shapes/Shape.cpp

@@ -42,12 +42,14 @@
 #include "platform/geometry/FloatRoundedRect.h"
 #include "platform/geometry/FloatSize.h"
 #include "platform/graphics/GraphicsTypes.h"
-#include "platform/graphics/ImageBuffer.h"
+#include "platform/graphics/StaticBitmapImage.h"
+#include "platform/graphics/paint/PaintCanvas.h"
 #include "platform/graphics/paint/PaintFlags.h"
 #include "platform/wtf/MathExtras.h"
 #include "platform/wtf/PtrUtil.h"
 #include "platform/wtf/typed_arrays/ArrayBufferContents.h"
 #include "public/platform/Platform.h"
+#include "third_party/skia/include/core/SkSurface.h"
 
 namespace blink {
 
@@ -235,8 +237,15 @@ static bool ExtractImageData(Image* image,
   if (!image)
     return false;
 
-  std::unique_ptr<ImageBuffer> image_buffer = ImageBuffer::Create(image_size);
-  if (!image_buffer)
+  CanvasColorParams color_params;
+  SkImageInfo info = SkImageInfo::Make(
+      image_size.Width(), image_size.Height(), color_params.GetSkColorType(),
+      color_params.GetSkAlphaType(),
+      color_params.GetSkColorSpaceForSkSurfaces());
+  sk_sp<SkSurface> surface =
+      SkSurface::MakeRaster(info, color_params.GetSkSurfaceProps());
+
+  if (!surface)
     return false;
 
   // FIXME: This is not totally correct but it is needed to prevent shapes
@@ -247,11 +256,18 @@ static bool ExtractImageData(Image* image,
   IntRect image_source_rect(IntPoint(), image->Size());
   IntRect image_dest_rect(IntPoint(), image_size);
   // TODO(ccameron): No color conversion is required here.
-  image->Draw(image_buffer->Canvas(), flags, image_dest_rect, image_source_rect,
+  std::unique_ptr<PaintCanvas> canvas =
+      color_params.WrapCanvas(surface->getCanvas());
+  canvas->save();
+  canvas->clear(SK_ColorTRANSPARENT);
+
+  image->Draw(canvas.get(), flags, image_dest_rect, image_source_rect,
               kDoNotRespectImageOrientation,
               Image::kDoNotClampImageToSourceRect, Image::kSyncDecode);
 
-  return image_buffer->GetImageData(image_dest_rect, contents);
+  return StaticBitmapImage::ConvertToArrayBufferContents(
+      StaticBitmapImage::Create(surface->makeImageSnapshot()), contents,
+      image_dest_rect, color_params);
 }
 
 static std::unique_ptr<RasterShapeIntervals> ExtractIntervalsFromImageData(

third_party/WebKit/Source/modules/canvas/canvas2d/BaseRenderingContext2D.cpp

@@ -1591,14 +1591,21 @@ ImageData* BaseRenderingContext2D::getImageData(
   }
 
   WTF::ArrayBufferContents contents;
-  bool is_gpu_readback_invoked = false;
-  if (!buffer->GetImageData(image_data_rect, contents,
-                            &is_gpu_readback_invoked)) {
+
+  const CanvasColorParams& color_params = buffer->ColorParams();
+  scoped_refptr<StaticBitmapImage> snapshot = buffer->NewImageSnapshot(
+      kPreferNoAcceleration, kSnapshotReasonGetImageData);
+
+  if (!StaticBitmapImage::ConvertToArrayBufferContents(
+          snapshot, contents, image_data_rect, color_params,
+          buffer->IsAccelerated())) {
     exception_state.ThrowRangeError("Out of memory at ImageData creation");
     return nullptr;
   }
 
-  if (is_gpu_readback_invoked) {
+  if (!!snapshot) {
+    // If source image is not null, the ConvertToArrayBufferContents function
+    // must have invoked SkImage::readPixels.
     DidInvokeGPUReadbackInCurrentFrame();
   }
 

third_party/WebKit/Source/platform/BUILD.gn

@@ -1810,8 +1810,8 @@ jumbo_source_set("blink_platform_unittests_sources") {
     "graphics/DeferredImageDecoderTestWoPlatform.cpp",
     "graphics/GraphicsContextTest.cpp",
     "graphics/HighContrastImageClassifierTest.cpp",
-    "graphics/ImageBufferTest.cpp",
     "graphics/PaintInvalidationReasonTest.cpp",
+    "graphics/StaticBitmapImageTest.cpp",
     "graphics/VideoFrameSubmitterTest.cpp",
     "graphics/compositing/CompositedLayerRasterInvalidatorTest.cpp",
     "graphics/compositing/PaintArtifactCompositorTest.cpp",

third_party/WebKit/Source/platform/graphics/ImageBuffer.cpp

@@ -54,7 +54,6 @@
 #include "platform/wtf/Vector.h"
 #include "platform/wtf/text/Base64.h"
 #include "platform/wtf/text/WTFString.h"
-#include "platform/wtf/typed_arrays/ArrayBufferContents.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebGraphicsContext3DProvider.h"
 #include "skia/ext/texture_handle.h"
@@ -266,70 +265,6 @@ void ImageBuffer::Draw(GraphicsContext& context,
   surface_->Draw(context, dest_rect, src_rect, op);
 }
 
-bool ImageBuffer::GetImageData(const IntRect& rect,
-                               WTF::ArrayBufferContents& contents,
-                               bool* is_gpu_readback_invoked) const {
-  uint8_t bytes_per_pixel = surface_->ColorParams().BytesPerPixel();
-  CheckedNumeric<int> data_size = bytes_per_pixel;
-  data_size *= rect.Size().Area();
-  if (!data_size.IsValid() ||
-      data_size.ValueOrDie() > v8::TypedArray::kMaxLength)
-    return false;
-
-  if (!IsSurfaceValid()) {
-    size_t alloc_size_in_bytes = rect.Size().Area() * bytes_per_pixel;
-    auto data = WTF::ArrayBufferContents::CreateDataHandle(
-        alloc_size_in_bytes, WTF::ArrayBufferContents::kZeroInitialize);
-    if (!data)
-      return false;
-    WTF::ArrayBufferContents result(std::move(data), alloc_size_in_bytes,
-                                    WTF::ArrayBufferContents::kNotShared);
-    result.Transfer(contents);
-    return true;
-  }
-
-  DCHECK(Canvas());
-
-  scoped_refptr<StaticBitmapImage> snapshot = surface_->NewImageSnapshot(
-      kPreferNoAcceleration, kSnapshotReasonGetImageData);
-  if (!snapshot)
-    return false;
-
-  const bool may_have_stray_area =
-      surface_->IsAccelerated()  // GPU readback may fail silently
-      || rect.X() < 0 || rect.Y() < 0 ||
-      rect.MaxX() > surface_->Size().Width() ||
-      rect.MaxY() > surface_->Size().Height();
-  size_t alloc_size_in_bytes = rect.Size().Area() * bytes_per_pixel;
-  WTF::ArrayBufferContents::InitializationPolicy initialization_policy =
-      may_have_stray_area ? WTF::ArrayBufferContents::kZeroInitialize
-                          : WTF::ArrayBufferContents::kDontInitialize;
-  auto data = WTF::ArrayBufferContents::CreateDataHandle(alloc_size_in_bytes,
-                                                         initialization_policy);
-  if (!data)
-    return false;
-  WTF::ArrayBufferContents result(std::move(data), alloc_size_in_bytes,
-                                  WTF::ArrayBufferContents::kNotShared);
-
-  SkColorType color_type =
-      (surface_->ColorParams().GetSkColorType() == kRGBA_F16_SkColorType)
-          ? kRGBA_F16_SkColorType
-          : kRGBA_8888_SkColorType;
-  SkImageInfo info = SkImageInfo::Make(rect.Width(), rect.Height(), color_type,
-                                       kUnpremul_SkAlphaType);
-  sk_sp<SkImage> sk_image = snapshot->PaintImageForCurrentFrame().GetSkImage();
-  bool read_pixels_successful = sk_image->readPixels(
-      info, result.Data(), info.minRowBytes(), rect.X(), rect.Y());
-  DCHECK(read_pixels_successful ||
-         !sk_image->bounds().intersect(SkIRect::MakeXYWH(
-             rect.X(), rect.Y(), info.width(), info.height())));
-  if (is_gpu_readback_invoked) {
-    *is_gpu_readback_invoked = true;
-  }
-  result.Transfer(contents);
-  return true;
-}
-
 void ImageBuffer::PutByteArray(const unsigned char* source,
                                const IntSize& source_size,
                                const IntRect& source_rect,

third_party/WebKit/Source/platform/graphics/ImageBuffer.h

@@ -54,12 +54,6 @@ class GLES2Interface;
 }
 }
 
-namespace WTF {
-
-class ArrayBufferContents;
-
-}  // namespace WTF
-
 namespace blink {
 
 class DrawingBuffer;
@@ -108,10 +102,6 @@ class PLATFORM_EXPORT ImageBuffer {
 
   void WillOverwriteCanvas() { surface_->WillOverwriteCanvas(); }
 
-  bool GetImageData(const IntRect&,
-                    WTF::ArrayBufferContents&,
-                    bool* is_gpu_readback_invoked = nullptr) const;
-
   void PutByteArray(const unsigned char* source,
                     const IntSize& source_size,
                     const IntRect& source_rect,
@@ -146,6 +136,9 @@ class PLATFORM_EXPORT ImageBuffer {
   // TODO(xlai): This function is an intermediate step making canvas element
   // have reference to Canvas2DLayerBridge. See crbug.com/776806.
   void OnCanvasDisposed();
+  const CanvasColorParams& ColorParams() const {
+    return surface_->ColorParams();
+  }
 
   base::WeakPtrFactory<ImageBuffer> weak_ptr_factory_;
 

third_party/WebKit/Source/platform/graphics/StaticBitmapImage.cpp

@@ -9,6 +9,8 @@
 #include "platform/graphics/ImageObserver.h"
 #include "platform/graphics/UnacceleratedStaticBitmapImage.h"
 #include "platform/graphics/paint/PaintImage.h"
+#include "platform/wtf/CheckedNumeric.h"
+#include "platform/wtf/typed_arrays/ArrayBufferContents.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkImage.h"
 #include "third_party/skia/include/core/SkPaint.h"
@@ -70,6 +72,62 @@ scoped_refptr<StaticBitmapImage> StaticBitmapImage::ConvertToColorSpace(
                                    ContextProviderWrapper());
 }
 
+bool StaticBitmapImage::ConvertToArrayBufferContents(
+    scoped_refptr<StaticBitmapImage> src_image,
+    WTF::ArrayBufferContents& dest_contents,
+    const IntRect& rect,
+    const CanvasColorParams& color_params,
+    bool is_accelerated) {
+  uint8_t bytes_per_pixel = color_params.BytesPerPixel();
+  CheckedNumeric<int> data_size = bytes_per_pixel;
+  data_size *= rect.Size().Area();
+  if (!data_size.IsValid() ||
+      data_size.ValueOrDie() > v8::TypedArray::kMaxLength)
+    return false;
+
+  size_t alloc_size_in_bytes = rect.Size().Area() * bytes_per_pixel;
+  if (!src_image) {
+    auto data = WTF::ArrayBufferContents::CreateDataHandle(
+        alloc_size_in_bytes, WTF::ArrayBufferContents::kZeroInitialize);
+    if (!data)
+      return false;
+    WTF::ArrayBufferContents result(std::move(data), alloc_size_in_bytes,
+                                    WTF::ArrayBufferContents::kNotShared);
+    result.Transfer(dest_contents);
+    return true;
+  }
+
+  const bool may_have_stray_area =
+      is_accelerated  // GPU readback may fail silently
+      || rect.X() < 0 || rect.Y() < 0 ||
+      rect.MaxX() > src_image->Size().Width() ||
+      rect.MaxY() > src_image->Size().Height();
+  WTF::ArrayBufferContents::InitializationPolicy initialization_policy =
+      may_have_stray_area ? WTF::ArrayBufferContents::kZeroInitialize
+                          : WTF::ArrayBufferContents::kDontInitialize;
+  auto data = WTF::ArrayBufferContents::CreateDataHandle(alloc_size_in_bytes,
+                                                         initialization_policy);
+  if (!data)
+    return false;
+  WTF::ArrayBufferContents result(std::move(data), alloc_size_in_bytes,
+                                  WTF::ArrayBufferContents::kNotShared);
+
+  SkColorType color_type =
+      (color_params.GetSkColorType() == kRGBA_F16_SkColorType)
+          ? kRGBA_F16_SkColorType
+          : kRGBA_8888_SkColorType;
+  SkImageInfo info = SkImageInfo::Make(rect.Width(), rect.Height(), color_type,
+                                       kUnpremul_SkAlphaType);
+  sk_sp<SkImage> sk_image = src_image->PaintImageForCurrentFrame().GetSkImage();
+  bool read_pixels_successful = sk_image->readPixels(
+      info, result.Data(), info.minRowBytes(), rect.X(), rect.Y());
+  DCHECK(read_pixels_successful ||
+         !sk_image->bounds().intersect(SkIRect::MakeXYWH(
+             rect.X(), rect.Y(), info.width(), info.height())));
+  result.Transfer(dest_contents);
+  return true;
+}
+
 gpu::SyncToken StaticBitmapImage::GetSyncToken() {
   return gpu::SyncToken();
 }

third_party/WebKit/Source/platform/graphics/StaticBitmapImage.h

@@ -8,11 +8,18 @@
 #include "base/memory/weak_ptr.h"
 #include "gpu/command_buffer/common/mailbox.h"
 #include "gpu/command_buffer/common/sync_token.h"
+#include "platform/graphics/CanvasColorParams.h"
 #include "platform/graphics/GraphicsTypes.h"
 #include "platform/graphics/Image.h"
 #include "third_party/khronos/GLES2/gl2.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 
+namespace WTF {
+
+class ArrayBufferContents;
+
+}  // namespace WTF
+
 namespace blink {
 
 class WebGraphicsContext3DProvider;
@@ -95,6 +102,13 @@ class PLATFORM_EXPORT StaticBitmapImage : public Image {
       sk_sp<SkColorSpace>,
       SkTransferFunctionBehavior);
 
+  static bool ConvertToArrayBufferContents(
+      scoped_refptr<StaticBitmapImage> src_image,
+      WTF::ArrayBufferContents& dest_contents,
+      const IntRect&,
+      const CanvasColorParams&,
+      bool is_accelerated = false);
+
  protected:
   // Helper for sub-classes
   void DrawHelper(PaintCanvas*,

third_party/WebKit/Source/platform/graphics/ImageBufferTest.cpp → third_party/WebKit/Source/platform/graphics/StaticBitmapImageTest.cpp

@@ -2,28 +2,34 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "platform/graphics/ImageBuffer.h"
+#include "platform/graphics/StaticBitmapImage.h"
 
 #include "platform/wtf/typed_arrays/ArrayBufferContents.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkSurface.h"
 #include "v8/include/v8.h"
 
 namespace blink {
 
-class ImageBufferTest : public ::testing::Test {};
+class StaticBitmapImageTest : public ::testing::Test {};
 
 // This test verifies if requesting a large ImageData that cannot be handled by
-// V8 is denied by ImageBuffer. This prevents V8 from crashing the renderer if
-// the user asks to get back the ImageData.
-TEST_F(ImageBufferTest, GetImageDataTooBigToAllocateDoesNotCrash) {
-  std::unique_ptr<ImageBuffer> image_buffer =
-      ImageBuffer::Create(IntSize(1, 1), kDoNotInitializeImagePixels);
-  EXPECT_TRUE(image_buffer->IsSurfaceValid());
+// V8 is denied by StaticBitmapImage. This prevents V8 from crashing the
+// renderer if the user asks to get back the ImageData.
+TEST_F(StaticBitmapImageTest,
+       ConvertArrayBufferContentsTooBigToAllocateDoesNotCrash) {
+  SkImageInfo info = SkImageInfo::MakeN32Premul(1, 1);
+  sk_sp<SkSurface> surface = SkSurface::MakeRaster(info);
+  EXPECT_TRUE(!!surface);
+
+  scoped_refptr<StaticBitmapImage> image =
+      StaticBitmapImage::Create(surface->makeImageSnapshot());
 
   IntRect too_big_rect(IntPoint(0, 0),
                        IntSize(1, (v8::TypedArray::kMaxLength / 4) + 1));
   WTF::ArrayBufferContents contents;
-  EXPECT_FALSE(image_buffer->GetImageData(too_big_rect, contents));
+  EXPECT_FALSE(StaticBitmapImage::ConvertToArrayBufferContents(
+      image, contents, too_big_rect, CanvasColorParams()));
 }
 
 }  // namespace blink