Catch lost context errors

Sync between client and service side to eliminate races.
Then execute any callbacks on the client thread task runner.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I7dc7a2aa0b7f6ed30f7a083c622fb5cbf68d521e
Reviewed-on: https://chromium-review.googlesource.com/1089637
Commit-Queue: Jonathan Backer <backer@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565393}

cc/paint/transfer_cache_unittest.cc

@@ -49,8 +49,7 @@ class TransferCacheTest : public testing::Test {
     auto result = context_->Initialize(
         /*service=*/nullptr, attribs, gpu::SharedMemoryLimits(),
         &gpu_memory_buffer_manager_, &image_factory_,
-        /*gpu_channel_manager_delegate=*/nullptr,
-        base::ThreadTaskRunnerHandle::Get());
+        /*gpu_channel_manager_delegate=*/nullptr);
 
     ASSERT_EQ(result, gpu::ContextResult::kSuccess);
     ASSERT_TRUE(context_->GetCapabilities().supports_oop_raster);

cc/test/test_in_process_context_provider.cc

@@ -75,8 +75,7 @@ TestInProcessContextProvider::TestInProcessContextProvider(
     auto result = raster_context_->Initialize(
         /*service=*/nullptr, attribs, gpu::SharedMemoryLimits(),
         &gpu_memory_buffer_manager_, &image_factory_,
-        /*gpu_channel_manager_delegate=*/nullptr,
-        base::ThreadTaskRunnerHandle::Get());
+        /*gpu_channel_manager_delegate=*/nullptr);
     DCHECK_EQ(result, gpu::ContextResult::kSuccess);
 
     cache_controller_.reset(

gpu/BUILD.gn

@@ -156,6 +156,7 @@ static_library("test_support") {
     "//gpu/command_buffer/client:gles2_interface",
   ]
   deps = [
+    "//base/test:test_support",
     "//gpu/command_buffer/client:raster",
     "//gpu/command_buffer/common",
     "//gpu/ipc:gl_in_process_context",

gpu/ipc/client/raster_in_process_context_tests.cc

@@ -4,7 +4,6 @@
 
 #include <memory>
 
-#include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "cc/paint/color_space_transfer_cache_entry.h"
 #include "components/viz/common/resources/resource_format.h"
@@ -41,8 +40,7 @@ class RasterInProcessCommandBufferTest : public ::testing::Test {
         /*service=*/nullptr, attributes, SharedMemoryLimits(),
         gpu_memory_buffer_manager_.get(),
         gpu_memory_buffer_factory_->AsImageFactory(),
-        /*gpu_channel_manager_delegate=*/nullptr,
-        base::ThreadTaskRunnerHandle::Get());
+        /*gpu_channel_manager_delegate=*/nullptr);
     DCHECK_EQ(result, ContextResult::kSuccess);
     return context;
   }

gpu/ipc/in_process_command_buffer.h

@@ -341,7 +341,6 @@ class GL_IN_PROCESS_CONTEXT_EXPORT InProcessCommandBuffer
   scoped_refptr<gl::GLSurface> surface_;
   scoped_refptr<SyncPointOrderData> sync_point_order_data_;
   scoped_refptr<SyncPointClientState> sync_point_client_state_;
-  base::Closure context_lost_callback_;
   // Used to throttle PerformDelayedWorkOnGpuThread.
   bool delayed_work_pending_ = false;
   ImageFactory* image_factory_ = nullptr;

gpu/ipc/raster_in_process_context.cc

@@ -8,6 +8,7 @@
 
 #include "base/command_line.h"
 #include "base/logging.h"
+#include "base/test/test_simple_task_runner.h"
 #include "gpu/command_buffer/client/gles2_cmd_helper.h"
 #include "gpu/command_buffer/client/raster_cmd_helper.h"
 #include "gpu/command_buffer/client/raster_implementation.h"
