Don't block IO thread for GpuMemoryBuffer allocations

Currently GpuServiceImpl::CreateGpuMemoryBuffer blocks the IO thread
until the DRM thread allocates the buffer. However, the DRM thread
cannot allocate the buffer until the corresponding AcceleratedWidget is
registered, which depends on the IO thread staying responsive in order
to receive the registration message from the browser and process it.
This causes a deadlock. In order to avoid this, make
GpuServiceImpl::CreateGpuMemoryBuffer asynchronous.

Bug: 981721,620927
Change-Id: Id35f17cc4ff3a800999e84cb76ca3e134bc13a8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1696353
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Peng Huang <penghuang@chromium.org>
Reviewed-by: Michael Spang <spang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676629}

components/viz/service/gl/gpu_service_impl.cc

@@ -491,8 +491,8 @@ void GpuServiceImpl::CreateGpuMemoryBuffer(
     CreateGpuMemoryBufferCallback callback) {
   DCHECK(io_runner_->BelongsToCurrentThread());
   // This needs to happen in the IO thread.
-  std::move(callback).Run(gpu_memory_buffer_factory_->CreateGpuMemoryBuffer(
-      id, size, format, usage, client_id, surface_handle));
+  gpu_memory_buffer_factory_->CreateGpuMemoryBufferAsync(
+      id, size, format, usage, client_id, surface_handle, std::move(callback));
 }
 
 void GpuServiceImpl::DestroyGpuMemoryBuffer(gfx::GpuMemoryBufferId id,

gpu/ipc/service/gpu_memory_buffer_factory.cc

@@ -44,4 +44,18 @@ GpuMemoryBufferFactory::CreateNativeType(
 #endif
 }
 
+void GpuMemoryBufferFactory::CreateGpuMemoryBufferAsync(
+    gfx::GpuMemoryBufferId id,
+    const gfx::Size& size,
+    gfx::BufferFormat format,
+    gfx::BufferUsage usage,
+    int client_id,
+    SurfaceHandle surface_handle,
+    CreateGpuMemoryBufferAsyncCallback callback) {
+  // By default, we assume it's ok to allocate GMBs synchronously on the IO
+  // thread. However, subclasses can override this assumption.
+  std::move(callback).Run(CreateGpuMemoryBuffer(id, size, format, usage,
+                                                client_id, surface_handle));
+}
+
 }  // namespace gpu

gpu/ipc/service/gpu_memory_buffer_factory.h

@@ -33,7 +33,9 @@ class GPU_IPC_SERVICE_EXPORT GpuMemoryBufferFactory {
       viz::VulkanContextProvider* vulkan_context_provider);
 
   // Creates a new GPU memory buffer instance. A valid handle is returned on
