ui/gl [android]: Update Android NDK ABI for Surface Control.

The notable changes from using the updated ABI are the use of read
fences returned by the framework to synchronize release of resources in
transaction acks.

The current code has some shortcomings in terms of potentially reading
the media buffers from GL (display compositor) and/or the framework
(system compositor). We currently always bind every new buffer to a
texture with an EGL fence wait, which is unnecessary if the buffer will
be passed to the system compositor. On the other end, we always and only
generate a read fence from GL that the AImageReader_deleteasync uses if
the current image has no external refs when a new image is acquired.
Ideally we should wait on read fences from GL and the framework if the
buffer was read by both (possible since the overlay decision could change
every frame), and only generate a read fence from GL if the buffer was
used by the display compositor.

All the above fence synchronization is meant to be correctly handled
using SharedImages, which can handle a buffer being imported by different
consumers and generating/synchronizing read fences bases on which API the
buffer is read by. Ensuring that these cases are handled correctly is left
until media is switched to Shared Images.

TBR=nyquist@chromium.org
R=liberato@chromium.org, piman@chromium.org

Bug: 889328
Change-Id: Ic9f49420540249c06943fd62f6fcba8830c6ccac
Reviewed-on: https://chromium-review.googlesource.com/c/1434918
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Auto-Submit: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626799}

base/android/scoped_hardware_buffer_fence_sync.h

@@ -26,6 +26,12 @@ class BASE_EXPORT ScopedHardwareBufferFenceSync {
   ScopedHardwareBufferHandle TakeBuffer();
   ScopedFD TakeFence();
 
+  // Provides fence which is signaled when the reads for this buffer are done
+  // and it can be reused. The method assumes a current GLContext and will only
+  // synchronize the reads with this context.
+  // Must only be called once.
+  virtual void SetReadFence(base::ScopedFD fence_fd) = 0;
+
  private:
   ScopedHardwareBufferHandle handle_;
   ScopedFD fence_fd_;

media/gpu/android/image_reader_gl_owner.cc

@@ -60,10 +60,16 @@ class ImageReaderGLOwner::ScopedHardwareBufferImpl
         texture_owner_(std::move(texture_owner)),
         image_(image) {}
   ~ScopedHardwareBufferImpl() override {
-    texture_owner_->ReleaseRefOnImage(image_);
+    texture_owner_->ReleaseRefOnImage(image_, std::move(read_fence_));
+  }
+
+  void SetReadFence(base::ScopedFD fence_fd) final {
+    DCHECK(!read_fence_.is_valid());
+    read_fence_ = std::move(fence_fd);
   }
 
  private:
+  base::ScopedFD read_fence_;
   scoped_refptr<ImageReaderGLOwner> texture_owner_;
   AImage* image_;
 };
@@ -292,35 +298,42 @@ ImageReaderGLOwner::GetAHardwareBuffer() {
   auto fence_fd = base::ScopedFD(HANDLE_EINTR(dup(current_image_fence_.get())));
 
   // Add a ref that the caller will release.
-  auto it = external_image_refs_.find(current_image_);
-  if (it == external_image_refs_.end())
-    external_image_refs_[current_image_] = 1;
-  else
-    it->second++;
-
+  external_image_refs_[current_image_].count++;
   return std::make_unique<ScopedHardwareBufferImpl>(
       this, current_image_,
       base::android::ScopedHardwareBufferHandle::Create(buffer),
       std::move(fence_fd));
 }
 
-void ImageReaderGLOwner::ReleaseRefOnImage(AImage* image) {
+void ImageReaderGLOwner::ReleaseRefOnImage(AImage* image,
+                                           base::ScopedFD fence_fd) {
   auto it = external_image_refs_.find(image);
   DCHECK(it != external_image_refs_.end());
-  DCHECK_GT(it->second, 0u);
-  it->second--;
 
-  if (it->second > 0)
-    return;
-  external_image_refs_.erase(it);
+  auto& image_ref = it->second;
+  DCHECK_GT(image_ref.count, 0u);
+  image_ref.count--;
+
+  // TODO(khushalsagar): We should probably merge this fence with any
+  // pre-existing fence, and there are also a couple of other cases that are
+  // being ignored here (delete image async if it is the |current_image| using
+  // this fence, combining display compositor fence with the |fence_fd| here).
+  // But all of this is going to be automagically fixed with SharedImages, so
+  // need to do the proper thing once media switches to that.
+  image_ref.fence_fd = std::move(fence_fd);
 
-  if (image == current_image_)
+  if (image_ref.count > 0)
     return;
 
-  // No refs on the image. If it is no longer current, delete it. Note that this
-  // can be deleted synchronously here since the caller ensures that any pending
-  // GPU work for the image is finished before marking it for release.
-  loader_.AImage_delete(image);
+  // Delete the image if it has no pending refs and it is not the current image.
+  if (image != current_image_) {
+    if (image_ref.fence_fd.is_valid())
+      loader_.AImage_deleteAsync(image, image_ref.fence_fd.release());
+    else
+      loader_.AImage_delete(image);
+  }
+
+  external_image_refs_.erase(it);
 }
 
 void ImageReaderGLOwner::GetTransformMatrix(float mtx[]) {
@@ -397,4 +410,10 @@ void ImageReaderGLOwner::WaitForFrameAvailable() {
   }
 }
 
+ImageReaderGLOwner::ImageRef::ImageRef() = default;
+ImageReaderGLOwner::ImageRef::~ImageRef() = default;
+ImageReaderGLOwner::ImageRef::ImageRef(ImageRef&& other) = default;
+ImageReaderGLOwner::ImageRef& ImageReaderGLOwner::ImageRef::operator=(
+    ImageRef&& other) = default;
+
 }  // namespace media

