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

gpu/command_buffer/service/in_process_command_buffer.cc

Index: gpu/command_buffer/service/in_process_command_buffer.cc
diff --git a/gpu/command_buffer/service/in_process_command_buffer.cc b/gpu/command_buffer/service/in_process_command_buffer.cc
index 27824398429933e1a0d14ac2ef2034f4577776fd..19bc1b4106065809b8e34b104a59ba1c8f302fe2 100644
--- a/gpu/command_buffer/service/in_process_command_buffer.cc
+++ b/gpu/command_buffer/service/in_process_command_buffer.cc
@@ -22,6 +22,7 @@
 #include "base/sequence_checker.h"
 #include "base/single_thread_task_runner.h"
 #include "base/thread_task_runner_handle.h"
+#include "gpu/command_buffer/client/gpu_control_client.h"
 #include "gpu/command_buffer/client/gpu_memory_buffer_manager.h"
 #include "gpu/command_buffer/common/gpu_memory_buffer_support.h"
 #include "gpu/command_buffer/common/sync_token.h"
@@ -210,6 +211,8 @@ InProcessCommandBuffer::InProcessCommandBuffer(
     const scoped_refptr<Service>& service)
     : command_buffer_id_(
           CommandBufferId::FromUnsafeValue(g_next_command_buffer_id.GetNext())),
+      gpu_control_client_(nullptr),
+      context_lost_(false),
       delayed_work_pending_(false),
       image_factory_(nullptr),
       last_put_offset_(-1),