-  // success. It can be called on any thread.
+  // success. This method is thread-safe but it should not be called on the IO
+  // thread as it can lead to deadlocks (see https://crbug.com/981721). Instead
+  // use the asynchronous version on the IO thread.
   virtual gfx::GpuMemoryBufferHandle CreateGpuMemoryBuffer(
       gfx::GpuMemoryBufferId id,
       const gfx::Size& size,
@@ -42,6 +44,20 @@ class GPU_IPC_SERVICE_EXPORT GpuMemoryBufferFactory {
       int client_id,
       SurfaceHandle surface_handle) = 0;
 
+  // Same as above, but returns the result asynchrounously. Safe to use on the
+  // IO thread. |callback| will be called on the same thread that calls this
+  // method, and it might be called on the same call stack.
+  using CreateGpuMemoryBufferAsyncCallback =
+      base::OnceCallback<void(gfx::GpuMemoryBufferHandle)>;
+  virtual void CreateGpuMemoryBufferAsync(
+      gfx::GpuMemoryBufferId id,
+      const gfx::Size& size,
+      gfx::BufferFormat format,
+      gfx::BufferUsage usage,
+      int client_id,
+      SurfaceHandle surface_handle,
+      CreateGpuMemoryBufferAsyncCallback callback);
+
   // Destroys GPU memory buffer identified by |id|. It can be called on any
   // thread.
   virtual void DestroyGpuMemoryBuffer(gfx::GpuMemoryBufferId id,

gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.cc

@@ -23,7 +23,7 @@
 namespace gpu {
 
 GpuMemoryBufferFactoryNativePixmap::GpuMemoryBufferFactoryNativePixmap()
-    : vulkan_context_provider_(nullptr) {}
+    : GpuMemoryBufferFactoryNativePixmap(nullptr) {}
 
 GpuMemoryBufferFactoryNativePixmap::GpuMemoryBufferFactoryNativePixmap(
     viz::VulkanContextProvider* vulkan_context_provider)
@@ -46,33 +46,37 @@ GpuMemoryBufferFactoryNativePixmap::CreateGpuMemoryBuffer(
           ->GetSurfaceFactoryOzone()
           ->CreateNativePixmap(surface_handle, GetVulkanDevice(), size, format,
                                usage);
-  if (!pixmap.get()) {
-    DLOG(ERROR) << "Failed to create pixmap " << size.ToString() << ",  "
-                << gfx::BufferFormatToString(format) << ", usage "
-                << static_cast<int>(usage);
-    return gfx::GpuMemoryBufferHandle();
-  }
-
-  gfx::GpuMemoryBufferHandle new_handle;
-  new_handle.type = gfx::NATIVE_PIXMAP;
-  new_handle.id = id;
-  new_handle.native_pixmap_handle = pixmap->ExportHandle();
-
-  // TODO(reveman): Remove this once crbug.com/628334 has been fixed.
-  {
-    base::AutoLock lock(native_pixmaps_lock_);
-    NativePixmapMapKey key(id.id, client_id);
-    DCHECK(native_pixmaps_.find(key) == native_pixmaps_.end());
-    native_pixmaps_[key] = pixmap;
-  }
-
-  return new_handle;
+  return CreateGpuMemoryBufferFromNativePixmap(id, size, format, usage,
+                                               client_id, std::move(pixmap));
 #else
   NOTIMPLEMENTED();
   return gfx::GpuMemoryBufferHandle();
 #endif
 }
 
+void GpuMemoryBufferFactoryNativePixmap::CreateGpuMemoryBufferAsync(
+    gfx::GpuMemoryBufferId id,
+    const gfx::Size& size,
+    gfx::BufferFormat format,
+    gfx::BufferUsage usage,
+    int client_id,
+    SurfaceHandle surface_handle,
+    CreateGpuMemoryBufferAsyncCallback callback) {
+#if defined(USE_OZONE)
+  ui::OzonePlatform::GetInstance()
+      ->GetSurfaceFactoryOzone()
+      ->CreateNativePixmapAsync(
+          surface_handle, GetVulkanDevice(), size, format, usage,
+          base::BindOnce(
+              &GpuMemoryBufferFactoryNativePixmap::OnNativePixmapCreated, id,
+              size, format, usage, client_id, std::move(callback),
+              weak_factory_.GetWeakPtr()));
+#else
+  NOTIMPLEMENTED();
+  std::move(callback).Run(gfx::GpuMemoryBufferHandle());
+#endif
+}
+
 void GpuMemoryBufferFactoryNativePixmap::DestroyGpuMemoryBuffer(
     gfx::GpuMemoryBufferId id,
     int client_id) {
@@ -183,4 +187,53 @@ VkDevice GpuMemoryBufferFactoryNativePixmap::GetVulkanDevice() {
              : VK_NULL_HANDLE;
 }
 
+// static
+void GpuMemoryBufferFactoryNativePixmap::OnNativePixmapCreated(
+    gfx::GpuMemoryBufferId id,
+    const gfx::Size& size,
+    gfx::BufferFormat format,
+    gfx::BufferUsage usage,
+    int client_id,
+    CreateGpuMemoryBufferAsyncCallback callback,
+    base::WeakPtr<GpuMemoryBufferFactoryNativePixmap> weak_ptr,
+    scoped_refptr<gfx::NativePixmap> pixmap) {
+  if (weak_ptr) {
+    std::move(callback).Run(weak_ptr->CreateGpuMemoryBufferFromNativePixmap(
+        id, size, format, usage, client_id, pixmap));
+  } else {
+    std::move(callback).Run(gfx::GpuMemoryBufferHandle());
+  }
+}
+
+gfx::GpuMemoryBufferHandle
+GpuMemoryBufferFactoryNativePixmap::CreateGpuMemoryBufferFromNativePixmap(
+    gfx::GpuMemoryBufferId id,
+    const gfx::Size& size,
+    gfx::BufferFormat format,
+    gfx::BufferUsage usage,
+    int client_id,
+    scoped_refptr<gfx::NativePixmap> pixmap) {
+  if (!pixmap.get()) {
+    DLOG(ERROR) << "Failed to create pixmap " << size.ToString() << ",  "
+                << gfx::BufferFormatToString(format) << ", usage "
+                << static_cast<int>(usage);
+    return gfx::GpuMemoryBufferHandle();
+  }
+
+  gfx::GpuMemoryBufferHandle new_handle;
+  new_handle.type = gfx::NATIVE_PIXMAP;
+  new_handle.id = id;
+  new_handle.native_pixmap_handle = pixmap->ExportHandle();
+
+  // TODO(reveman): Remove this once crbug.com/628334 has been fixed.
+  {
+    base::AutoLock lock(native_pixmaps_lock_);
+    NativePixmapMapKey key(id.id, client_id);
+    DCHECK(native_pixmaps_.find(key) == native_pixmaps_.end());
+    native_pixmaps_[key] = pixmap;
+  }
+
+  return new_handle;
+}
+
 }  // namespace gpu

gpu/ipc/service/gpu_memory_buffer_factory_native_pixmap.h

@@ -41,6 +41,14 @@ class GPU_IPC_SERVICE_EXPORT GpuMemoryBufferFactoryNativePixmap
       gfx::BufferUsage usage,
       int client_id,
       SurfaceHandle surface_handle) override;
+  void CreateGpuMemoryBufferAsync(
+      gfx::GpuMemoryBufferId id,
+      const gfx::Size& size,
+      gfx::BufferFormat format,
+      gfx::BufferUsage usage,
+      int client_id,
+      SurfaceHandle surface_handle,
+      CreateGpuMemoryBufferAsyncCallback callback) override;
   void DestroyGpuMemoryBuffer(gfx::GpuMemoryBufferId id,
                               int client_id) override;
   ImageFactory* AsImageFactory() override;
@@ -66,6 +74,24 @@ class GPU_IPC_SERVICE_EXPORT GpuMemoryBufferFactoryNativePixmap
                                              scoped_refptr<gfx::NativePixmap>,
                                              NativePixmapMapKeyHash>;
 
+  static void OnNativePixmapCreated(
+      gfx::GpuMemoryBufferId id,
+      const gfx::Size& size,
+      gfx::BufferFormat format,
+      gfx::BufferUsage usage,
+      int client_id,
+      CreateGpuMemoryBufferAsyncCallback callback,
+      base::WeakPtr<GpuMemoryBufferFactoryNativePixmap> weak_ptr,
+      scoped_refptr<gfx::NativePixmap> pixmap);
+
+  gfx::GpuMemoryBufferHandle CreateGpuMemoryBufferFromNativePixmap(
+      gfx::GpuMemoryBufferId id,
+      const gfx::Size& size,
+      gfx::BufferFormat format,
+      gfx::BufferUsage usage,
+      int client_id,
+      scoped_refptr<gfx::NativePixmap> pixmap);
+
   VkDevice GetVulkanDevice();
 
   scoped_refptr<viz::VulkanContextProvider> vulkan_context_provider_;
@@ -73,6 +99,8 @@ class GPU_IPC_SERVICE_EXPORT GpuMemoryBufferFactoryNativePixmap
   NativePixmapMap native_pixmaps_;
   base::Lock native_pixmaps_lock_;
 
+  base::WeakPtrFactory<GpuMemoryBufferFactoryNativePixmap> weak_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(GpuMemoryBufferFactoryNativePixmap);
 };
 

ui/ozone/platform/drm/gpu/drm_thread.cc

@@ -181,6 +181,19 @@ void DrmThread::CreateBuffer(gfx::AcceleratedWidget widget,
   }
 }
 
