Improve GpuMemoryBufferFactoryAndroidHardwareBuffer thread safety.

The overridden functions for GpuMemoryBufferFactory can be called on
multiple threads with OOP-D. Add a lock to protect |buffer_map_| which
is used in both CreateGpuMemoryBuffer() and DestroyGpuMemoryBuffer().

Bug: 902163
Change-Id: Ib80169103677f5bdaccc2d4803efa029f4910b03
Reviewed-on: https://chromium-review.googlesource.com/c/1329547
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606989}

gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.cc

@@ -7,6 +7,7 @@
 #include "base/android/android_hardware_buffer_compat.h"
 #include "base/logging.h"
 #include "base/memory/shared_memory_handle.h"
+#include "base/stl_util.h"
 #include "build/build_config.h"
 #include "gpu/ipc/common/gpu_memory_buffer_impl_android_hardware_buffer.h"
 #include "ui/gl/gl_image_ahardwarebuffer.h"
@@ -14,10 +15,10 @@
 namespace gpu {
 
 GpuMemoryBufferFactoryAndroidHardwareBuffer::
-    GpuMemoryBufferFactoryAndroidHardwareBuffer() {}
+    GpuMemoryBufferFactoryAndroidHardwareBuffer() = default;
 
 GpuMemoryBufferFactoryAndroidHardwareBuffer::
-    ~GpuMemoryBufferFactoryAndroidHardwareBuffer() {}
+    ~GpuMemoryBufferFactoryAndroidHardwareBuffer() = default;
 
 gfx::GpuMemoryBufferHandle
 GpuMemoryBufferFactoryAndroidHardwareBuffer::CreateGpuMemoryBuffer(
@@ -27,11 +28,6 @@ GpuMemoryBufferFactoryAndroidHardwareBuffer::CreateGpuMemoryBuffer(
     gfx::BufferUsage usage,
     int client_id,
     SurfaceHandle surface_handle) {
-  if (buffer_map_.find(id) != buffer_map_.end()) {
-    LOG(ERROR) << "Tried to create new GpuMemoryBuffer with an existing id";
-    return gfx::GpuMemoryBufferHandle();
-  }
-
   auto buffer = GpuMemoryBufferImplAndroidHardwareBuffer::Create(
       id, size, format, usage, GpuMemoryBufferImpl::DestructionCallback());
   if (!buffer) {
@@ -39,20 +35,23 @@ GpuMemoryBufferFactoryAndroidHardwareBuffer::CreateGpuMemoryBuffer(
     return gfx::GpuMemoryBufferHandle();
   }
   auto handle = buffer->CloneHandle();
-  buffer_map_[id] = std::move(buffer);
+
+  {
+    base::AutoLock lock(lock_);
+    BufferMapKey key(id, client_id);
+    DLOG_IF(ERROR, base::ContainsKey(buffer_map_, key))
+        << "Created GpuMemoryBuffer with duplicate id";
+    buffer_map_[key] = std::move(buffer);
+  }
   return handle;
 }
 
 void GpuMemoryBufferFactoryAndroidHardwareBuffer::DestroyGpuMemoryBuffer(
     gfx::GpuMemoryBufferId id,
     int client_id) {
-  auto it = buffer_map_.find(id);
-  if (it == buffer_map_.end()) {
-    LOG(ERROR) << "Tried to delete non existent GpuMemoryBuffer";
-    return;
-  }
-
-  buffer_map_.erase(it);
+  base::AutoLock lock(lock_);
+  BufferMapKey key(id, client_id);
+  buffer_map_.erase(key);
 }
 
 ImageFactory* GpuMemoryBufferFactoryAndroidHardwareBuffer::AsImageFactory() {

gpu/ipc/service/gpu_memory_buffer_factory_android_hardware_buffer.h

@@ -5,7 +5,11 @@
 #ifndef GPU_IPC_SERVICE_GPU_MEMORY_BUFFER_FACTORY_ANDROID_HARDWARE_BUFFER_H_
 #define GPU_IPC_SERVICE_GPU_MEMORY_BUFFER_FACTORY_ANDROID_HARDWARE_BUFFER_H_
 
+#include <memory>
+#include <utility>
+
 #include "base/containers/flat_map.h"
+#include "base/synchronization/lock.h"
 #include "gpu/command_buffer/service/image_factory.h"
 #include "gpu/ipc/service/gpu_ipc_service_export.h"
 #include "gpu/ipc/service/gpu_memory_buffer_factory.h"
@@ -47,10 +51,13 @@ class GPU_IPC_SERVICE_EXPORT GpuMemoryBufferFactoryAndroidHardwareBuffer
   unsigned RequiredTextureType() override;
 
  private:
+  using BufferMapKey = std::pair<gfx::GpuMemoryBufferId, int>;
   using BufferMap =
-      base::flat_map<gfx::GpuMemoryBufferId,
+      base::flat_map<BufferMapKey,
                      std::unique_ptr<GpuMemoryBufferImplAndroidHardwareBuffer>>;
-  BufferMap buffer_map_;
+
+  base::Lock lock_;
+  BufferMap buffer_map_ GUARDED_BY(lock_);
 
   DISALLOW_COPY_AND_ASSIGN(GpuMemoryBufferFactoryAndroidHardwareBuffer);
 };