@@ -219,6 +222,7 @@ InProcessCommandBuffer::InProcessCommandBuffer(
       flush_event_(false, false),
       service_(GetInitialService(service)),
       fence_sync_wait_event_(false, false),
+      client_thread_weak_ptr_factory_(this),
       gpu_thread_weak_ptr_factory_(this) {
   DCHECK(service_.get());
   next_image_id_.GetNext();
@@ -245,7 +249,7 @@ bool InProcessCommandBuffer::MakeCurrent() {
   return true;
 }
 
-void InProcessCommandBuffer::PumpCommands() {
+void InProcessCommandBuffer::PumpCommandsOnGpuThread() {
   CheckSequencedThread();
   command_buffer_lock_.AssertAcquired();
 
@@ -255,13 +259,6 @@ void InProcessCommandBuffer::PumpCommands() {
   executor_->PutChanged();
 }
 
-bool InProcessCommandBuffer::GetBufferChanged(int32_t transfer_buffer_id) {
-  CheckSequencedThread();
-  command_buffer_lock_.AssertAcquired();
-  command_buffer_->SetGetBuffer(transfer_buffer_id);
-  return true;
-}
-
 bool InProcessCommandBuffer::Initialize(
     scoped_refptr<gfx::GLSurface> surface,
     bool is_offscreen,
@@ -269,12 +266,10 @@ bool InProcessCommandBuffer::Initialize(
     const gfx::Size& size,
     const std::vector<int32_t>& attribs,
     gfx::GpuPreference gpu_preference,
-    const base::Closure& context_lost_callback,
     InProcessCommandBuffer* share_group,
     GpuMemoryBufferManager* gpu_memory_buffer_manager,
     ImageFactory* image_factory) {
   DCHECK(!share_group || service_.get() == share_group->service_.get());
-  context_lost_callback_ = WrapCallback(context_lost_callback);
 
   if (surface.get()) {
     // GPU thread must be the same as client thread due to GLSurface not being
@@ -283,6 +278,9 @@ bool InProcessCommandBuffer::Initialize(
     surface_ = surface;
   }
 
+  origin_task_runner_ = base::ThreadTaskRunnerHandle::Get();

[boliu, 2016/04/08 19:51:17]

Oh! Need to deal with the case that this is null. Happens on android webview on
android's render thread.

In that case, the client and server threads are the same, so a direct call is
ok. Guess need to compare PlatformThreadHandle to get that case.

[danakj, 2016/04/08 19:55:41]

The old code dealt with null in WrapCallback and PostCallback by just calling
the function directly. I should be doing the same thing in
OnContextLostOnGpuThread().

I guess it would call IsSet tho before calling Get, I can restore that.

[danakj, 2016/04/08 19:56:53]

Are there no tests where IsSet is false?

[boliu, 2016/04/08 20:02:34]

Most of the webview instrumentation tests won't have a message loop here I
believe. But they don't explicitly exercise the context loss path here.

OnContextLostOnGpuThread already checks for null origin_task_runner_, so maybe
this should be ok. Not sure why those tests are blowing up then.. Need to wait
for the bot to finish to get symbolized stack, hopefully.

[danakj, 2016/04/08 20:14:04]

I updated the CL to only set these when surface is null (which is when we don't
set the sequence_checker_ also).

[boliu, 2016/04/08 21:01:06]

And webview tests green:
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...

\o/

+  client_thread_weak_ptr_ = client_thread_weak_ptr_factory_.GetWeakPtr();
+
   gpu::Capabilities capabilities;
   InitializeOnGpuThreadParams params(is_offscreen,
                                      window,
@@ -328,9 +326,9 @@ bool InProcessCommandBuffer::InitializeOnGpuThread(
   scoped_ptr<CommandBufferService> command_buffer(
       new CommandBufferService(transfer_buffer_manager_.get()));
   command_buffer->SetPutOffsetChangeCallback(base::Bind(
-      &InProcessCommandBuffer::PumpCommands, gpu_thread_weak_ptr_));
+      &InProcessCommandBuffer::PumpCommandsOnGpuThread, gpu_thread_weak_ptr_));
   command_buffer->SetParseErrorCallback(base::Bind(
-      &InProcessCommandBuffer::OnContextLost, gpu_thread_weak_ptr_));
+      &InProcessCommandBuffer::OnContextLostOnGpuThread, gpu_thread_weak_ptr_));
 
   if (!command_buffer->Initialize()) {
     LOG(ERROR) << "Could not initialize command buffer.";
@@ -455,7 +453,7 @@ bool InProcessCommandBuffer::InitializeOnGpuThread(
 
 void InProcessCommandBuffer::Destroy() {
   CheckSequencedThread();
-
+  client_thread_weak_ptr_factory_.InvalidateWeakPtrs();
   base::WaitableEvent completion(true, false);
   bool result = false;
   base::Callback<bool(void)> destroy_task = base::Bind(
@@ -475,14 +473,14 @@ bool InProcessCommandBuffer::DestroyOnGpuThread() {
     decoder_->Destroy(have_context);
     decoder_.reset();
   }
-  context_ = NULL;
-  surface_ = NULL;
-  sync_point_client_ = NULL;
+  context_ = nullptr;
+  surface_ = nullptr;
+  sync_point_client_ = nullptr;
   if (sync_point_order_data_) {
     sync_point_order_data_->Destroy();
     sync_point_order_data_ = nullptr;
   }
-  gl_share_group_ = NULL;
+  gl_share_group_ = nullptr;
 #if defined(OS_ANDROID)
   stream_texture_manager_.reset();
 #endif
@@ -495,12 +493,22 @@ void InProcessCommandBuffer::CheckSequencedThread() {
          sequence_checker_->CalledOnValidSequencedThread());
 }
 
+void InProcessCommandBuffer::OnContextLostOnGpuThread() {
+  if (context_lost_)
+    return;
+  if (!origin_task_runner_ || origin_task_runner_->BelongsToCurrentThread())
+    return OnContextLost();
+  origin_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&InProcessCommandBuffer::OnContextLost,
+                            client_thread_weak_ptr_));
+}
+
 void InProcessCommandBuffer::OnContextLost() {
   CheckSequencedThread();
-  if (!context_lost_callback_.is_null()) {
-    context_lost_callback_.Run();
-    context_lost_callback_.Reset();
-  }
+  DCHECK(gpu_control_client_);
+  if (!context_lost_)
+    gpu_control_client_->OnGpuControlLostContext();
+  context_lost_ = true;
 }
 
 CommandBuffer::State InProcessCommandBuffer::GetStateFast() {
@@ -554,7 +562,7 @@ void InProcessCommandBuffer::FlushOnGpuThread(int32_t put_offset,
   }
 }
 
-void InProcessCommandBuffer::PerformDelayedWork() {
+void InProcessCommandBuffer::PerformDelayedWorkOnGpuThread() {
   CheckSequencedThread();
   delayed_work_pending_ = false;
   base::AutoLock lock(command_buffer_lock_);
@@ -572,8 +580,9 @@ void InProcessCommandBuffer::ScheduleDelayedWorkOnGpuThread() {
   if (delayed_work_pending_)
     return;
   delayed_work_pending_ = true;
-  service_->ScheduleDelayedWork(base::Bind(
-      &InProcessCommandBuffer::PerformDelayedWork, gpu_thread_weak_ptr_));
+  service_->ScheduleDelayedWork(
+      base::Bind(&InProcessCommandBuffer::PerformDelayedWorkOnGpuThread,
+                 gpu_thread_weak_ptr_));
 }
 
 void InProcessCommandBuffer::Flush(int32_t put_offset) {
@@ -670,6 +679,10 @@ void InProcessCommandBuffer::DestroyTransferBufferOnGpuThread(int32_t id) {
   command_buffer_->DestroyTransferBuffer(id);
 }
 
+void InProcessCommandBuffer::SetGpuControlClient(GpuControlClient* client) {
+  gpu_control_client_ = client;
+}
+
 gpu::Capabilities InProcessCommandBuffer::GetCapabilities() {
   return capabilities_;
 }