+void DrmThread::CreateBufferAsync(gfx::AcceleratedWidget widget,
+                                  const gfx::Size& size,
+                                  gfx::BufferFormat format,
+                                  gfx::BufferUsage usage,
+                                  uint32_t client_flags,
+                                  CreateBufferAsyncCallback callback) {
+  std::unique_ptr<GbmBuffer> buffer;
+  scoped_refptr<DrmFramebuffer> framebuffer;
+  CreateBuffer(widget, size, format, usage, client_flags, &buffer,
+               &framebuffer);
+  std::move(callback).Run(std::move(buffer), std::move(framebuffer));
+}
+
 void DrmThread::CreateBufferFromHandle(
     gfx::AcceleratedWidget widget,
     const gfx::Size& size,

ui/ozone/platform/drm/gpu/drm_thread.h

@@ -72,6 +72,15 @@ class DrmThread : public base::Thread,
                     uint32_t flags,
                     std::unique_ptr<GbmBuffer>* buffer,
                     scoped_refptr<DrmFramebuffer>* framebuffer);
+  using CreateBufferAsyncCallback =
+      base::OnceCallback<void(std::unique_ptr<GbmBuffer>,
+                              scoped_refptr<DrmFramebuffer>)>;
+  void CreateBufferAsync(gfx::AcceleratedWidget widget,
+                         const gfx::Size& size,
+                         gfx::BufferFormat format,
+                         gfx::BufferUsage usage,
+                         uint32_t flags,
+                         CreateBufferAsyncCallback callback);
   void CreateBufferFromHandle(gfx::AcceleratedWidget widget,
                               const gfx::Size& size,
                               gfx::BufferFormat format,

ui/ozone/platform/drm/gpu/drm_thread_proxy.cc

@@ -15,6 +15,20 @@
 
 namespace ui {
 
+namespace {
+
+void OnBufferCreatedOnDrmThread(
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner,
+    DrmThreadProxy::CreateBufferAsyncCallback callback,
+    std::unique_ptr<GbmBuffer> buffer,
+    scoped_refptr<DrmFramebuffer> framebuffer) {
+  task_runner->PostTask(FROM_HERE,
+                        base::BindOnce(std::move(callback), std::move(buffer),
+                                       std::move(framebuffer)));
+}
+
+}  // namespace
+
 DrmThreadProxy::DrmThreadProxy() = default;
 
 DrmThreadProxy::~DrmThreadProxy() = default;
@@ -49,6 +63,24 @@ void DrmThreadProxy::CreateBuffer(gfx::AcceleratedWidget widget,
                      widget, size, format, usage, flags, buffer, framebuffer));
 }
 
