Call ASurfaceTransaction_apply() to avoid surface leak

ASurfaceTransaction_setOnComplete and setOnCommit increase reference
count of SurfaceControl in following steps in AOSP.
1. SurfaceComposerClient::Transaction::addTransactionCallback
2. TransactionCompletedListener::addCallbackFunction

In Step2, reference count of SurfaceControl increase and the
count only decrease in TransactionCompletedListener::
onTransactionCompleted(). So if Apply() is not called for the
transaction, the reference count never decrease.
TranasactionCompletedListener is singleton object, so the reference
does not decrease even though the transaction is destroyed.

In T OS, ASurfaceTransaction_setBuffer call setOnComplete internally, so
SurfaceControl will leak if apply() is not called.

This patch make sure to call apply() in ~Transaction()
1. if setBuffer has been called. (T OS bug)
2. if setOnCommit or setOnComplete has been called.

Bug: 1395271
Change-Id: Idbd28915c53e760a7514dc30b6d2ad697f41c43b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4074994
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Commit-Queue: Sangheon Kim <sangheon77.kim@samsung.com>
Reviewed-by: Bo Liu <boliu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084825}

ui/gfx/android/android_surface_control_compat.cc

@@ -631,31 +631,45 @@ SurfaceControl::Transaction::Transaction()
 }
 
 SurfaceControl::Transaction::~Transaction() {
-  if (transaction_)
-    SurfaceControlMethods::Get().ASurfaceTransaction_deleteFn(transaction_);
+  DestroyIfNeeded();
+}
+
+void SurfaceControl::Transaction::DestroyIfNeeded() {
+  if (!transaction_)
+    return;
+  if (need_to_apply_)
+    SurfaceControlMethods::Get().ASurfaceTransaction_applyFn(transaction_);
+  SurfaceControlMethods::Get().ASurfaceTransaction_deleteFn(transaction_);
+  transaction_ = nullptr;
 }
 
 SurfaceControl::Transaction::Transaction(Transaction&& other)
     : id_(other.id_),
       transaction_(other.transaction_),
       on_commit_cb_(std::move(other.on_commit_cb_)),
-      on_complete_cb_(std::move(other.on_complete_cb_)) {
+      on_complete_cb_(std::move(other.on_complete_cb_)),
+      need_to_apply_(other.need_to_apply_) {
   other.transaction_ = nullptr;
   other.id_ = 0;
+  other.need_to_apply_ = false;
 }
 
 SurfaceControl::Transaction& SurfaceControl::Transaction::operator=(
     Transaction&& other) {
-  if (transaction_)
-    SurfaceControlMethods::Get().ASurfaceTransaction_deleteFn(transaction_);
+  if (this == &other)
+    return *this;
+
+  DestroyIfNeeded();
 
   transaction_ = other.transaction_;
   id_ = other.id_;
   on_commit_cb_ = std::move(other.on_commit_cb_);
   on_complete_cb_ = std::move(other.on_complete_cb_);
+  need_to_apply_ = other.need_to_apply_;
 
   other.transaction_ = nullptr;
   other.id_ = 0;
+  other.need_to_apply_ = false;
   return *this;
 }
 
@@ -676,6 +690,13 @@ void SurfaceControl::Transaction::SetBuffer(const Surface& surface,
   SurfaceControlMethods::Get().ASurfaceTransaction_setBufferFn(
       transaction_, surface.surface(), buffer,
       fence_fd.is_valid() ? fence_fd.release() : -1);
+  // In T OS, setBuffer call setOnComplete internally, so Apply() is required to
+  // decrease ref count of SurfaceControl.
+  // TODO(crbug.com/1395271): remove this if AOSP fix the issue
+  if (base::android::BuildInfo::GetInstance()->sdk_int() >=
+      base::android::SDK_VERSION_T) {
+    need_to_apply_ = true;
+  }
 }
 
 void SurfaceControl::Transaction::SetGeometry(const Surface& surface,
@@ -820,10 +841,12 @@ void SurfaceControl::Transaction::Apply() {
 
   PrepareCallbacks();
   SurfaceControlMethods::Get().ASurfaceTransaction_applyFn(transaction_);
+  need_to_apply_ = false;
 }
 
 ASurfaceTransaction* SurfaceControl::Transaction::GetTransaction() {
   PrepareCallbacks();
+  need_to_apply_ = false;
   return transaction_;
 }
 
@@ -835,6 +858,9 @@ void SurfaceControl::Transaction::PrepareCallbacks() {
 
     SurfaceControlMethods::Get().ASurfaceTransaction_setOnCommitFn(
         transaction_, ack_ctx, &OnTransactiOnCommittedOnAnyThread);
+    // setOnCommit and setOnComplete increase ref count of SurfaceControl and
+    // Apply() is required to decrease the ref count.
+    need_to_apply_ = true;
   }
 
   if (on_complete_cb_) {
@@ -844,6 +870,7 @@ void SurfaceControl::Transaction::PrepareCallbacks() {
 
     SurfaceControlMethods::Get().ASurfaceTransaction_setOnCompleteFn(
         transaction_, ack_ctx, &OnTransactionCompletedOnAnyThread);
+    need_to_apply_ = true;
   }
 }
 

ui/gfx/android/android_surface_control_compat.h

@@ -175,15 +175,19 @@ class GFX_EXPORT SurfaceControl {
                        scoped_refptr<base::SingleThreadTaskRunner> task_runner);
 
     void Apply();
+    // Caller(e.g.,WebView) must call ASurfaceTransaction_apply(), otherwise
+    // SurfaceControl leaks.
     ASurfaceTransaction* GetTransaction();
 
    private:
     void PrepareCallbacks();
+    void DestroyIfNeeded();
 
     int id_;
     ASurfaceTransaction* transaction_;
     OnCommitCb on_commit_cb_;
     OnCompleteCb on_complete_cb_;
+    bool need_to_apply_ = false;
   };
 };
 }  // namespace gfx