WGC3DCommandBufferImpl: Acquire lock when accessing share group.

Grab the share group lock when accessing the list and also to assure that ShareGroup creation is not racy.

Also do not use an existing ShareGroup is we are not sharing resources.

Remove the nowadays unused share_resources flag that was still being passed around.

On a related note, remove the non-threadsafe
SetGLES2ImplementationForDestruction() which is fortunately unused.
(It was used by StrictSharedIdHandler once, but the current handlers
don't need to talk to the commandbuffer during destruction.)

TBR=brettw@chromium.org
NOTRY=True

Review URL: https://chromiumcodereview.appspot.com/20826002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@214242 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 9 insertions, 63 deletions

diff --git a/gpu/command_buffer/client/gl_in_process_context.cc b/gpu/command_buffer/client/gl_in_process_context.cc
index ccf3b80..2359a05 100644
--- a/gpu/command_buffer/client/gl_in_process_context.cc
+++ b/gpu/command_buffer/client/gl_in_process_context.cc
@@ -461,7 +461,6 @@ bool GLInProcessContextImpl::Initialize(
       gles2_helper_.get(),
       context_group ? context_group->GetImplementation()->share_group() : NULL,
       transfer_buffer_.get(),
-      true,
       false,
       this));
 
diff --git a/gpu/command_buffer/client/gles2_implementation.cc b/gpu/command_buffer/client/gles2_implementation.cc
index e13a1ec..a61da77 100644
--- a/gpu/command_buffer/client/gles2_implementation.cc
+++ b/gpu/command_buffer/client/gles2_implementation.cc
@@ -85,7 +85,6 @@ GLES2Implementation::GLES2Implementation(
       GLES2CmdHelper* helper,
       ShareGroup* share_group,
       TransferBufferInterface* transfer_buffer,
-      bool share_resources,
       bool bind_generates_resource,
       ImageFactory* image_factory)
     : helper_(helper),
@@ -125,8 +124,8 @@ GLES2Implementation::GLES2Implementation(
         switches::kEnableGPUClientLogging);
   });
 
-  share_group_ = (share_group ? share_group : new ShareGroup(
-      share_resources, bind_generates_resource));
+  share_group_ =
+      (share_group ? share_group : new ShareGroup(bind_generates_resource));
 
   memset(&reserved_ids_, 0, sizeof(reserved_ids_));
 }
@@ -266,11 +265,6 @@ GLES2Implementation::~GLES2Implementation() {
 #endif
   buffer_tracker_.reset();
 
-  // The share group needs to be able to use a command buffer to talk
-  // to service if it's destroyed so set one for it then release the reference.
-  // If it's destroyed it will use this GLES2Implemenation.
-  share_group_->SetGLES2ImplementationForDestruction(this);
-  share_group_ = NULL;
   // Make sure the commands make it the service.
   WaitForCmd();
 }
diff --git a/gpu/command_buffer/client/gles2_implementation.h b/gpu/command_buffer/client/gles2_implementation.h
index 44283db..c2bf49f 100644
--- a/gpu/command_buffer/client/gles2_implementation.h
+++ b/gpu/command_buffer/client/gles2_implementation.h
@@ -176,7 +176,6 @@ class GLES2_IMPL_EXPORT GLES2Implementation : public GLES2Interface {
       GLES2CmdHelper* helper,
       ShareGroup* share_group,
       TransferBufferInterface* transfer_buffer,
-      bool share_resources,
       bool bind_generates_resource,
       ImageFactory* image_factory);
 
diff --git a/gpu/command_buffer/client/gles2_implementation_unittest.cc b/gpu/command_buffer/client/gles2_implementation_unittest.cc
index bdbde8f..e58907c 100644
--- a/gpu/command_buffer/client/gles2_implementation_unittest.cc
+++ b/gpu/command_buffer/client/gles2_implementation_unittest.cc
@@ -353,7 +353,7 @@ class GLES2ImplementationTest : public testing::Test {
     return gl_->query_tracker_->GetQuery(id);
   }
 