+void DrmThreadProxy::CreateBufferAsync(gfx::AcceleratedWidget widget,
+                                       const gfx::Size& size,
+                                       gfx::BufferFormat format,
+                                       gfx::BufferUsage usage,
+                                       uint32_t flags,
+                                       CreateBufferAsyncCallback callback) {
+  DCHECK(drm_thread_.task_runner())
+      << "no task runner! in DrmThreadProxy::CreateBufferAsync";
+  drm_thread_.task_runner()->PostTask(
+      FROM_HERE,
+      base::BindOnce(&DrmThread::CreateBufferAsync,
+                     base::Unretained(&drm_thread_), widget, size, format,
+                     usage, flags,
+                     base::BindOnce(OnBufferCreatedOnDrmThread,
+                                    base::ThreadTaskRunnerHandle::Get(),
+                                    std::move(callback))));
+}
+
 void DrmThreadProxy::CreateBufferFromHandle(
     gfx::AcceleratedWidget widget,
     const gfx::Size& size,

ui/ozone/platform/drm/gpu/drm_thread_proxy.h

@@ -48,6 +48,16 @@ class DrmThreadProxy {
                     std::unique_ptr<GbmBuffer>* buffer,
                     scoped_refptr<DrmFramebuffer>* framebuffer);
 
+  using CreateBufferAsyncCallback =
+      base::OnceCallback<void(std::unique_ptr<GbmBuffer>,
+                              scoped_refptr<DrmFramebuffer>)>;
+  void CreateBufferAsync(gfx::AcceleratedWidget widget,
+                         const gfx::Size& size,
+                         gfx::BufferFormat format,
+                         gfx::BufferUsage usage,
+                         uint32_t flags,
+                         CreateBufferAsyncCallback callback);
+
   void CreateBufferFromHandle(gfx::AcceleratedWidget widget,
                               const gfx::Size& size,
                               gfx::BufferFormat format,

ui/ozone/platform/drm/gpu/gbm_surface_factory.cc

@@ -135,6 +135,18 @@ std::vector<gfx::BufferFormat> EnumerateSupportedBufferFormatsForTexturing() {
   return supported_buffer_formats;
 }
 
+void OnNativePixmapCreated(GbmSurfaceFactory::NativePixmapCallback callback,
+                           base::WeakPtr<GbmSurfaceFactory> weak_ptr,
+                           std::unique_ptr<GbmBuffer> buffer,
+                           scoped_refptr<DrmFramebuffer> framebuffer) {
+  if (!weak_ptr || !buffer) {
+    std::move(callback).Run(nullptr);
+  } else {
+    std::move(callback).Run(base::MakeRefCounted<GbmPixmap>(
+        weak_ptr.get(), std::move(buffer), std::move(framebuffer)));
+  }
+}
+
 }  // namespace
 
 GbmSurfaceFactory::GbmSurfaceFactory(DrmThreadProxy* drm_thread_proxy)
@@ -281,6 +293,18 @@ scoped_refptr<gfx::NativePixmap> GbmSurfaceFactory::CreateNativePixmap(
                                          std::move(framebuffer));
 }
 
