Fix crash caused by concurrent access to framebuffer_combo_complete_map_.

gpu/command_buffer/service/framebuffer_manager_unittest.cc

 // Copyright (c) 2012 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 "gpu/command_buffer/service/error_state_mock.h"
 #include "gpu/command_buffer/service/framebuffer_manager.h"
 #include "gpu/command_buffer/service/feature_info.h"
 #include "gpu/command_buffer/service/gpu_service_test.h"
 #include "gpu/command_buffer/service/renderbuffer_manager.h"
 #include "gpu/command_buffer/service/test_helper.h"
@@ skipping 16 matching lines @@
 const GLint kMaxSamples = 4;
 const uint32 kMaxDrawBuffers = 16;
 const uint32 kMaxColorAttachments = 16;
 const bool kUseDefaultTextures = false;
 
 }  // namespace
 
 class FramebufferManagerTest : public GpuServiceTest {
  public:
   FramebufferManagerTest()
-      : manager_(1, 1, ContextGroup::CONTEXT_TYPE_UNDEFINED),
+      : manager_(1, 1, ContextGroup::CONTEXT_TYPE_UNDEFINED, NULL),
         feature_info_(new FeatureInfo()) {
     texture_manager_.reset(new TextureManager(NULL,
                                               feature_info_.get(),
                                               kMaxTextureSize,
                                               kMaxCubemapSize,
                                               kMaxRectangleTextureSize,
                                               kMax3DTextureSize,
                                               kUseDefaultTextures));
     renderbuffer_manager_.reset(new RenderbufferManager(NULL,
                                                         kMaxRenderbufferSize,
@@ skipping 56 matching lines @@
   framebuffer1 = manager_.GetFramebuffer(kClient1Id);
   ASSERT_TRUE(framebuffer1 == NULL);
 }
 
 class FramebufferInfoTestBase : public GpuServiceTest {
  public:
   static const GLuint kClient1Id = 1;
   static const GLuint kService1Id = 11;
 
   explicit FramebufferInfoTestBase(ContextGroup::ContextType context_type)
-      : manager_(kMaxDrawBuffers,  kMaxColorAttachments, context_type),
+      : manager_(kMaxDrawBuffers, kMaxColorAttachments, context_type, NULL),
         feature_info_(new FeatureInfo()) {
+    manager_.SetFramebufferComboCompleteCacheForTesting(
+        new FramebufferCompletenessCache);
     texture_manager_.reset(new TextureManager(NULL,
                                               feature_info_.get(),
                                               kMaxTextureSize,
                                               kMaxCubemapSize,
                                               kMaxRectangleTextureSize,
                                               kMax3DTextureSize,
                                               kUseDefaultTextures));
     renderbuffer_manager_.reset(new RenderbufferManager(NULL,
                                                         kMaxRenderbufferSize,
                                                         kMaxSamples,
@@ skipping 702 matching lines @@
       texture_manager_->GetTexture(kTextureClient2Id));
   ASSERT_TRUE(texture2.get() != NULL);
   texture_manager_->SetTarget(texture2.get(), GL_TEXTURE_2D);
 
   EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(GL_FRAMEBUFFER))
       .WillOnce(Return(GL_FRAMEBUFFER_COMPLETE))
       .RetiresOnSaturation();
   framebuffer_->GetStatus(texture_manager_.get(), GL_FRAMEBUFFER);
 
   // Check a second call for the same type does not call anything
-  if (!framebuffer_->AllowFramebufferComboCompleteMapForTesting()) {
-    EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(GL_FRAMEBUFFER))

[Ken Russell (switch to Gerrit), 2015/08/20 00:04:10]

Why are these CheckFramebufferStatusEXT expectations removed? It looks like it's
still being called from Framebuffer::GetStatus.

[no sievers, 2015/08/20 00:11:29]

Actually if you are setting the expectation than you are testing that the cache
*does* not work and calls into the driver.

It was done because of the Mac ifdef. However, we don't need to do this for the
tests - they can work with the cache on Mac - and we can just disable the cache
for Mac in real production builds only.

[Tobias Sargeant, 2015/08/20 11:05:13]

Acknowledged.

[Tobias Sargeant, 2015/08/20 11:05:13]

Acknowledged.

-        .WillOnce(Return(GL_FRAMEBUFFER_COMPLETE))
-        .RetiresOnSaturation();
-  }
   framebuffer_->GetStatus(texture_manager_.get(), GL_FRAMEBUFFER);
 
   // Check changing the attachments calls CheckFramebufferStatus.
   framebuffer_->AttachTexture(
       GL_COLOR_ATTACHMENT0, texture2.get(), kTarget1, kLevel1, kSamples1);
   EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(GL_FRAMEBUFFER))
       .WillOnce(Return(GL_FRAMEBUFFER_COMPLETE)).RetiresOnSaturation();
   framebuffer_->GetStatus(texture_manager_.get(), GL_FRAMEBUFFER);
 
   // Check a second call for the same type does not call anything.
-  if (!framebuffer_->AllowFramebufferComboCompleteMapForTesting()) {
-    EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(GL_FRAMEBUFFER))
-        .WillOnce(Return(GL_FRAMEBUFFER_COMPLETE))
-        .RetiresOnSaturation();
-  }
   framebuffer_->GetStatus(texture_manager_.get(), GL_FRAMEBUFFER);
 
   // Check a second call with a different target calls CheckFramebufferStatus.
   EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(GL_READ_FRAMEBUFFER))
       .WillOnce(Return(GL_FRAMEBUFFER_COMPLETE))
       .RetiresOnSaturation();
   framebuffer_->GetStatus(texture_manager_.get(), GL_READ_FRAMEBUFFER);
 
   // Check a second call for the same type does not call anything.
-  if (!framebuffer_->AllowFramebufferComboCompleteMapForTesting()) {
-    EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(GL_READ_FRAMEBUFFER))
-        .WillOnce(Return(GL_FRAMEBUFFER_COMPLETE))
-        .RetiresOnSaturation();
-  }
   framebuffer_->GetStatus(texture_manager_.get(), GL_READ_FRAMEBUFFER);
 
   // Check adding another attachment calls CheckFramebufferStatus.
   framebuffer_->AttachRenderbuffer(GL_DEPTH_ATTACHMENT, renderbuffer1);
   EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(GL_READ_FRAMEBUFFER))
       .WillOnce(Return(GL_FRAMEBUFFER_COMPLETE))
       .RetiresOnSaturation();
   framebuffer_->GetStatus(texture_manager_.get(), GL_READ_FRAMEBUFFER);
 
   // Check a second call for the same type does not call anything.
