Bind framebuffer to fbo_ object before readPixels, rather than multisample_fbo_

third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferSoftwareRenderingTest.cpp

Index: third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferSoftwareRenderingTest.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferSoftwareRenderingTest.cpp b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferSoftwareRenderingTest.cpp
index edcd8be52c9820136312cdf408d0288e0b8e36af..84752e4642e366069b9e29bf32c67a123ca5ceb3 100644
--- a/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferSoftwareRenderingTest.cpp
+++ b/third_party/WebKit/Source/platform/graphics/gpu/DrawingBufferSoftwareRenderingTest.cpp
@@ -49,17 +49,19 @@ class DrawingBufferSoftwareRenderingTest : public Test {
     IntSize initial_size(kInitialWidth, kInitialHeight);
     std::unique_ptr<GLES2InterfaceForTests> gl =
         WTF::WrapUnique(new GLES2InterfaceForTests);
+    gl_ = gl.get();

[Zhenyao Mo, 2017/05/26 22:38:34]

I don't think you should cache it here. Instead, just use
WebGraphicsContext3DProviderSoftwareRenderingForTests::ContextGL() for access.

[xinghua.cao, 2017/06/01 02:26:51]

Line 58, After "std::move(provider)", it seems that provider is already null, so
the next parameter cannot use "provider->ContextGL()", is that right?

     std::unique_ptr<WebGraphicsContext3DProviderSoftwareRenderingForTests>
         provider = WTF::WrapUnique(
             new WebGraphicsContext3DProviderSoftwareRenderingForTests(
                 std::move(gl)));
     drawing_buffer_ = DrawingBufferForTests::Create(
-        std::move(provider), nullptr, initial_size, DrawingBuffer::kPreserve,
+        std::move(provider), gl_, initial_size, DrawingBuffer::kPreserve,
         kDisableMultisampling);
     CHECK(drawing_buffer_);
   }
 
   RefPtr<DrawingBufferForTests> drawing_buffer_;
+  GLES2InterfaceForTests* gl_;
   bool is_software_rendering_ = false;
 };
 
@@ -101,5 +103,28 @@ TEST_F(DrawingBufferSoftwareRenderingTest, bitmapRecycling) {
   drawing_buffer_->BeginDestruction();
 }
 
+TEST_F(DrawingBufferSoftwareRenderingTest, framebufferBinding) {

[Zhenyao Mo, 2017/05/26 22:38:34]

I think you can just merge this test with the above one. 

[xinghua.cao, 2017/06/01 02:26:51]

Hi, Zhenyao, I think there are little relationship between these two tests, if
merge them into one, I have no idea how to name this test.

+  cc::TextureMailbox texture_mailbox;
+  std::unique_ptr<cc::SingleReleaseCallback> release_callback;
+  IntSize initial_size(kInitialWidth, kInitialHeight);
+  GLint drawBinding = 0, readBinding = 0;
+
+  GLuint draw_framebuffer_binding = 0xbeef3;
+  GLuint read_framebuffer_binding = 0xbeef4;
+  gl_->BindFramebuffer(GL_DRAW_FRAMEBUFFER, draw_framebuffer_binding);
+  gl_->BindFramebuffer(GL_READ_FRAMEBUFFER, read_framebuffer_binding);
+  gl_->SaveState();
+  drawing_buffer_->Resize(initial_size);
+  drawing_buffer_->MarkContentsChanged();
+  drawing_buffer_->PrepareTextureMailbox(&texture_mailbox, &release_callback);
+  gl_->GetIntegerv(GL_DRAW_FRAMEBUFFER_BINDING, &drawBinding);
+  gl_->GetIntegerv(GL_READ_FRAMEBUFFER_BINDING, &readBinding);
+  EXPECT_EQ(static_cast<GLint>(draw_framebuffer_binding), drawBinding);
+  EXPECT_EQ(static_cast<GLint>(read_framebuffer_binding), readBinding);
+  release_callback->Run(gpu::SyncToken(), false /* lostResource */);
+
+  drawing_buffer_->BeginDestruction();
+}
+
 }  // unnamed namespace
 }  // blink