-  void Initialize(bool shared_resources, bool bind_generates_resource) {
+  void Initialize(bool bind_generates_resource) {
     command_buffer_.reset(new StrictMock<MockClientCommandBuffer>());
     ASSERT_TRUE(command_buffer_->Initialize());
 
@@ -400,7 +400,6 @@ class GLES2ImplementationTest : public testing::Test {
           helper_.get(),
           NULL,
           transfer_buffer_.get(),
-          shared_resources,
           bind_generates_resource,
           NULL));
       ASSERT_TRUE(gl_->Initialize(
@@ -482,7 +481,7 @@ class GLES2ImplementationTest : public testing::Test {
 };
 
 void GLES2ImplementationTest::SetUp() {
-  Initialize(false, true);
+  Initialize(true);
 }
 
 void GLES2ImplementationTest::TearDown() {
@@ -500,7 +499,7 @@ class GLES2ImplementationStrictSharedTest : public GLES2ImplementationTest {
 };
 
 void GLES2ImplementationStrictSharedTest::SetUp() {
-  Initialize(true, false);
+  Initialize(false);
 }
 
 // GCC requires these declarations, but MSVC requires they not be present
diff --git a/gpu/command_buffer/client/share_group.cc b/gpu/command_buffer/client/share_group.cc
index 16163a6..9526df2 100644
--- a/gpu/command_buffer/client/share_group.cc
+++ b/gpu/command_buffer/client/share_group.cc
@@ -22,10 +22,6 @@ class IdHandler : public IdHandlerInterface {
   virtual ~IdHandler() { }
 
   // Overridden from IdHandlerInterface.
-  virtual void Destroy(GLES2Implementation* /* gl_impl */) OVERRIDE {
-  }
-
-  // Overridden from IdHandlerInterface.
   virtual void MakeIds(
       GLES2Implementation* /* gl_impl */,
       GLuint id_offset, GLsizei n, GLuint* ids) OVERRIDE {
@@ -83,10 +79,6 @@ class NonReusedIdHandler : public IdHandlerInterface {
   virtual ~NonReusedIdHandler() {}
 
   // Overridden from IdHandlerInterface.
-  virtual void Destroy(GLES2Implementation* /* gl_impl */) OVERRIDE {
-  }
-
-  // Overridden from IdHandlerInterface.
   virtual void MakeIds(
       GLES2Implementation* /* gl_impl */,
       GLuint id_offset, GLsizei n, GLuint* ids) OVERRIDE {
@@ -124,10 +116,6 @@ class SharedIdHandler : public IdHandlerInterface {
 
   virtual ~SharedIdHandler() {}
 
-  // Overridden from IdHandlerInterface.
-  virtual void Destroy(GLES2Implementation* /* gl_impl */) OVERRIDE {
-  }
-
   virtual void MakeIds(GLES2Implementation* gl_impl,
                        GLuint id_offset,
                        GLsizei n,
@@ -164,12 +152,6 @@ class ThreadSafeIdHandlerWrapper : public IdHandlerInterface {
   virtual ~ThreadSafeIdHandlerWrapper() { }
 
   // Overridden from IdHandlerInterface.
-  virtual void Destroy(GLES2Implementation* gl_impl) OVERRIDE {
-    AutoLock auto_lock(lock_);
-    id_handler_->Destroy(gl_impl);
-  }
-
-  // Overridden from IdHandlerInterface.
   virtual void MakeIds(GLES2Implementation* gl_impl,
                        GLuint id_offset,
                        GLsizei n,
@@ -198,10 +180,8 @@ class ThreadSafeIdHandlerWrapper : public IdHandlerInterface {
    Lock lock_;
 };
 
-ShareGroup::ShareGroup(bool share_resources, bool bind_generates_resource)
-    : sharing_resources_(share_resources),
-      bind_generates_resource_(bind_generates_resource),
-      gles2_(NULL) {
+ShareGroup::ShareGroup(bool bind_generates_resource)
+    : bind_generates_resource_(bind_generates_resource) {
   if (bind_generates_resource) {
     for (int i = 0; i < id_namespaces::kNumIdNamespaces; ++i) {
       if (i == id_namespaces::kProgramsAndShaders) {
@@ -226,21 +206,11 @@ ShareGroup::ShareGroup(bool share_resources, bool bind_generates_resource)
   program_info_manager_.reset(ProgramInfoManager::Create(false));
 }
 
-void ShareGroup::SetGLES2ImplementationForDestruction(
-    GLES2Implementation* gl_impl) {
-  gles2_ = gl_impl;
-}
-
 void ShareGroup::set_program_info_manager(ProgramInfoManager* manager) {
   program_info_manager_.reset(manager);
 }
 
-ShareGroup::~ShareGroup() {
-  for (int i = 0; i < id_namespaces::kNumIdNamespaces; ++i) {
-    id_handlers_[i]->Destroy(gles2_);
-    id_handlers_[i].reset();
-  }
-}
+ShareGroup::~ShareGroup() {}
 
 }  // namespace gles2
 }  // namespace gpu
diff --git a/gpu/command_buffer/client/share_group.h b/gpu/command_buffer/client/share_group.h
index 15d141c..68ea6a9 100644
--- a/gpu/command_buffer/client/share_group.h
+++ b/gpu/command_buffer/client/share_group.h
@@ -26,9 +26,6 @@ class IdHandlerInterface {
   IdHandlerInterface() { }
   virtual ~IdHandlerInterface() { }
 
-  // Free everything.
-  virtual void Destroy(GLES2Implementation* gl_impl) = 0;
-
   // Makes some ids at or above id_offset.
   virtual void MakeIds(
       GLES2Implementation* gl_impl,
@@ -47,13 +44,7 @@ class IdHandlerInterface {
 class GLES2_IMPL_EXPORT ShareGroup
     : public gpu::RefCountedThreadSafe<ShareGroup> {
  public:
-  ShareGroup(bool share_resources, bool bind_generates_resource);
-
-  void SetGLES2ImplementationForDestruction(GLES2Implementation* gl_impl);
-
-  bool sharing_resources() const {
-    return sharing_resources_;
-  }
+  ShareGroup(bool bind_generates_resource);
 
   bool bind_generates_resource() const {
     return bind_generates_resource_;
@@ -80,12 +71,8 @@ class GLES2_IMPL_EXPORT ShareGroup
   scoped_ptr<IdHandlerInterface> id_handlers_[id_namespaces::kNumIdNamespaces];
   scoped_ptr<ProgramInfoManager> program_info_manager_;
 
-  // Whether or not this context is sharing resources.
-  bool sharing_resources_;
   bool bind_generates_resource_;
 
-  GLES2Implementation* gles2_;
-
   DISALLOW_COPY_AND_ASSIGN(ShareGroup);
 };
 
diff --git a/gpu/command_buffer/tests/gl_manager.cc b/gpu/command_buffer/tests/gl_manager.cc
index f4f6c36..c4b84a2 100644
--- a/gpu/command_buffer/tests/gl_manager.cc
+++ b/gpu/command_buffer/tests/gl_manager.cc
@@ -70,7 +70,6 @@ void GLManager::Initialize(const GLManager::Options& options) {
   const size_t kStartTransferBufferSize = 4 * 1024 * 1024;
   const size_t kMinTransferBufferSize = 1 * 256 * 1024;
   const size_t kMaxTransferBufferSize = 16 * 1024 * 1024;
-  const bool kShareResources = true;
 
   context_lost_allowed_ = options.context_lost_allowed;
 
@@ -200,7 +199,6 @@ void GLManager::Initialize(const GLManager::Options& options) {
       gles2_helper_.get(),
       client_share_group,
       transfer_buffer_.get(),
-      kShareResources,
       options.bind_generates_resource,
       options.image_factory));