Simplify / fix SkBitmap::ReadRawPixels()

We no longer need to look at the field snugRB except to check for the
simple no-pixels case.  This is good, because our snugRB <= ramRB check
is actually too weak, and is the source of this linked Chromium issue.

BUG=chromium:765858

Instead of doing complicated checks against that stored snugRB and the
computed ramRB, we now just ignore snugRB.  We know the images written
by write_row_bytes() will be snug, so we can just look at width, height,
and color type to figure out exactly how many bytes we should be
reading.

Then it becomes the call to readByteArray()'s responsibility to make
sure that we have an array there of exactly that many bytes to read.
We've just got to make sure we check for its failure.

Change-Id: Ia05c36d8a77b0de16ee03a80f6cb2dab6fcedbae
Reviewed-on: https://skia-review.googlesource.com/50800
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>

1 files changed, 9 insertions, 22 deletions

diff --git a/src/core/SkBitmap.cpp b/src/core/SkBitmap.cpp
index 812dbb6182..c3bc4a9d63 100644
--- a/src/core/SkBitmap.cpp
+++ b/src/core/SkBitmap.cpp
@@ -656,9 +656,8 @@ void SkBitmap::WriteRawPixels(SkWriteBuffer* buffer, const SkBitmap& bitmap) {
 }
 
 bool SkBitmap::ReadRawPixels(SkReadBuffer* buffer, SkBitmap* bitmap) {
-    const size_t snugRB = buffer->readUInt();
-    if (0 == snugRB) {  // no pixels
-        return false;
+    if (0 == buffer->readUInt()) {
+        return false;  // no pixels
     }
 
     SkImageInfo info;
@@ -669,34 +668,22 @@ bool SkBitmap::ReadRawPixels(SkReadBuffer* buffer, SkBitmap* bitmap) {
     }
 
     // If there was an error reading "info" or if it is bogus,
-    // don't use it to compute minRowBytes()
+    // don't use it to compute minRowBytes().
     if (!buffer->validate(SkColorTypeValidateAlphaType(info.colorType(),
                                                        info.alphaType()))) {
         return false;
     }
 
-    const size_t ramRB = info.minRowBytes();
-    const int height = SkMax32(info.height(), 0);
-    const uint64_t snugSize = sk_64_mul(snugRB, height);
-    const uint64_t ramSize = sk_64_mul(ramRB, height);
-    static const uint64_t max_size_t = (size_t)(-1);
-    if (!buffer->validate((snugSize <= ramSize) && (ramSize <= max_size_t))) {
+    // write_raw_pixels() always writes snug buffers with rowBytes == minRowBytes().
+    size_t bytes = info.getSafeSize(info.minRowBytes());
+    if (!buffer->validate(bytes != 0)) {
         return false;
     }
 
-    sk_sp<SkData> data(SkData::MakeUninitialized(SkToSizeT(ramSize)));
+    sk_sp<SkData> data(SkData::MakeUninitialized(bytes));
     unsigned char* dst = (unsigned char*)data->writable_data();
-    buffer->readByteArray(dst, SkToSizeT(snugSize));
-
-    if (snugSize != ramSize) {
-        const unsigned char* srcRow = dst + snugRB * (height - 1);
-        unsigned char* dstRow = dst + ramRB * (height - 1);
-        for (int y = height - 1; y >= 1; --y) {
-            memmove(dstRow, srcRow, snugRB);
-            srcRow -= snugRB;
-            dstRow -= ramRB;
-        }
-        SkASSERT(srcRow == dstRow); // first row does not need to be moved
+    if (!buffer->readByteArray(dst, bytes)) {
+        return false;
     }
 
     if (buffer->readBool()) {