Fix check in GrGLGpu for whether PBO 0 is bound

Found in OOP-R canvas in Chrome. Most likely async read followed by
sync read with an intervening resetContext().

Bug: chromium:1208212

Change-Id: Ibf85f070d0a4cd389f67e9b249655b0ef667c66e
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/435277
Commit-Queue: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
Auto-Submit: Brian Salomon <bsalomon@google.com>
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>

src/gpu/gl/GrGLGpu.cpp

@@ -952,9 +952,10 @@ bool GrGLGpu::onTransferPixelsFrom(GrSurface* surface,
 void GrGLGpu::unbindXferBuffer(GrGpuBufferType type) {
     SkASSERT(type == GrGpuBufferType::kXferCpuToGpu || type == GrGpuBufferType::kXferGpuToCpu);
     auto* xferBufferState = this->hwBufferState(type);
-    if (!xferBufferState->fBoundBufferUniqueID.isInvalid()) {
+    if (!xferBufferState->fBufferZeroKnownBound) {
         GL_CALL(BindBuffer(xferBufferState->fGLTarget, 0));
-        xferBufferState->invalidate();
+        xferBufferState->fBoundBufferUniqueID.makeInvalid();
+        xferBufferState->fBufferZeroKnownBound = true;
     }
 }
 

tests/ReadWritePixelsGpuTest.cpp

@@ -1239,3 +1239,59 @@ DEF_GPUTEST_FOR_RENDERING_CONTEXTS(SurfaceContextWritePixelsMipped, reporter, ct
         }
     }
 }
+
+// Tests a bug found in OOP-R canvas2d in Chrome. The GPU backend would incorrectly not bind
+// buffer 0 to GL_PIXEL_PACK_BUFFER before a glReadPixels() that was supposed to read into
+// client memory if a GrDirectContext::resetContext() occurred.
+DEF_GPUTEST_FOR_GL_RENDERING_CONTEXTS(GLReadPixelsUnbindPBO, reporter, ctxInfo) {
+    // Start with a async read so that we bind to GL_PIXEL_PACK_BUFFER.
+    auto info = SkImageInfo::Make(16, 16, kRGBA_8888_SkColorType, kPremul_SkAlphaType);
+    SkAutoPixmapStorage pmap = make_ref_data(info, /*forceOpaque=*/false);
+    auto image = SkImage::MakeFromRaster(pmap, nullptr, nullptr);
+    image = image->makeTextureImage(ctxInfo.directContext());
+    if (!image) {
+        ERRORF(reporter, "Couldn't make texture image.");
+        return;
+    }
+
+    AsyncContext asyncContext;
+    image->asyncRescaleAndReadPixels(info,
+                                     SkIRect::MakeSize(info.dimensions()),
+                                     SkImage::RescaleGamma::kSrc,
+                                     SkImage::RescaleMode::kNearest,
+                                     async_callback,
+                                     &asyncContext);
+
+    // This will force the async readback to finish.
+    ctxInfo.directContext()->flushAndSubmit(true);
+    if (!asyncContext.fCalled) {
+        ERRORF(reporter, "async_callback not called.");
+    }
+    if (!asyncContext.fResult) {
+        ERRORF(reporter, "async read failed.");
+    }
+
+    SkPixmap asyncResult(info, asyncContext.fResult->data(0), asyncContext.fResult->rowBytes(0));
+
+    // Bug was that this would cause GrGLGpu to think no buffer was left bound to
+    // GL_PIXEL_PACK_BUFFER even though async transfer did leave one bound. So the sync read
+    // wouldn't bind buffer 0.
+    ctxInfo.directContext()->resetContext();
+
+    SkBitmap syncResult;
+    syncResult.allocPixels(info);
+    syncResult.eraseARGB(0xFF, 0xFF, 0xFF, 0xFF);
+
+    image->readPixels(ctxInfo.directContext(), syncResult.pixmap(), 0, 0);
+
+    float tol[4] = {};  // expect exactly same pixels, no conversions.
+    auto error = std::function<ComparePixmapsErrorReporter>([&](int x, int y,
+                                                                const float diffs[4]) {
+      SkASSERT(x >= 0 && y >= 0);
+      ERRORF(reporter, "Expect sync and async read to be the same. "
+             "Error at %d, %d. Diff in floats: (%f, %f, %f, %f)",
+             x, y, diffs[0], diffs[1], diffs[2], diffs[3]);
+    });
+
+    ComparePixels(syncResult.pixmap(), asyncResult, tol, error);
+}