@@ -20,19 +21,18 @@
 #include "gpu/config/gpu_feature_info.h"
 #include "gpu/config/gpu_switches.h"
 #include "gpu/ipc/common/surface_handle.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 namespace gpu {
 
 RasterInProcessContext::RasterInProcessContext() = default;
 
 RasterInProcessContext::~RasterInProcessContext() {
-  // First flush the contexts to ensure that any pending frees of resources
-  // are completed. Otherwise, if this context is part of a share group,
-  // those resources might leak. Also, any remaining side effects of commands
-  // issued on this context might not be visible to other contexts in the
-  // share group.
+  // Trigger any pending lost contexts. First do a full sync between client
+  // and service threads. Then execute any pending tasks.
   if (raster_implementation_) {
-    raster_implementation_->Flush();
+    raster_implementation_->Finish();
+    client_task_runner_->RunUntilIdle();
     raster_implementation_.reset();
   }
   transfer_buffer_.reset();
@@ -46,8 +46,7 @@ ContextResult RasterInProcessContext::Initialize(
     const SharedMemoryLimits& memory_limits,
     GpuMemoryBufferManager* gpu_memory_buffer_manager,
     ImageFactory* image_factory,
-    GpuChannelManagerDelegate* gpu_channel_manager_delegate,
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
+    GpuChannelManagerDelegate* gpu_channel_manager_delegate) {
   DCHECK(attribs.enable_raster_interface);
   if (!attribs.enable_raster_interface) {
     return ContextResult::kFatalFailure;
@@ -57,11 +56,12 @@ ContextResult RasterInProcessContext::Initialize(
     return ContextResult::kFatalFailure;
   }
 
+  client_task_runner_ = base::MakeRefCounted<base::TestSimpleTaskRunner>();
   command_buffer_ = std::make_unique<InProcessCommandBuffer>(service);
   auto result = command_buffer_->Initialize(
       nullptr /* surface */, true /* is_offscreen */, kNullSurfaceHandle,
       attribs, nullptr /* share_command_buffer */, gpu_memory_buffer_manager,
-      image_factory, gpu_channel_manager_delegate, std::move(task_runner));
+      image_factory, gpu_channel_manager_delegate, client_task_runner_);
   if (result != ContextResult::kSuccess) {
     DLOG(ERROR) << "Failed to initialize InProcessCommmandBuffer";
     return result;
@@ -86,11 +86,12 @@ ContextResult RasterInProcessContext::Initialize(
   }
   transfer_buffer_ = std::make_unique<TransferBuffer>(raster_helper.get());
 
-  auto raster_implementation = std::make_unique<raster::RasterImplementation>(
+  raster_implementation_ = std::make_unique<raster::RasterImplementation>(
       raster_helper.get(), transfer_buffer_.get(), bind_generates_resource,
       attribs.lose_context_when_out_of_memory, command_buffer_.get());
-  result = raster_implementation->Initialize(memory_limits);
-  raster_implementation_ = std::move(raster_implementation);
+  result = raster_implementation_->Initialize(memory_limits);
+  raster_implementation_->SetLostContextCallback(base::BindOnce(
+      []() { EXPECT_TRUE(false) << "Unexpected lost context."; }));
   helper_ = std::move(raster_helper);
   return result;
 }

gpu/ipc/raster_in_process_context.h

@@ -12,6 +12,10 @@
 #include "base/single_thread_task_runner.h"
 #include "gpu/ipc/in_process_command_buffer.h"
 
+namespace base {
+class TestSimpleTaskRunner;
+}  // namespace base
+
 namespace gpu {
 class CommandBufferHelper;
 class ContextSupport;
@@ -41,15 +45,14 @@ class RasterInProcessContext {
       const SharedMemoryLimits& memory_limits,
       GpuMemoryBufferManager* gpu_memory_buffer_manager,
       ImageFactory* image_factory,
-      GpuChannelManagerDelegate* gpu_channel_manager_delegate,
-      scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+      GpuChannelManagerDelegate* gpu_channel_manager_delegate);
 
   const Capabilities& GetCapabilities() const;
   const GpuFeatureInfo& GetGpuFeatureInfo() const;
 
   // Allows direct access to the RasterImplementation so a
   // RasterInProcessContext can be used without making it current.
-  raster::RasterInterface* GetImplementation();
+  gpu::raster::RasterInterface* GetImplementation();
 
   ContextSupport* GetContextSupport();
 
@@ -62,6 +65,7 @@ class RasterInProcessContext {
   std::unique_ptr<TransferBuffer> transfer_buffer_;
   std::unique_ptr<raster::RasterImplementation> raster_implementation_;
   std::unique_ptr<InProcessCommandBuffer> command_buffer_;
+  scoped_refptr<base::TestSimpleTaskRunner> client_task_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(RasterInProcessContext);
 };