+void GbmSurfaceFactory::CreateNativePixmapAsync(gfx::AcceleratedWidget widget,
+                                                VkDevice vk_device,
+                                                gfx::Size size,
+                                                gfx::BufferFormat format,
+                                                gfx::BufferUsage usage,
+                                                NativePixmapCallback callback) {
+  drm_thread_proxy_->CreateBufferAsync(
+      widget, size, format, usage, 0 /* flags */,
+      base::BindOnce(OnNativePixmapCreated, std::move(callback),
+                     weak_factory_.GetWeakPtr()));
+}
+
 scoped_refptr<gfx::NativePixmap>
 GbmSurfaceFactory::CreateNativePixmapFromHandleInternal(
     gfx::AcceleratedWidget widget,

ui/ozone/platform/drm/gpu/gbm_surface_factory.h

@@ -60,6 +60,12 @@ class GbmSurfaceFactory : public SurfaceFactoryOzone {
       gfx::Size size,
       gfx::BufferFormat format,
       gfx::BufferUsage usage) override;
+  void CreateNativePixmapAsync(gfx::AcceleratedWidget widget,
+                               VkDevice vk_device,
+                               gfx::Size size,
+                               gfx::BufferFormat format,
+                               gfx::BufferUsage usage,
+                               NativePixmapCallback callback) override;
   scoped_refptr<gfx::NativePixmap> CreateNativePixmapFromHandle(
       gfx::AcceleratedWidget widget,
       gfx::Size size,
@@ -94,6 +100,8 @@ class GbmSurfaceFactory : public SurfaceFactoryOzone {
 
   GetProtectedNativePixmapCallback get_protected_native_pixmap_callback_;
 
+  base::WeakPtrFactory<GbmSurfaceFactory> weak_factory_{this};
+
   DISALLOW_COPY_AND_ASSIGN(GbmSurfaceFactory);
 };
 

ui/ozone/public/surface_factory_ozone.cc

@@ -78,6 +78,16 @@ scoped_refptr<gfx::NativePixmap> SurfaceFactoryOzone::CreateNativePixmap(
   return nullptr;
 }
 
+void SurfaceFactoryOzone::CreateNativePixmapAsync(
+    gfx::AcceleratedWidget widget,
+    VkDevice vk_device,
+    gfx::Size size,
+    gfx::BufferFormat format,
+    gfx::BufferUsage usage,
+    NativePixmapCallback callback) {
+  std::move(callback).Run(nullptr);
+}
+
 scoped_refptr<gfx::NativePixmap>
 SurfaceFactoryOzone::CreateNativePixmapFromHandle(
     gfx::AcceleratedWidget widget,

ui/ozone/public/surface_factory_ozone.h

@@ -121,6 +121,16 @@ class COMPONENT_EXPORT(OZONE_BASE) SurfaceFactoryOzone {
       gfx::BufferFormat format,
       gfx::BufferUsage usage);
 
+  // Similar to CreateNativePixmap, but returns the result asynchronously.
+  using NativePixmapCallback =
+      base::OnceCallback<void(scoped_refptr<gfx::NativePixmap>)>;
+  virtual void CreateNativePixmapAsync(gfx::AcceleratedWidget widget,
+                                       VkDevice vk_device,
+                                       gfx::Size size,
+                                       gfx::BufferFormat format,
+                                       gfx::BufferUsage usage,
+                                       NativePixmapCallback callback);
+
   // Create a single native buffer from an existing handle. Takes ownership of
   // |handle| and can be called on any thread.
   virtual scoped_refptr<gfx::NativePixmap> CreateNativePixmapFromHandle(