media/gpu/android/image_reader_gl_owner.h

@@ -63,7 +63,10 @@ class MEDIA_GPU_EXPORT ImageReaderGLOwner : public TextureOwner {
   bool MaybeDeleteCurrentImage();
 
   void EnsureTexImageBound();
-  void ReleaseRefOnImage(AImage* image);
+
+  // Releases an external ref on the image, with the fence that must be signaled
+  // before the |image| can be resued by the AImageReader.
+  void ReleaseRefOnImage(AImage* image, base::ScopedFD fence_fd);
 
   // AImageReader instance
   AImageReader* image_reader_;
@@ -80,7 +83,19 @@ class MEDIA_GPU_EXPORT ImageReaderGLOwner : public TextureOwner {
   // A map consisting of pending external refs on an AImage. If an image has any
   // external refs, it is automatically released once the ref-count is 0 and the
   // image is no longer current.
-  using AImageRefMap = base::flat_map<AImage*, size_t>;
+  struct ImageRef {
+    ImageRef();
+    ~ImageRef();
+
+    ImageRef(ImageRef&& other);
+    ImageRef& operator=(ImageRef&& other);
+
+    size_t count = 0u;
+    base::ScopedFD fence_fd;
+
+    DISALLOW_COPY_AND_ASSIGN(ImageRef);
+  };
+  using AImageRefMap = base::flat_map<AImage*, ImageRef>;
   AImageRefMap external_image_refs_;
 
   // reference to the class instance which is used to dynamically

ui/gl/android/android_surface_control_compat.cc

@@ -7,16 +7,21 @@
 #include <dlfcn.h>
 
 #include "base/android/build_info.h"
+#include "base/bind.h"
 #include "base/memory/ptr_util.h"
 #include "base/no_destructor.h"
 
 extern "C" {
+typedef struct ASurfaceTransactionStats ASurfaceTransactionStats;
+typedef void (*ASurfaceTransaction_OnComplete)(void* context,
+                                               ASurfaceTransactionStats* stats);
+
 // ASurface
 using pASurfaceControl_createFromWindow =
     ASurfaceControl* (*)(ANativeWindow* parent, const char* name);
 using pASurfaceControl_create = ASurfaceControl* (*)(ASurfaceControl* parent,
                                                      const char* name);
-using pASurfaceControl_destroy = void (*)(ASurfaceControl*);
+using pASurfaceControl_release = void (*)(ASurfaceControl*);
 
 // ASurfaceTransaction enums
 enum {
@@ -61,6 +66,18 @@ using pASurfaceTransaction_setDamageRegion =
              ASurfaceControl* surface,
              const ARect rects[],
              uint32_t count);
+
+// ASurfaceTransactionStats
+using pASurfaceTransactionStats_getPresentFenceFd =
+    int (*)(ASurfaceTransactionStats* stats);
+using pASurfaceTransactionStats_getASurfaceControls =
+    void (*)(ASurfaceTransactionStats* stats,
+             ASurfaceControl*** surface_controls,
+             size_t* size);
+using pASurfaceTransactionStats_releaseASurfaceControls =
+    void (*)(ASurfaceControl** surface_controls);
+using pASurfaceTransactionStats_getPreviousReleaseFenceFd =
+    int (*)(ASurfaceTransactionStats* stats, ASurfaceControl* surface_control);
 }
 
 namespace gl {
@@ -92,7 +109,7 @@ struct SurfaceControlMethods {
 
     LOAD_FUNCTION(main_dl_handle, ASurfaceControl_createFromWindow);
     LOAD_FUNCTION(main_dl_handle, ASurfaceControl_create);
-    LOAD_FUNCTION(main_dl_handle, ASurfaceControl_destroy);
+    LOAD_FUNCTION(main_dl_handle, ASurfaceControl_release);
 
     LOAD_FUNCTION(main_dl_handle, ASurfaceTransaction_create);
     LOAD_FUNCTION(main_dl_handle, ASurfaceTransaction_delete);
@@ -104,6 +121,13 @@ struct SurfaceControlMethods {
     LOAD_FUNCTION(main_dl_handle, ASurfaceTransaction_setGeometry);
     LOAD_FUNCTION(main_dl_handle, ASurfaceTransaction_setBufferTransparency);
     LOAD_FUNCTION(main_dl_handle, ASurfaceTransaction_setDamageRegion);
+
+    LOAD_FUNCTION(main_dl_handle, ASurfaceTransactionStats_getPresentFenceFd);
+    LOAD_FUNCTION(main_dl_handle, ASurfaceTransactionStats_getASurfaceControls);
+    LOAD_FUNCTION(main_dl_handle,
+                  ASurfaceTransactionStats_releaseASurfaceControls);
+    LOAD_FUNCTION(main_dl_handle,
+                  ASurfaceTransactionStats_getPreviousReleaseFenceFd);
   }
 
   ~SurfaceControlMethods() = default;
@@ -112,7 +136,7 @@ struct SurfaceControlMethods {
   // Surface methods.
   pASurfaceControl_createFromWindow ASurfaceControl_createFromWindowFn;
   pASurfaceControl_create ASurfaceControl_createFn;
-  pASurfaceControl_destroy ASurfaceControl_destroyFn;
+  pASurfaceControl_release ASurfaceControl_releaseFn;
 
   // Transaction methods.
   pASurfaceTransaction_create ASurfaceTransaction_createFn;
@@ -126,6 +150,16 @@ struct SurfaceControlMethods {
   pASurfaceTransaction_setBufferTransparency
       ASurfaceTransaction_setBufferTransparencyFn;
   pASurfaceTransaction_setDamageRegion ASurfaceTransaction_setDamageRegionFn;
+
+  // TransactionStats methods.
+  pASurfaceTransactionStats_getPresentFenceFd
+      ASurfaceTransactionStats_getPresentFenceFdFn;
+  pASurfaceTransactionStats_getASurfaceControls
+      ASurfaceTransactionStats_getASurfaceControlsFn;
+  pASurfaceTransactionStats_releaseASurfaceControls
+      ASurfaceTransactionStats_releaseASurfaceControlsFn;
+  pASurfaceTransactionStats_getPreviousReleaseFenceFd
+      ASurfaceTransactionStats_getPreviousReleaseFenceFdFn;
 };
 
 ARect RectToARect(const gfx::Rect& rect) {
@@ -153,6 +187,56 @@ int32_t OverlayTransformToWindowTransform(gfx::OverlayTransform transform) {
   NOTREACHED();
   return ANATIVEWINDOW_TRANSFORM_IDENTITY;
 }
+
+SurfaceControl::TransactionStats ToTransactionStats(
+    ASurfaceTransactionStats* stats) {
+  SurfaceControl::TransactionStats transaction_stats;
+  transaction_stats.present_fence = base::ScopedFD(
+      SurfaceControlMethods::Get().ASurfaceTransactionStats_getPresentFenceFdFn(
+          stats));
+
+  ASurfaceControl** surface_controls = nullptr;
+  size_t size = 0u;
+  SurfaceControlMethods::Get().ASurfaceTransactionStats_getASurfaceControlsFn(
+      stats, &surface_controls, &size);
+  transaction_stats.surface_stats.resize(size);
+  for (size_t i = 0u; i < size; ++i) {
+    transaction_stats.surface_stats[i].surface = surface_controls[i];
+    int fence_fd = SurfaceControlMethods::Get()
+                       .ASurfaceTransactionStats_getPreviousReleaseFenceFdFn(
+                           stats, surface_controls[i]);
+    if (fence_fd != -1) {
+      transaction_stats.surface_stats[i].fence = base::ScopedFD(fence_fd);
+    }
+  }
+  SurfaceControlMethods::Get()
+      .ASurfaceTransactionStats_releaseASurfaceControlsFn(surface_controls);
+
+  return transaction_stats;
+}
+
+struct TransactionAckCtx {
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner;
+  SurfaceControl::Transaction::OnCompleteCb callback;
+};
+
+// Note that the framework API states that this callback can be dispatched on
+// any thread (in practice it should be the binder thread).
+void OnTransactionCompletedOnAnyThread(void* context,
+                                       ASurfaceTransactionStats* stats) {
+  auto* ack_ctx = static_cast<TransactionAckCtx*>(context);
+  auto transaction_stats = ToTransactionStats(stats);
+
+  if (ack_ctx->task_runner) {
+    ack_ctx->task_runner->PostTask(
+        FROM_HERE, base::BindOnce(std::move(ack_ctx->callback),
+                                  std::move(transaction_stats)));
+  } else {
+    std::move(ack_ctx->callback).Run(std::move(transaction_stats));
+  }
+
+  delete ack_ctx;
+}
 };
 
 // static
@@ -185,22 +269,23 @@ SurfaceControl::Surface::Surface(ANativeWindow* parent, const char* name) {
 
 SurfaceControl::Surface::~Surface() {
   if (surface_)
-    SurfaceControlMethods::Get().ASurfaceControl_destroyFn(surface_);
+    SurfaceControlMethods::Get().ASurfaceControl_releaseFn(surface_);
 }
 
-SurfaceControl::Surface::Surface(Surface&& other) {
-  surface_ = other.surface_;
-  other.surface_ = nullptr;
-}
+SurfaceControl::SurfaceStats::SurfaceStats() = default;
+SurfaceControl::SurfaceStats::~SurfaceStats() = default;
 
-SurfaceControl::Surface& SurfaceControl::Surface::operator=(Surface&& other) {
-  if (surface_)
-    SurfaceControlMethods::Get().ASurfaceControl_destroyFn(surface_);
+SurfaceControl::SurfaceStats::SurfaceStats(SurfaceStats&& other) = default;
+SurfaceControl::SurfaceStats& SurfaceControl::SurfaceStats::operator=(
+    SurfaceStats&& other) = default;
 
-  surface_ = other.surface_;
-  other.surface_ = nullptr;
-  return *this;
-}
+SurfaceControl::TransactionStats::TransactionStats() = default;
+SurfaceControl::TransactionStats::~TransactionStats() = default;
+
+SurfaceControl::TransactionStats::TransactionStats(TransactionStats&& other) =
+    default;
+SurfaceControl::TransactionStats& SurfaceControl::TransactionStats::operator=(
+    TransactionStats&& other) = default;
 
 SurfaceControl::Transaction::Transaction() {
   transaction_ = SurfaceControlMethods::Get().ASurfaceTransaction_createFn();
@@ -254,11 +339,15 @@ void SurfaceControl::Transaction::SetDamageRect(const Surface& surface,
       transaction_, surface.surface(), &a_rect, 1u);
 }
 
-void SurfaceControl::Transaction::SetOnCompleteFunc(
-    ASurfaceTransaction_OnComplete func,
-    void* ctx) {
-  SurfaceControlMethods::Get().ASurfaceTransaction_setOnCompleteFn(transaction_,
-                                                                   ctx, func);
+void SurfaceControl::Transaction::SetOnCompleteCb(
+    OnCompleteCb cb,
+    scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
+  TransactionAckCtx* ack_ctx = new TransactionAckCtx;
+  ack_ctx->callback = std::move(cb);
+  ack_ctx->task_runner = std::move(task_runner);
+
+  SurfaceControlMethods::Get().ASurfaceTransaction_setOnCompleteFn(
+      transaction_, ack_ctx, &OnTransactionCompletedOnAnyThread);
 }
 
 void SurfaceControl::Transaction::Apply() {

ui/gl/android/android_surface_control_compat.h

@@ -11,6 +11,8 @@
 #include <android/native_window.h>
 
 #include "base/files/scoped_file.h"
+#include "base/memory/ref_counted.h"
+#include "base/single_thread_task_runner.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/overlay_transform.h"
 #include "ui/gl/gl_export.h"
@@ -18,8 +20,6 @@
 extern "C" {
 typedef struct ASurfaceControl ASurfaceControl;
 typedef struct ASurfaceTransaction ASurfaceTransaction;
-typedef void (*ASurfaceTransaction_OnComplete)(void* context,
-                                               int32_t present_fence);
 }
 
 namespace gl {
@@ -28,20 +28,52 @@ class GL_EXPORT SurfaceControl {
  public:
   static bool IsSupported();
 
-  class GL_EXPORT Surface {
+  class GL_EXPORT Surface : public base::RefCounted<Surface> {
    public:
     Surface();
     Surface(const Surface& parent, const char* name);
     Surface(ANativeWindow* parent, const char* name);
-    ~Surface();
-
-    Surface(Surface&& other);
-    Surface& operator=(Surface&& other);
 
     ASurfaceControl* surface() const { return surface_; }
 
    private:
+    friend class base::RefCounted<Surface>;
+    ~Surface();
+
     ASurfaceControl* surface_ = nullptr;
+
+    DISALLOW_COPY_AND_ASSIGN(Surface);
+  };
+
+  struct GL_EXPORT SurfaceStats {
+    SurfaceStats();
+    ~SurfaceStats();
+
+    SurfaceStats(SurfaceStats&& other);
+    SurfaceStats& operator=(SurfaceStats&& other);
+
+    ASurfaceControl* surface = nullptr;
+
+    // The fence which is signaled when the reads for the previous buffer for
+    // the given |surface| are finished.
+    base::ScopedFD fence;
+  };
+
+  struct GL_EXPORT TransactionStats {
+   public:
+    TransactionStats();
+    ~TransactionStats();
+
+    TransactionStats(TransactionStats&& other);
+    TransactionStats& operator=(TransactionStats&& other);
+
+    // The fence which is signaled when this transaction is presented by the
+    // display.
+    base::ScopedFD present_fence;
+    std::vector<SurfaceStats> surface_stats;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(TransactionStats);
   };
 
   class GL_EXPORT Transaction {
@@ -60,7 +92,16 @@ class GL_EXPORT SurfaceControl {
                      gfx::OverlayTransform transform);
     void SetOpaque(const Surface& surface, bool opaque);
     void SetDamageRect(const Surface& surface, const gfx::Rect& rect);
-    void SetOnCompleteFunc(ASurfaceTransaction_OnComplete func, void* ctx);
+
+    // Sets the callback which will be dispatched when the transaction is acked
+    // by the framework.
+    // |task_runner| provides an optional task runner on which the callback
+    // should be run.
+    using OnCompleteCb = base::OnceCallback<void(TransactionStats stats)>;
+    void SetOnCompleteCb(
+        OnCompleteCb cb,
+        scoped_refptr<base::SingleThreadTaskRunner> task_runner);
+
     void Apply();
 
    private:

ui/gl/gl_image_ahardwarebuffer.cc

@@ -3,14 +3,37 @@
 // found in the LICENSE file.
 
 #include "ui/gl/gl_image_ahardwarebuffer.h"
+
 #include "base/android/android_hardware_buffer_compat.h"
 #include "base/android/scoped_hardware_buffer_fence_sync.h"
-
 #include "ui/gl/gl_bindings.h"
+#include "ui/gl/gl_fence_android_native_fence_sync.h"
 
 namespace gl {
 namespace {
 
+class ScopedHardwareBufferFenceSyncImpl
+    : public base::android::ScopedHardwareBufferFenceSync {
+ public:
+  ScopedHardwareBufferFenceSyncImpl(
+      base::android::ScopedHardwareBufferHandle handle,
+      base::ScopedFD fence_fd)
+      : ScopedHardwareBufferFenceSync(std::move(handle), std::move(fence_fd)) {}
+  ~ScopedHardwareBufferFenceSyncImpl() override = default;
+
+  void SetReadFence(base::ScopedFD fence_fd) override {
+    // Insert a service wait for this fence to ensure any resource reuse is
+    // after it is signaled.
+    gfx::GpuFenceHandle handle;
+    handle.type = gfx::GpuFenceHandleType::kAndroidNativeFenceSync;
+    handle.native_fd =
+        base::FileDescriptor(fence_fd.release(), /*auto_close=*/true);
+    gfx::GpuFence gpu_fence(handle);
+    auto gl_fence = GLFence::CreateFromGpuFence(gpu_fence);
+    gl_fence->ServerWait();
+  }
+};
+
 uint32_t GetBufferFormat(const AHardwareBuffer* buffer) {
   AHardwareBuffer_Desc desc = {};
   base::AndroidHardwareBufferCompat::GetInstance().Describe(buffer, &desc);
@@ -86,7 +109,7 @@ void GLImageAHardwareBuffer::OnMemoryDump(
 
 std::unique_ptr<base::android::ScopedHardwareBufferFenceSync>
 GLImageAHardwareBuffer::GetAHardwareBuffer() {
-  return std::make_unique<base::android::ScopedHardwareBufferFenceSync>(
+  return std::make_unique<ScopedHardwareBufferFenceSyncImpl>(
       base::android::ScopedHardwareBufferHandle::Create(handle_.get()),
       base::ScopedFD());
 }

ui/gl/gl_surface_egl_surface_control.cc

@@ -9,7 +9,7 @@
 #include "base/bind.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "ui/gfx/geometry/rect_conversions.h"
-#include "ui/gl/gl_fence_android_native_fence_sync.h"
+#include "ui/gl/gl_context.h"
 #include "ui/gl/gl_image_ahardwarebuffer.h"
 
 namespace gl {
@@ -24,27 +24,12 @@ gfx::Size GetBufferSize(const AHardwareBuffer* buffer) {
   return gfx::Size(desc.width, desc.height);
 }
 
-struct TransactionAckCtx {
-  scoped_refptr<base::SingleThreadTaskRunner> task_runner;
-  base::OnceCallback<void(int32_t)> callback;
-};
-
-// Note that the framework API states that this callback can be dispatched on
-// any thread (in practice it should be the binder thread), so we need to post
-// a task back to the GPU thread.
-void OnTransactionCompletedOnAnyThread(void* ctx, int32_t present_fence) {
-  auto* ack_ctx = static_cast<TransactionAckCtx*>(ctx);
-  ack_ctx->task_runner->PostTask(
-      FROM_HERE, base::BindOnce(std::move(ack_ctx->callback), present_fence));
-  delete ack_ctx;
-}
-
 }  // namespace
 
 GLSurfaceEGLSurfaceControl::GLSurfaceEGLSurfaceControl(
     ANativeWindow* window,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-    : root_surface_(window, kRootSurfaceName),
+    : root_surface_(new SurfaceControl::Surface(window, kRootSurfaceName)),
       gpu_task_runner_(std::move(task_runner)),
       weak_factory_(this) {}
 
@@ -63,7 +48,7 @@ bool GLSurfaceEGLSurfaceControl::Initialize(GLSurfaceFormat format) {
 void GLSurfaceEGLSurfaceControl::Destroy() {
   pending_transaction_.reset();
   surface_list_.clear();
-  root_surface_ = SurfaceControl::Surface();
+  root_surface_.reset();
 }
 
 bool GLSurfaceEGLSurfaceControl::Resize(const gfx::Size& size,
@@ -116,18 +101,11 @@ void GLSurfaceEGLSurfaceControl::CommitPendingTransaction(
   current_frame_resources_.swap(pending_frame_resources_);
   pending_frame_resources_.clear();
 
-  // Set up the callback to be notified when the frame is presented by the
-  // framework. Note that it is assumed that all GPU/display work for this frame
-  // is finished when the callback is dispatched, and all resources from the
-  // previous frame can be reused.
-  TransactionAckCtx* ack_ctx = new TransactionAckCtx;
-  ack_ctx->task_runner = gpu_task_runner_;
-  ack_ctx->callback =
+  SurfaceControl::Transaction::OnCompleteCb callback =
       base::BindOnce(&GLSurfaceEGLSurfaceControl::OnTransactionAckOnGpuThread,
                      weak_factory_.GetWeakPtr(), completion_callback,
                      present_callback, std::move(resources_to_release));
-  pending_transaction_->SetOnCompleteFunc(&OnTransactionCompletedOnAnyThread,
-                                          ack_ctx);
+  pending_transaction_->SetOnCompleteCb(std::move(callback), gpu_task_runner_);
 
   pending_transaction_->Apply();
   pending_transaction_.reset();
@@ -142,6 +120,7 @@ gfx::Size GLSurfaceEGLSurfaceControl::GetSize() {
 }
 
 bool GLSurfaceEGLSurfaceControl::OnMakeCurrent(GLContext* context) {
+  context_ = context;
   return true;
 }
 
@@ -159,14 +138,14 @@ bool GLSurfaceEGLSurfaceControl::ScheduleOverlayPlane(
   bool uninitialized = false;
   if (pending_surfaces_count_ == surface_list_.size()) {
     uninitialized = true;
-    surface_list_.emplace_back(root_surface_);
+    surface_list_.emplace_back(*root_surface_);
   }
   pending_surfaces_count_++;
   auto& surface_state = surface_list_.at(pending_surfaces_count_ - 1);
 
   if (uninitialized || surface_state.z_order != z_order) {
     surface_state.z_order = z_order;
-    pending_transaction_->SetZOrder(surface_state.surface, z_order);
+    pending_transaction_->SetZOrder(*surface_state.surface, z_order);
   }
 
   AHardwareBuffer* hardware_buffer = nullptr;
@@ -175,7 +154,13 @@ bool GLSurfaceEGLSurfaceControl::ScheduleOverlayPlane(
   if (scoped_hardware_buffer) {
     hardware_buffer = scoped_hardware_buffer->buffer();
     fence_fd = scoped_hardware_buffer->TakeFence();
-    pending_frame_resources_.push_back(std::move(scoped_hardware_buffer));
+
+    auto* a_surface = surface_state.surface->surface();
+    DCHECK_EQ(pending_frame_resources_.count(a_surface), 0u);
+
+    auto& resource_ref = pending_frame_resources_[a_surface];
+    resource_ref.surface = surface_state.surface;
+    resource_ref.scoped_buffer = std::move(scoped_hardware_buffer);
   }
 
   if (uninitialized || surface_state.hardware_buffer != hardware_buffer) {
@@ -188,7 +173,7 @@ bool GLSurfaceEGLSurfaceControl::ScheduleOverlayPlane(
       fence_fd = base::ScopedFD(fence_handle.native_fd.fd);
     }
 
-    pending_transaction_->SetBuffer(surface_state.surface,
+    pending_transaction_->SetBuffer(*surface_state.surface,
                                     surface_state.hardware_buffer,
                                     std::move(fence_fd));
   }
@@ -209,7 +194,7 @@ bool GLSurfaceEGLSurfaceControl::ScheduleOverlayPlane(
       surface_state.src = src;
       surface_state.dst = dst;
       surface_state.transform = transform;
-      pending_transaction_->SetGeometry(surface_state.surface, src, dst,
+      pending_transaction_->SetGeometry(*surface_state.surface, src, dst,
                                         transform);
     }
   }
@@ -217,7 +202,7 @@ bool GLSurfaceEGLSurfaceControl::ScheduleOverlayPlane(
   bool opaque = !enable_blend;
   if (uninitialized || surface_state.opaque != opaque) {
     surface_state.opaque = opaque;
-    pending_transaction_->SetOpaque(surface_state.surface, opaque);
+    pending_transaction_->SetOpaque(*surface_state.surface, opaque);
   }
 
   return true;
@@ -256,34 +241,32 @@ void GLSurfaceEGLSurfaceControl::OnTransactionAckOnGpuThread(
     SwapCompletionCallback completion_callback,
     PresentationCallback presentation_callback,
     ResourceRefs released_resources,
-    int32_t present_fence) {
+    SurfaceControl::TransactionStats transaction_stats) {
   DCHECK(gpu_task_runner_->BelongsToCurrentThread());
-
-  // Insert a service wait for this fence to ensure any resource reuse is after
-  // it is signaled.
-  gfx::GpuFenceHandle handle;
-  handle.type = gfx::GpuFenceHandleType::kAndroidNativeFenceSync;
-  handle.native_fd = base::FileDescriptor(present_fence, /*auto_close=*/true);
-  gfx::GpuFence gpu_fence(handle);
-  // TODO(khushalsagar): But what about vulkan?
-  auto gl_fence = GLFence::CreateFromGpuFence(gpu_fence);
-  gl_fence->ServerWait();
+  context_->MakeCurrent(this);
 
   // The presentation feedback callback must run after swap completion.
   completion_callback.Run(gfx::SwapResult::SWAP_ACK, nullptr);
 
-  // TODO(khushalsagar): Maintain a queue of fences so we poll to see if they
-  // are signaled every frame, and get a signal timestamp to feed into this
+  // TODO(khushalsagar): Maintain a queue of present fences so we poll to see if
+  // they are signaled every frame, and get a signal timestamp to feed into this
   // feedback.
   gfx::PresentationFeedback feedback(base::TimeTicks::Now(), base::TimeDelta(),
                                      0 /* flags */);
   presentation_callback.Run(feedback);
+
+  for (auto& surface_stat : transaction_stats.surface_stats) {
+    auto it = released_resources.find(surface_stat.surface);
+    DCHECK(it != released_resources.end());
+    if (surface_stat.fence.is_valid())
+      it->second.scoped_buffer->SetReadFence(std::move(surface_stat.fence));
+  }
   released_resources.clear();
 }
 
 GLSurfaceEGLSurfaceControl::SurfaceState::SurfaceState(
     const SurfaceControl::Surface& parent)
-    : surface(parent, kChildSurfaceName) {}
+    : surface(new SurfaceControl::Surface(parent, kChildSurfaceName)) {}
 
 GLSurfaceEGLSurfaceControl::SurfaceState::SurfaceState() = default;
 GLSurfaceEGLSurfaceControl::SurfaceState::SurfaceState(SurfaceState&& other) =
@@ -294,4 +277,12 @@ GLSurfaceEGLSurfaceControl::SurfaceState::operator=(SurfaceState&& other) =
 
 GLSurfaceEGLSurfaceControl::SurfaceState::~SurfaceState() = default;
 
+GLSurfaceEGLSurfaceControl::ResourceRef::ResourceRef() = default;
+GLSurfaceEGLSurfaceControl::ResourceRef::~ResourceRef() = default;
+GLSurfaceEGLSurfaceControl::ResourceRef::ResourceRef(ResourceRef&& other) =
+    default;
+GLSurfaceEGLSurfaceControl::ResourceRef&
+GLSurfaceEGLSurfaceControl::ResourceRef::operator=(ResourceRef&& other) =
+    default;
+
 }  // namespace gl

ui/gl/gl_surface_egl_surface_control.h

@@ -9,6 +9,7 @@
 #include <memory>
 
 #include "base/android/scoped_hardware_buffer_handle.h"
+#include "base/containers/flat_map.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "ui/gl/android/android_surface_control_compat.h"
@@ -25,7 +26,7 @@ class ScopedHardwareBufferFenceSync;
 
 namespace gl {
 
-class GL_EXPORT GLSurfaceEGLSurfaceControl : public gl::GLSurfaceEGL {
+class GL_EXPORT GLSurfaceEGLSurfaceControl : public GLSurfaceEGL {
  public:
   explicit GLSurfaceEGLSurfaceControl(
       ANativeWindow* window,
@@ -33,7 +34,7 @@ class GL_EXPORT GLSurfaceEGLSurfaceControl : public gl::GLSurfaceEGL {
 
   // GLSurface implementation.
   int GetBufferCount() const override;
-  bool Initialize(gl::GLSurfaceFormat format) override;
+  bool Initialize(GLSurfaceFormat format) override;
   void Destroy() override;
   bool Resize(const gfx::Size& size,
               float scale_factor,
@@ -50,10 +51,10 @@ class GL_EXPORT GLSurfaceEGLSurfaceControl : public gl::GLSurfaceEGL {
       const SwapCompletionCallback& completion_callback,
       const PresentationCallback& presentation_callback) override;
   gfx::Size GetSize() override;
-  bool OnMakeCurrent(gl::GLContext* context) override;
+  bool OnMakeCurrent(GLContext* context) override;
   bool ScheduleOverlayPlane(int z_order,
                             gfx::OverlayTransform transform,
-                            gl::GLImage* image,
+                            GLImage* image,
                             const gfx::Rect& bounds_rect,
                             const gfx::RectF& crop_rect,
                             bool enable_blend,
@@ -85,11 +86,20 @@ class GL_EXPORT GLSurfaceEGLSurfaceControl : public gl::GLSurfaceEGL {
     gfx::OverlayTransform transform = gfx::OVERLAY_TRANSFORM_NONE;
     bool opaque = true;
 
-    gl::SurfaceControl::Surface surface;
+    scoped_refptr<SurfaceControl::Surface> surface;
   };
 
-  using ResourceRefs = std::vector<
-      std::unique_ptr<base::android::ScopedHardwareBufferFenceSync>>;
+  struct ResourceRef {
+    ResourceRef();
+    ~ResourceRef();
+
+    ResourceRef(ResourceRef&& other);
+    ResourceRef& operator=(ResourceRef&& other);
+
+    scoped_refptr<SurfaceControl::Surface> surface;
+    std::unique_ptr<base::android::ScopedHardwareBufferFenceSync> scoped_buffer;
+  };
+  using ResourceRefs = base::flat_map<ASurfaceControl*, ResourceRef>;
 
   void CommitPendingTransaction(
       const SwapCompletionCallback& completion_callback,
@@ -97,13 +107,14 @@ class GL_EXPORT GLSurfaceEGLSurfaceControl : public gl::GLSurfaceEGL {
 
   // Called on the |gpu_task_runner_| when a transaction is acked by the
   // framework.
-  void OnTransactionAckOnGpuThread(SwapCompletionCallback completion_callback,
-                                   PresentationCallback presentation_callback,
-                                   ResourceRefs released_resources,
-                                   int32_t present_fence);
+  void OnTransactionAckOnGpuThread(
+      SwapCompletionCallback completion_callback,
+      PresentationCallback presentation_callback,
+      ResourceRefs released_resources,
+      SurfaceControl::TransactionStats transaction_stats);
 
   // Holds the surface state changes made since the last call to SwapBuffers.
-  base::Optional<gl::SurfaceControl::Transaction> pending_transaction_;
+  base::Optional<SurfaceControl::Transaction> pending_transaction_;
 
   // The list of Surfaces and the corresponding state. The initial
   // |pending_surfaces_count_| surfaces in this list are surfaces with state
@@ -128,7 +139,10 @@ class GL_EXPORT GLSurfaceEGLSurfaceControl : public gl::GLSurfaceEGL {
 
   // The root surface tied to the ANativeWindow that places the content of this
   // GLSurface in the java view tree.
-  gl::SurfaceControl::Surface root_surface_;
+  scoped_refptr<SurfaceControl::Surface> root_surface_;
+
+  // The last context made current with this surface.
+  scoped_refptr<GLContext> context_;
 
   scoped_refptr<base::SingleThreadTaskRunner> gpu_task_runner_;
   base::WeakPtrFactory<GLSurfaceEGLSurfaceControl> weak_factory_;