Make lost context and error message callbacks on GpuControl go to client

gpu/command_buffer/client/gl_in_process_context.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/client/gl_in_process_context.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <set>
@@ skipping 31 matching lines @@
 
 namespace {
 
 const int32_t kDefaultCommandBufferSize = 1024 * 1024;
 const unsigned int kDefaultStartTransferBufferSize = 4 * 1024 * 1024;
 const unsigned int kDefaultMinTransferBufferSize = 1 * 256 * 1024;
 const unsigned int kDefaultMaxTransferBufferSize = 16 * 1024 * 1024;
 
 class GLInProcessContextImpl
     : public GLInProcessContext,
+      public GpuControlClient,
       public base::SupportsWeakPtr<GLInProcessContextImpl> {
  public:
   explicit GLInProcessContextImpl(
       const GLInProcessContextSharedMemoryLimits& mem_limits);
   ~GLInProcessContextImpl() override;
 
   bool Initialize(scoped_refptr<gfx::GLSurface> surface,
                   bool is_offscreen,
                   bool use_global_share_group,
                   GLInProcessContext* share_context,
                   gfx::AcceleratedWidget window,
                   const gfx::Size& size,
                   const gpu::gles2::ContextCreationAttribHelper& attribs,
                   gfx::GpuPreference gpu_preference,
                   const scoped_refptr<InProcessCommandBuffer::Service>& service,
                   GpuMemoryBufferManager* gpu_memory_buffer_manager,
                   ImageFactory* image_factory);
 
-  // GLInProcessContext implementation:
-  void SetContextLostCallback(const base::Closure& callback) override;
+  // GLInProcessContext implementation.
   gles2::GLES2Implementation* GetImplementation() override;
   size_t GetMappedMemoryLimit() override;
   void SetLock(base::Lock* lock) override;
 
+  // GpuControlClient implementation. These methods are called on the GPU thread

[danakj, 2016/04/06 02:14:15]

It all gets quite hairy in here. The InProcessCommandBuffer runs on the GPU
thread, so these methods get called on the GPU thread. Which is ok I guess in
theory.

Except, in this patchset I have GLInProcessContextImpl as the GpuControlClient,
but the correct relationship is to have GLES2Implementation as the client. So I
should be calling

command_buffer_->SetGpuControlClient(gles2_implementation_.get());

But.. GLES2Implementation is not expecting those methods to be called on a GPU
thread.

And it turns out we want to intercept the lost context event in here somehow and
store lost_context_ = true on the context...

Alternatively we need multiple lost context callbacks on GLES2Implementation I
guess. But still we're calling into GLES2Implementation on the wrong thread
then..

[danakj, 2016/04/06 02:19:58]

Oh no I realized this class should not be calling SetGpuControlClient() on the
InProcessCommandBuffer at all. GLES2Implementation will already do that.

But GLES2Implementation is going to get called on the wrong thread then.

So.. do we have to do this with callbacks (wrapped in callbacks inside
callbacks) to thread hop here? :/

[danakj, 2016/04/06 02:25:44]

It seems like what's weird here (or different) is that InProcessCommandBuffer is
entirely living on the GPU thread (aka server side), whereas in the OOP version
we have CommandBufferProxyImpl on client side.

Perhaps we should have a thread split on InProcessCommandBuffer, and the
GpuControl parts should be on the main thread, and the rest of it should be on
the GPU thread? Then calling back to the GpuControlClient would be a thread hop
in the InProcessCommandBuffer (but wouldn't need nested callback dolls yay)
since it would be going to the "GpuControl" thread.

+  // by the in-process GpuControl.
+  void OnGpuControlErrorMessage(const char* message, int32_t id) override {}
+  void OnGpuControlLostContext() override;
+
 #if defined(OS_ANDROID)
   scoped_refptr<gfx::SurfaceTexture> GetSurfaceTexture(
       uint32_t stream_id) override;
   uint32_t CreateStreamTexture(uint32_t texture_id) override;
 #endif
 
  private:
   void Destroy();
-  void OnContextLost();
+  bool IsContextLost() const;
   void OnSignalSyncPoint(const base::Closure& callback);
 
   scoped_ptr<gles2::GLES2CmdHelper> gles2_helper_;
   scoped_ptr<TransferBuffer> transfer_buffer_;
   scoped_ptr<gles2::GLES2Implementation> gles2_implementation_;
   scoped_ptr<InProcessCommandBuffer> command_buffer_;
 
   const GLInProcessContextSharedMemoryLimits mem_limits_;
+  mutable base::Lock context_lost_lock_;

[danakj, 2016/04/06 02:14:15]

We used to wrap callbacks inside callbacks to get to the right thread, but all
that just to set a context_lost_ bool.. so, I just added a lock instead.

Where I start to get lost is why we had this other |lock_|, which we'd grab when
setting the context_lost_ bool. The lock_ is set via SetLock in the
GLInProcessContext API, and passed to the InProcessCommandBuffer, but never
used!

The lock_ is actually owned out in (for example) AwRenderThreadContextProvider,
which calls SetLock in its own public SetupLock which is part of the
ContextProvider API.

So.. I don't get the point of this lock_ at all basically, and I removed it
(replaced with this new locally-owned lock). But then.. why have a lock on
AwRenderThreadContextProvider? Everything is confusing, halp.

   bool context_lost_;
-  base::Closure context_lost_callback_;
-  base::Lock* lock_;
 
   DISALLOW_COPY_AND_ASSIGN(GLInProcessContextImpl);
 };
 
 base::LazyInstance<base::Lock> g_all_shared_contexts_lock =
     LAZY_INSTANCE_INITIALIZER;
 base::LazyInstance<std::set<GLInProcessContextImpl*> > g_all_shared_contexts =
     LAZY_INSTANCE_INITIALIZER;
 
 GLInProcessContextImpl::GLInProcessContextImpl(
     const GLInProcessContextSharedMemoryLimits& mem_limits)
-    : mem_limits_(mem_limits), context_lost_(false), lock_(nullptr) {
-}
+    : mem_limits_(mem_limits) {}
 
 GLInProcessContextImpl::~GLInProcessContextImpl() {
   {
     base::AutoLock lock(g_all_shared_contexts_lock.Get());
     g_all_shared_contexts.Get().erase(this);
   }
   Destroy();
 }
 
 gles2::GLES2Implementation* GLInProcessContextImpl::GetImplementation() {
   return gles2_implementation_.get();
 }
 
 size_t GLInProcessContextImpl::GetMappedMemoryLimit() {
   return mem_limits_.mapped_memory_reclaim_limit;
 }
 
 void GLInProcessContextImpl::SetLock(base::Lock* lock) {
+  // TODO(danakj): This isn't used... remove from the GLInProcessContext api?
   command_buffer_->SetLock(lock);
-  lock_ = lock;
 }
 
-void GLInProcessContextImpl::SetContextLostCallback(
-    const base::Closure& callback) {
-  context_lost_callback_ = callback;
+void GLInProcessContextImpl::OnGpuControlLostContext() {
+  // This is called on the GPU thread.
+  base::AutoLock lock(context_lost_lock_);
+  context_lost_ = true;
 }
 
-void GLInProcessContextImpl::OnContextLost() {
-  scoped_ptr<base::AutoLock> lock;
-  if (lock_)
-    lock.reset(new base::AutoLock(*lock_));
-  context_lost_ = true;
-  if (!context_lost_callback_.is_null()) {
-    context_lost_callback_.Run();
-  }
+bool GLInProcessContextImpl::IsContextLost() const {
+  base::AutoLock lock(context_lost_lock_);
+  return context_lost_;
 }
 
 bool GLInProcessContextImpl::Initialize(
     scoped_refptr<gfx::GLSurface> surface,
     bool is_offscreen,
     bool use_global_share_group,
     GLInProcessContext* share_context,
     gfx::AcceleratedWidget window,
     const gfx::Size& size,
     const gles2::ContextCreationAttribHelper& attribs,
     gfx::GpuPreference gpu_preference,
     const scoped_refptr<InProcessCommandBuffer::Service>& service,
     GpuMemoryBufferManager* gpu_memory_buffer_manager,
     ImageFactory* image_factory) {
   DCHECK(!use_global_share_group || !share_context);
   DCHECK(size.width() >= 0 && size.height() >= 0);
 
   std::vector<int32_t> attrib_vector;
   attribs.Serialize(&attrib_vector);
 
-  base::Closure wrapped_callback =
-      base::Bind(&GLInProcessContextImpl::OnContextLost, AsWeakPtr());
   command_buffer_.reset(new InProcessCommandBuffer(service));
 
   scoped_ptr<base::AutoLock> scoped_shared_context_lock;
   scoped_refptr<gles2::ShareGroup> share_group;
   InProcessCommandBuffer* share_command_buffer = NULL;
   if (use_global_share_group) {
     scoped_shared_context_lock.reset(
         new base::AutoLock(g_all_shared_contexts_lock.Get()));
     for (std::set<GLInProcessContextImpl*>::const_iterator it =
              g_all_shared_contexts.Get().begin();
          it != g_all_shared_contexts.Get().end();
          it++) {
       const GLInProcessContextImpl* context = *it;
-      if (!context->context_lost_) {
+      if (!context->IsContextLost()) {
         share_group = context->gles2_implementation_->share_group();
         share_command_buffer = context->command_buffer_.get();
         DCHECK(share_group.get());
         DCHECK(share_command_buffer);
         break;
       }
     }
   } else if (share_context) {
     GLInProcessContextImpl* impl =
         static_cast<GLInProcessContextImpl*>(share_context);
     share_group = impl->gles2_implementation_->share_group();
     share_command_buffer = impl->command_buffer_.get();
     DCHECK(share_group.get());
     DCHECK(share_command_buffer);
   }
 
+  command_buffer_->SetGpuControlClient(this);
+
   if (!command_buffer_->Initialize(surface,
                                    is_offscreen,
                                    window,
                                    size,
                                    attrib_vector,
                                    gpu_preference,
-                                   wrapped_callback,
                                    share_command_buffer,
                                    gpu_memory_buffer_manager,
                                    image_factory)) {
     LOG(ERROR) << "Failed to initialize InProcessCommmandBuffer";
     return false;
   }
 
   // Create the GLES2 helper, which writes the command buffer protocol.
   gles2_helper_.reset(new gles2::GLES2CmdHelper(command_buffer_.get()));
   if (!gles2_helper_->Initialize(mem_limits_.command_buffer_size)) {
@@ skipping 107 matching lines @@
                            gpu_preference,
                            service,
                            gpu_memory_buffer_manager,
                            image_factory))
     return NULL;
 
   return context.release();
 }
 
 }  // namespace gpu