Upgrade blink side ReadPixels size validation to consider ES3 pack parameters.

Note that this CL didn't do the upgrade for unpack, although the
foundation is there.

BUG=563714
TEST=conformance2/reading/read-pixels-pack-parameters.html
R=kbr@chromium.org,bajones@chromium.org

Review URL: https://codereview.chromium.org/1566283003

Cr-Commit-Position: refs/heads/master@{#368477}

6 files changed, 163 insertions, 41 deletions

diff --git a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
index 9478f11..dc6b275 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
+++ b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp
@@ -3443,4 +3443,26 @@ const WebGLSamplerState* WebGL2RenderingContextBase::getTextureUnitSamplerState(
     return WebGLRenderingContextBase::getTextureUnitSamplerState(target, unit);
 }
 
+WebGLImageConversion::PixelStoreParams WebGL2RenderingContextBase::getPackPixelStoreParams()
+{
+    WebGLImageConversion::PixelStoreParams params;
+    params.alignment = m_packAlignment;
+    params.rowLength = m_packRowLength;
+    params.skipPixels = m_packSkipPixels;
+    params.skipRows = m_packSkipRows;
+    return params;
+}
+
+WebGLImageConversion::PixelStoreParams WebGL2RenderingContextBase::getUnpackPixelStoreParams()
+{
+    WebGLImageConversion::PixelStoreParams params;
+    params.alignment = m_unpackAlignment;
+    params.rowLength = m_unpackRowLength;
+    params.imageHeight = m_unpackImageHeight;
+    params.skipPixels = m_unpackSkipPixels;
+    params.skipRows = m_unpackSkipRows;
+    params.skipImages = m_unpackSkipImages;
+    return params;
+}
+
 } // namespace blink
diff --git a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h
index f71f068..e083e50 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h
+++ b/third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.h
@@ -212,6 +212,9 @@ protected:
     bool validateBufferBaseTarget(const char* functionName, GLenum target);
     bool validateAndUpdateBufferBindBaseTarget(const char* functionName, GLenum, GLuint, WebGLBuffer*);
 
+    WebGLImageConversion::PixelStoreParams getPackPixelStoreParams() override;
+    WebGLImageConversion::PixelStoreParams getUnpackPixelStoreParams() override;
+
     bool checkAndTranslateAttachments(const char* functionName, GLenum, const Vector<GLenum>&, Vector<GLenum>&);
 
     /* WebGLRenderingContextBase overrides */
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
index 71ac672..324450b 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
+++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
@@ -3806,6 +3806,20 @@ DOMArrayBufferView::ViewType WebGLRenderingContextBase::readPixelsExpectedArrayB
     }
 }
 
