Bind framebuffer to fbo_ object before readPixels, rather than multisample_fbo_

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "platform/graphics/gpu/DrawingBuffer.h"
 
 #include "cc/resources/single_release_callback.h"
 #include "cc/resources/texture_mailbox.h"
 #include "gpu/command_buffer/client/gles2_interface_stub.h"
 #include "platform/graphics/gpu/DrawingBufferTestHelpers.h"
@@ skipping 31 matching lines @@
  private:
   std::unique_ptr<gpu::gles2::GLES2Interface> gl_;
 };
 
 class DrawingBufferSoftwareRenderingTest : public Test {
  protected:
   void SetUp() override {
     IntSize initial_size(kInitialWidth, kInitialHeight);
     std::unique_ptr<GLES2InterfaceForTests> gl =
         WTF::WrapUnique(new GLES2InterfaceForTests);
+    gl_ = gl.get();

[Ken Russell (switch to Gerrit), 2017/06/01 21:37:43]

Agree with Mo's point that caching this weak pointer could lead to fragile code.
I do see however that several of the other tests are doing it this way.

Could you instead add to DrawingBufferForTests:

  GLES2InterfaceForTests* ContextGLForTests() {
    return static_cast<GLES2InterfaceForTests*>(drawing_buffer_->ContextGL());
  }

This assumes that all DrawingBufferForTests instances are set up with a
GLES2InterfaceForTests. I think that's a safe assumption. Then call this method
where the GLES2InterfaceForTests is needed. It would be nice to clean up the
other tests which are doing this caching as well.

[xinghua.cao, 2017/06/02 16:06:28]

Hi, ken, Do you mean that add ContextGLForTests() function in
DrawingBufferForTests class? But DrawingBufferForTests does not have the
drawing_buffer_ member, I think here only need to call "ContextGL()" directly,
is that right?

[Ken Russell (switch to Gerrit), 2017/06/02 21:54:14]

Ah I misunderstood. Anyway, you fixed this in your most recent patch set --
thanks.

     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;
 };
 
-TEST_F(DrawingBufferSoftwareRenderingTest, bitmapRecycling) {
+TEST_F(DrawingBufferSoftwareRenderingTest, BitmapRecycling) {
   cc::TextureMailbox texture_mailbox;
   std::unique_ptr<cc::SingleReleaseCallback> release_callback1;
   std::unique_ptr<cc::SingleReleaseCallback> release_callback2;
   std::unique_ptr<cc::SingleReleaseCallback> release_callback3;
   IntSize initial_size(kInitialWidth, kInitialHeight);
   IntSize alternate_size(kInitialWidth, kAlternateHeight);
 
   drawing_buffer_->Resize(initial_size);
   drawing_buffer_->MarkContentsChanged();
   drawing_buffer_->PrepareTextureMailbox(
@@ skipping 17 matching lines @@
   drawing_buffer_->PrepareTextureMailbox(
       &texture_mailbox,
       &release_callback3);  // cause recycling queue to be purged due to resize
   EXPECT_EQ(0, drawing_buffer_->RecycledBitmapCount());
   release_callback3->Run(gpu::SyncToken(), false /* lostResource */);
   EXPECT_EQ(1, drawing_buffer_->RecycledBitmapCount());
 
   drawing_buffer_->BeginDestruction();
 }
 
+TEST_F(DrawingBufferSoftwareRenderingTest, FramebufferBinding) {
+  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