-  if (!framebuffer_->AllowFramebufferComboCompleteMapForTesting()) {
-    EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(GL_READ_FRAMEBUFFER))
-        .WillOnce(Return(GL_FRAMEBUFFER_COMPLETE))
-        .RetiresOnSaturation();
-  }
   framebuffer_->GetStatus(texture_manager_.get(), GL_READ_FRAMEBUFFER);
 
   // Check changing the format calls CheckFramebuffferStatus.
   TestHelper::SetTexParameteriWithExpectations(gl_.get(),
                                                error_state_.get(),
                                                texture_manager_.get(),
                                                texture2.get(),
                                                GL_TEXTURE_WRAP_S,
                                                GL_CLAMP_TO_EDGE,
                                                GL_NO_ERROR);
 
   EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(GL_READ_FRAMEBUFFER))
       .WillOnce(Return(GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT))
       .WillOnce(Return(GL_FRAMEBUFFER_COMPLETE))
       .RetiresOnSaturation();
   framebuffer_->GetStatus(texture_manager_.get(), GL_READ_FRAMEBUFFER);
 
   // Check since it did not return FRAMEBUFFER_COMPLETE that it calls
   // CheckFramebufferStatus
   framebuffer_->GetStatus(texture_manager_.get(), GL_READ_FRAMEBUFFER);
 
   // Check putting it back does not call CheckFramebufferStatus.
-  if (!framebuffer_->AllowFramebufferComboCompleteMapForTesting()) {
-    EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(GL_READ_FRAMEBUFFER))
-        .WillOnce(Return(GL_FRAMEBUFFER_COMPLETE))
-        .RetiresOnSaturation();
-  }
   TestHelper::SetTexParameteriWithExpectations(gl_.get(),
                                                error_state_.get(),
                                                texture_manager_.get(),
                                                texture2.get(),
                                                GL_TEXTURE_WRAP_S,
                                                GL_REPEAT,
                                                GL_NO_ERROR);
   framebuffer_->GetStatus(texture_manager_.get(), GL_READ_FRAMEBUFFER);
 
   // Check Unbinding does not call CheckFramebufferStatus
   framebuffer_->UnbindRenderbuffer(GL_RENDERBUFFER, renderbuffer1);
-  if (!framebuffer_->AllowFramebufferComboCompleteMapForTesting()) {
-    EXPECT_CALL(*gl_, CheckFramebufferStatusEXT(GL_READ_FRAMEBUFFER))
-        .WillOnce(Return(GL_FRAMEBUFFER_COMPLETE))
-        .RetiresOnSaturation();
-  }
   framebuffer_->GetStatus(texture_manager_.get(), GL_READ_FRAMEBUFFER);
 }
 
 class FramebufferInfoES3Test : public FramebufferInfoTestBase {
  public:
   FramebufferInfoES3Test()
       : FramebufferInfoTestBase(ContextGroup::CONTEXT_TYPE_WEBGL2) {
   }
 };
 
@@ skipping 58 matching lines @@
       renderbuffer_manager_->GetRenderbuffer(kRenderbufferClientId);
   ASSERT_TRUE(renderbuffer != NULL);
   framebuffer_->AttachRenderbuffer(GL_COLOR_ATTACHMENT1, renderbuffer);
   EXPECT_TRUE(framebuffer_->GetReadBufferAttachment());
 }
 
 }  // namespace gles2
 }  // namespace gpu