+WebGLImageConversion::PixelStoreParams WebGLRenderingContextBase::getPackPixelStoreParams()
+{
+    WebGLImageConversion::PixelStoreParams params;
+    params.alignment = m_packAlignment;
+    return params;
+}
+
+WebGLImageConversion::PixelStoreParams WebGLRenderingContextBase::getUnpackPixelStoreParams()
+{
+    WebGLImageConversion::PixelStoreParams params;
+    params.alignment = m_unpackAlignment;
+    return params;
+}
+
 bool WebGLRenderingContextBase::validateReadPixelsFuncParameters(GLsizei width, GLsizei height, GLenum format, GLenum type, long long bufferSize)
 {
     if (!validateReadPixelsFormatAndType(format, type))
@@ -3817,15 +3831,14 @@ bool WebGLRenderingContextBase::validateReadPixelsFuncParameters(GLsizei width,
     if (!validateReadPixelsFormatTypeCombination(format, type, readBufferInternalFormat, readBufferType))
         return false;
 
-    // Calculate array size, taking into consideration of PACK_ALIGNMENT.
-    unsigned totalBytesRequired = 0;
-    unsigned padding = 0;
-    GLenum error = WebGLImageConversion::computeImageSizeInBytes(format, type, width, height, 1, m_packAlignment, &totalBytesRequired, &padding);
+    // Calculate array size, taking into consideration of pack parameters.
+    unsigned totalBytesRequired = 0, totalSkipBytes = 0;
+    GLenum error = WebGLImageConversion::computeImageSizeInBytes(format, type, width, height, 1, getPackPixelStoreParams(), &totalBytesRequired, 0, &totalSkipBytes);
     if (error != GL_NO_ERROR) {
         synthesizeGLError(error, "readPixels", "invalid dimensions");
         return false;
     }
-    if (bufferSize < static_cast<long long>(totalBytesRequired)) {
+    if (bufferSize < static_cast<long long>(totalBytesRequired + totalSkipBytes)) {
         synthesizeGLError(GL_INVALID_OPERATION, "readPixels", "buffer is not large enough for dimensions");
         return false;
     }
@@ -5862,19 +5875,12 @@ bool WebGLRenderingContextBase::validateTexFuncData(const char* functionName, GL
     }
 
     unsigned totalBytesRequired;
-    GLenum error = WebGLImageConversion::computeImageSizeInBytes(format, type, width, height, depth, m_unpackAlignment, &totalBytesRequired, 0);
+    GLenum error = WebGLImageConversion::computeImageSizeInBytes(format, type, width, height, depth, getUnpackPixelStoreParams(), &totalBytesRequired, 0, 0);
     if (error != GL_NO_ERROR) {
         synthesizeGLError(error, functionName, "invalid texture dimensions");
         return false;
     }
     if (pixels->byteLength() < totalBytesRequired) {
-        if (m_unpackAlignment != 1) {
-            error = WebGLImageConversion::computeImageSizeInBytes(format, type, width, height, depth, 1, &totalBytesRequired, 0);
-            if (pixels->byteLength() == totalBytesRequired) {
-                synthesizeGLError(GL_INVALID_OPERATION, functionName, "ArrayBufferView not big enough for request with UNPACK_ALIGNMENT > 1");
-                return false;
-            }
-        }
         synthesizeGLError(GL_INVALID_OPERATION, functionName, "ArrayBufferView not big enough for request");
         return false;
     }
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
index 4ce5acc..a15a0c4 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
+++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.h
@@ -832,6 +832,9 @@ protected:
     void handleTextureCompleteness(const char*, bool);
     void createFallbackBlackTextures1x1();
 
+    virtual WebGLImageConversion::PixelStoreParams getPackPixelStoreParams();
+    virtual WebGLImageConversion::PixelStoreParams getUnpackPixelStoreParams();
+
     // Helper function for copyTex{Sub}Image, check whether the internalformat
     // and the color buffer format of the current bound framebuffer combination
     // is valid.
diff --git a/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp b/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp
index 94ed963..38849d9 100644
--- a/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp
+++ b/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.cpp
@@ -1923,8 +1923,26 @@ void FormatConverter::convert()
     return;
 }
 
+bool frameIsValid(const SkBitmap& frameBitmap)
+{
+    return !frameBitmap.isNull()
+        && !frameBitmap.empty()
+        && frameBitmap.isImmutable()
+        && frameBitmap.colorType() == kN32_SkColorType;
+}
+
 } // anonymous namespace
 
+WebGLImageConversion::PixelStoreParams::PixelStoreParams()
+    : alignment(4)
+    , rowLength(0)
+    , imageHeight(0)
+    , skipPixels(0)
+    , skipRows(0)
+    , skipImages(0)
+{
+}
+
 bool WebGLImageConversion::computeFormatAndTypeParameters(GLenum format, GLenum type, unsigned* componentsPerPixel, unsigned* bytesPerComponent)
 {
     switch (format) {
@@ -2000,40 +2018,102 @@ bool WebGLImageConversion::computeFormatAndTypeParameters(GLenum format, GLenum
     return true;
 }
 
-GLenum WebGLImageConversion::computeImageSizeInBytes(GLenum format, GLenum type, GLsizei width, GLsizei height, GLsizei depth, GLint alignment, unsigned* imageSizeInBytes, unsigned* paddingInBytes)
+GLenum WebGLImageConversion::computeImageSizeInBytes(GLenum format, GLenum type, GLsizei width, GLsizei height, GLsizei depth, const PixelStoreParams& params, unsigned* imageSizeInBytes, unsigned* paddingInBytes, unsigned* skipSizeInBytes)
 {
     ASSERT(imageSizeInBytes);
-    ASSERT(alignment == 1 || alignment == 2 || alignment == 4 || alignment == 8);
+    ASSERT(params.alignment == 1 || params.alignment == 2 || params.alignment == 4 || params.alignment == 8);
+    ASSERT(params.rowLength >= 0 && params.imageHeight >= 0);
+    ASSERT(params.skipPixels >= 0 && params.skipRows >= 0 && params.skipImages >= 0);
     if (width < 0 || height < 0 || depth < 0)
         return GL_INVALID_VALUE;
-    unsigned bytesPerComponent, componentsPerPixel;
-    if (!computeFormatAndTypeParameters(format, type, &bytesPerComponent, &componentsPerPixel))
-        return GL_INVALID_ENUM;
     if (!width || !height || !depth) {
         *imageSizeInBytes = 0;
         if (paddingInBytes)
             *paddingInBytes = 0;
+        if (skipSizeInBytes)
+            *skipSizeInBytes = 0;
         return GL_NO_ERROR;
     }
-    CheckedInt<uint32_t> checkedValue(bytesPerComponent * componentsPerPixel);
-    checkedValue *=  width;
+
+    int rowLength = params.rowLength > 0 ? params.rowLength : width;
+    int imageHeight = params.imageHeight > 0 ? params.imageHeight : height;
+
+    unsigned bytesPerComponent, componentsPerPixel;
+    if (!computeFormatAndTypeParameters(format, type, &bytesPerComponent, &componentsPerPixel))
+        return GL_INVALID_ENUM;
+    unsigned bytesPerGroup = bytesPerComponent * componentsPerPixel;
+    CheckedInt<uint32_t> checkedValue = static_cast<uint32_t>(rowLength);
+    checkedValue *=  bytesPerGroup;
     if (!checkedValue.isValid())
         return GL_INVALID_VALUE;
-    unsigned validRowSize = checkedValue.value();
+
+    unsigned lastRowSize;
+    if (params.rowLength > 0 && params.rowLength != width) {
+        CheckedInt<uint32_t> tmp = width;
+        tmp *= bytesPerGroup;
+        if (!tmp.isValid())
+            return GL_INVALID_VALUE;
+        lastRowSize = tmp.value();
+    } else {
+        lastRowSize = checkedValue.value();
+    }
+
     unsigned padding = 0;
-    unsigned residual = validRowSize % alignment;
+    unsigned residual = checkedValue.value() % params.alignment;
     if (residual) {
-        padding = alignment - residual;
+        padding = params.alignment - residual;
         checkedValue += padding;
     }
-    // Last row needs no padding.
-    checkedValue *= (height * depth - 1);
-    checkedValue += validRowSize;
+    if (!checkedValue.isValid())
+        return GL_INVALID_VALUE;
+    unsigned paddedRowSize = checkedValue.value();
+
+    CheckedInt<uint32_t> rows = imageHeight;
+    rows *= (depth - 1);
+    // Last image is not affected by IMAGE_HEIGHT parameter.
+    rows += height;
+    if (!rows.isValid())
+        return GL_INVALID_VALUE;
+    checkedValue *= (rows.value() - 1);
+    // Last row is not affected by ROW_LENGTH parameter.
+    checkedValue += lastRowSize;
     if (!checkedValue.isValid())
         return GL_INVALID_VALUE;
     *imageSizeInBytes = checkedValue.value();
     if (paddingInBytes)
         *paddingInBytes = padding;
+
+    CheckedInt<uint32_t> skipSize = 0;
+    if (params.skipImages > 0) {
+        CheckedInt<uint32_t> tmp = paddedRowSize;
+        tmp *= imageHeight;
+        tmp *= params.skipImages;
+        if (!tmp.isValid())
+            return GL_INVALID_VALUE;
+        skipSize += tmp.value();
+    }
+    if (params.skipRows > 0) {
+        CheckedInt<uint32_t> tmp = paddedRowSize;
+        tmp *= params.skipRows;
+        if (!tmp.isValid())
+            return GL_INVALID_VALUE;
+        skipSize += tmp.value();
+    }
+    if (params.skipPixels > 0) {
+        CheckedInt<uint32_t> tmp = bytesPerGroup;
+        tmp *= params.skipPixels;
+        if (!tmp.isValid())
+            return GL_INVALID_VALUE;
+        skipSize += tmp.value();
+    }
+    if (!skipSize.isValid())
+        return GL_INVALID_VALUE;
+    if (skipSizeInBytes)
+        *skipSizeInBytes = skipSize.value();
+
+    checkedValue += skipSize.value();
+    if (!checkedValue.isValid())
+        return GL_INVALID_VALUE;
     return GL_NO_ERROR;
 }
 
@@ -2044,18 +2124,6 @@ WebGLImageConversion::ImageExtractor::ImageExtractor(Image* image, ImageHtmlDomS
     extractImage(premultiplyAlpha, ignoreGammaAndColorProfile);
 }
 
-namespace {
-
-bool frameIsValid(const SkBitmap& frameBitmap)
-{
-    return !frameBitmap.isNull()
-        && !frameBitmap.empty()
-        && frameBitmap.isImmutable()
-        && frameBitmap.colorType() == kN32_SkColorType;
-}
-
-} // anonymous namespace
-
 void WebGLImageConversion::ImageExtractor::extractImage(bool premultiplyAlpha, bool ignoreGammaAndColorProfile)
 {
     ASSERT(!m_imagePixelLocker);
@@ -2230,7 +2298,9 @@ bool WebGLImageConversion::packImageData(
 
     unsigned packedSize;
     // Output data is tightly packed (alignment == 1).
-    if (computeImageSizeInBytes(format, type, width, height, 1, 1, &packedSize, 0) != GL_NO_ERROR)
+    PixelStoreParams params;
+    params.alignment = 1;
+    if (computeImageSizeInBytes(format, type, width, height, 1, params, &packedSize, 0, 0) != GL_NO_ERROR)
         return false;
     data.resize(packedSize);
 
@@ -2257,7 +2327,9 @@ bool WebGLImageConversion::extractImageData(
 
     unsigned packedSize;
     // Output data is tightly packed (alignment == 1).
-    if (computeImageSizeInBytes(format, type, width, height, 1, 1, &packedSize, 0) != GL_NO_ERROR)
+    PixelStoreParams params;
+    params.alignment = 1;
+    if (computeImageSizeInBytes(format, type, width, height, 1, params, &packedSize, 0, 0) != GL_NO_ERROR)
         return false;
     data.resize(packedSize);
 
diff --git a/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.h b/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.h
index fddb53a..5f17e62 100644
--- a/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.h
+++ b/third_party/WebKit/Source/platform/graphics/gpu/WebGLImageConversion.h
@@ -115,6 +115,17 @@ public:
         HtmlDomNone = 3
     };
 
+    struct PLATFORM_EXPORT PixelStoreParams final {
+        PixelStoreParams();
+
+        GLint alignment;
+        GLint rowLength;
+        GLint imageHeight;
+        GLint skipPixels;
+        GLint skipRows;
+        GLint skipImages;
+    };
+
     class PLATFORM_EXPORT ImageExtractor final {
         STACK_ALLOCATED();
     public:
@@ -152,7 +163,12 @@ public:
     // return the suggested GL error indicating the cause of the failure:
     //   INVALID_VALUE if width/height/depth is negative or overflow happens.
     //   INVALID_ENUM if format/type is illegal.
-    static GLenum computeImageSizeInBytes(GLenum format, GLenum type, GLsizei width, GLsizei height, GLsizei depth, GLint alignment, unsigned* imageSizeInBytes, unsigned* paddingInBytes);
+    // Note that imageSizeBytes does not include skipSizeInBytes, but it is
+    // guaranteed if NO_ERROR is returned, adding the two sizes won't cause
+    // overflow.
+    // |paddingInBytes| and |skipSizeInBytes| are optional and can be null, but
+    // the overflow validation is still performed.
+    static GLenum computeImageSizeInBytes(GLenum format, GLenum type, GLsizei width, GLsizei height, GLsizei depth, const PixelStoreParams&, unsigned* imageSizeInBytes, unsigned* paddingInBytes, unsigned* skipSizeInBytes);
 
     // Check if the format is one of the formats from the ImageData or DOM elements.
     // The formats from ImageData is always RGBA8.