mus: Fix screen tearing on Chromebook.

If you were running mus/mash on a Chromebook with two displays then
there would be constant screen tearing. It turns out that the buffer for
one display was sometimes ending up on the other display. There are two
InProcessCommandBuffers, one for each display, that independently
allocated IDs from CreateImageCHROMIUM starting at 1. This resulted in
ID collisions.

Make all InProcessCommandBuffers allocate IDs for CreateImageCHROMIUM
from a global AtomicSequenceNumber to ensure there are no ID collisions.

Bug: 735631
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I4501afe854041304b4860f0a2a67ca5c1a0e1df9
Reviewed-on: https://chromium-review.googlesource.com/621529
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: weiliangc <weiliangc@chromium.org>
Reviewed-by: Victor Miura <vmiura@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496458}

gpu/BUILD.gn

@@ -191,6 +191,7 @@ test("gl_tests") {
     "//base",
     "//base/test:test_support",
     "//base/third_party/dynamic_annotations",
+    "//components/viz/test:test_support",
     "//gpu/command_buffer/client:gles2_c_lib",
     "//gpu/command_buffer/client:gles2_implementation",
     "//gpu/command_buffer/common:gles2_utils",

gpu/ipc/client/DEPS

@@ -4,3 +4,9 @@ include_rules = [
   "+ui/base",
   "+ui/latency",
 ]
+
+specific_include_rules = {
+  "gpu_in_process_context_tests.cc": [
+    "+components/viz/test/test_gpu_memory_buffer_manager.h",
+  ]
+}

gpu/ipc/client/gpu_in_process_context_tests.cc

@@ -10,6 +10,8 @@
 #include <vector>
 
 #include "base/threading/thread_task_runner_handle.h"
+#include "build/build_config.h"
+#include "components/viz/test/test_gpu_memory_buffer_manager.h"
 #include "gpu/command_buffer/client/gles2_implementation.h"
 #include "gpu/command_buffer/client/shared_memory_limits.h"
 #include "gpu/ipc/common/surface_handle.h"
@@ -21,7 +23,7 @@ namespace {
 
 class ContextTestBase : public testing::Test {
  public:
-  void SetUp() override {
+  std::unique_ptr<gpu::GLInProcessContext> CreateGLInProcessContext() {
     gpu::gles2::ContextCreationAttribHelper attributes;
     attributes.alpha_size = 8;
     attributes.depth_size = 24;
@@ -33,25 +35,34 @@ class ContextTestBase : public testing::Test {
     attributes.sample_buffers = 1;
     attributes.bind_generates_resource = false;
 
-    context_.reset(
-        gpu::GLInProcessContext::Create(nullptr,                 /* service */
-                                        nullptr,                 /* surface */
-                                        true,                    /* offscreen */
-                                        gpu::kNullSurfaceHandle, /* window */
-                                        nullptr, /* share_context */
-                                        attributes, gpu::SharedMemoryLimits(),
-                                        nullptr, /* gpu_memory_buffer_manager */
-                                        nullptr, /* image_factory */
-                                        base::ThreadTaskRunnerHandle::Get()));
+    return base::WrapUnique(gpu::GLInProcessContext::Create(
+        nullptr,                 /* service */
+        nullptr,                 /* surface */
+        true,                    /* offscreen */
+        gpu::kNullSurfaceHandle, /* window */
+        nullptr,                 /* share_context */
+        attributes, gpu::SharedMemoryLimits(), gpu_memory_buffer_manager_.get(),
+        nullptr, /* image_factory */
+        base::ThreadTaskRunnerHandle::Get()));
+  }
+
+  void SetUp() override {
+    gpu_memory_buffer_manager_ =
+        std::make_unique<viz::TestGpuMemoryBufferManager>();
+    context_ = CreateGLInProcessContext();
     gl_ = context_->GetImplementation();
     context_support_ = context_->GetImplementation();
   }
 
-  void TearDown() override { context_.reset(NULL); }
+  void TearDown() override {
+    context_.reset();
+    gpu_memory_buffer_manager_.reset();
+  }
 
  protected:
   gpu::gles2::GLES2Interface* gl_;
   gpu::ContextSupport* context_support_;
+  std::unique_ptr<gpu::GpuMemoryBufferManager> gpu_memory_buffer_manager_;
 
  private:
   std::unique_ptr<gpu::GLInProcessContext> context_;
@@ -62,3 +73,42 @@ class ContextTestBase : public testing::Test {
 // Include the actual tests.
 #define CONTEXT_TEST_F TEST_F
 #include "gpu/ipc/client/gpu_context_tests.h"
+
+using InProcessCommandBufferTest = ContextTestBase;
+
+TEST_F(InProcessCommandBufferTest, CreateImage) {
+  constexpr gfx::BufferFormat kBufferFormat = gfx::BufferFormat::RGBA_8888;
+  constexpr gfx::BufferUsage kBufferUsage = gfx::BufferUsage::SCANOUT;
+  constexpr gfx::Size kBufferSize(100, 100);
+
+#if defined(OS_WIN)
+  // The IPC version of ContextTestBase::SetUpOnMainThread does not succeed on
+  // some platforms.
+  if (!gl_)
+    return;
+#endif
+
+  // Calling CreateImageCHROMIUM() should allocate an image id starting at 1.
+  std::unique_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer1 =
+      gpu_memory_buffer_manager_->CreateGpuMemoryBuffer(
+          kBufferSize, kBufferFormat, kBufferUsage, gpu::kNullSurfaceHandle);
+  int image_id1 = gl_->CreateImageCHROMIUM(gpu_memory_buffer1->AsClientBuffer(),
+                                           kBufferSize.width(),
+                                           kBufferSize.height(), GL_RGBA);
+
+  EXPECT_EQ(image_id1, 1);
+
+  // Create a second GLInProcessContext that is backed by a different
+  // InProcessCommandBuffer. Calling CreateImageCHROMIUM() should return a
+  // different id than the first call.
+  std::unique_ptr<gpu::GLInProcessContext> context2 =
+      CreateGLInProcessContext();
+  std::unique_ptr<gfx::GpuMemoryBuffer> buffer2 =
+      gpu_memory_buffer_manager_->CreateGpuMemoryBuffer(
+          kBufferSize, kBufferFormat, kBufferUsage, gpu::kNullSurfaceHandle);
+  int image_id2 = context2->GetImplementation()->CreateImageCHROMIUM(
+      buffer2->AsClientBuffer(), kBufferSize.width(), kBufferSize.height(),
+      GL_RGBA);
+
+  EXPECT_EQ(image_id2, 2);
+}

gpu/ipc/in_process_command_buffer.cc

@@ -11,6 +11,7 @@
 #include <set>
 #include <utility>
 
+#include "base/atomic_sequence_num.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ -66,6 +67,7 @@ namespace gpu {
 namespace {
 
 base::AtomicSequenceNumber g_next_command_buffer_id;
+base::AtomicSequenceNumber g_next_image_id;
 
 template <typename T>
 static void RunTaskWithResult(base::Callback<T(void)> task,
@@ -207,7 +209,6 @@ InProcessCommandBuffer::InProcessCommandBuffer(
       client_thread_weak_ptr_factory_(this),
       gpu_thread_weak_ptr_factory_(this) {
   DCHECK(service_.get());
-  next_image_id_.GetNext();
 }
 
 InProcessCommandBuffer::~InProcessCommandBuffer() {
@@ -729,7 +730,7 @@ int32_t InProcessCommandBuffer::CreateImage(ClientBuffer buffer,
       reinterpret_cast<gfx::GpuMemoryBuffer*>(buffer);
   DCHECK(gpu_memory_buffer);
 
-  int32_t new_id = next_image_id_.GetNext();
+  int32_t new_id = g_next_image_id.GetNext() + 1;
 
   DCHECK(gpu::IsImageFromGpuMemoryBufferFormatSupported(
       gpu_memory_buffer->GetFormat(), capabilities_));

gpu/ipc/in_process_command_buffer.h

@@ -12,7 +12,6 @@
 #include <memory>
 #include <vector>
 
-#include "base/atomic_sequence_num.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
@@ -335,7 +334,6 @@ class GPU_EXPORT InProcessCommandBuffer : public CommandBuffer,
   int32_t last_put_offset_;
   gpu::Capabilities capabilities_;
   GpuMemoryBufferManager* gpu_memory_buffer_manager_;
-  base::AtomicSequenceNumber next_image_id_;
   uint64_t next_fence_sync_release_;
   uint64_t flushed_fence_sync_release_;