From 4df4151bc0b45208bb0318dca2f03b8ff036a1ce Mon Sep 17 00:00:00 2001 From: Huihong Luo Date: Thu, 24 Jun 2021 10:04:32 -0700 Subject: [PATCH] Fix a crash caused by transaction A native transaction passed from webview is sent back to native side, so java side does not manage the life cycle of the transaction. Bug: 191414767 Test: Play a video, switch to another app, wait for 10 seconds Change-Id: I013052c202b445438d6cb6497f5f9a2fc22a2b85 --- core/java/android/view/SurfaceControl.java | 10 ---------- core/java/android/view/ViewRootImpl.java | 8 ++++++-- .../java/android/graphics/BLASTBufferQueue.java | 8 ++++++++ .../java/android/graphics/HardwareRenderer.java | 2 +- .../hwui/jni/android_graphics_HardwareRenderer.cpp | 14 ++++++++------ libs/hwui/renderthread/CanvasContext.cpp | 5 ++--- libs/hwui/renderthread/CanvasContext.h | 4 ++-- libs/hwui/renderthread/RenderProxy.cpp | 2 +- libs/hwui/renderthread/RenderProxy.h | 2 +- 9 files changed, 29 insertions(+), 26 deletions(-) diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index 4e66ceb76a607..c03db6d673569 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -2610,16 +2610,6 @@ public final class SurfaceControl implements Parcelable { = sRegistry.registerNativeAllocation(this, mNativeObject); } - /** - * Create a transaction object that wraps a native peer. - * @hide - */ - Transaction(long nativeObject) { - mNativeObject = nativeObject; - mFreeNativeResources = - sRegistry.registerNativeAllocation(this, mNativeObject); - } - private Transaction(Parcel in) { readFromParcel(in); } diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java index 210f10c10ad16..5a248af7a097e 100644 --- a/core/java/android/view/ViewRootImpl.java +++ b/core/java/android/view/ViewRootImpl.java @@ -1371,8 +1371,12 @@ public final class ViewRootImpl implements ViewParent, HardwareRenderer.ASurfaceTransactionCallback callback = (nativeTransactionObj, nativeSurfaceControlObj, frameNr) -> { - Transaction t = new Transaction(nativeTransactionObj); - mergeWithNextTransaction(t, frameNr); + if (mBlastBufferQueue == null) { + return false; + } else { + mBlastBufferQueue.mergeWithNextTransaction(nativeTransactionObj, frameNr); + return true; + } }; mAttachInfo.mThreadedRenderer.setASurfaceTransactionCallback(callback); } diff --git a/graphics/java/android/graphics/BLASTBufferQueue.java b/graphics/java/android/graphics/BLASTBufferQueue.java index 4534d36342dbc..6c1c2ee1ee575 100644 --- a/graphics/java/android/graphics/BLASTBufferQueue.java +++ b/graphics/java/android/graphics/BLASTBufferQueue.java @@ -131,4 +131,12 @@ public final class BLASTBufferQueue { nativeMergeWithNextTransaction(mNativeObject, t.mNativeObject, frameNumber); } + /** + * Merge the transaction passed in to the next transaction in BlastBufferQueue. + * @param nativeTransaction native handle passed from native c/c++ code. + */ + public void mergeWithNextTransaction(long nativeTransaction, long frameNumber) { + nativeMergeWithNextTransaction(mNativeObject, nativeTransaction, frameNumber); + } + } diff --git a/graphics/java/android/graphics/HardwareRenderer.java b/graphics/java/android/graphics/HardwareRenderer.java index e141d5178570d..30d1e0fdb9d8c 100644 --- a/graphics/java/android/graphics/HardwareRenderer.java +++ b/graphics/java/android/graphics/HardwareRenderer.java @@ -912,7 +912,7 @@ public class HardwareRenderer { * @param aSurfaceControlNativeObj ASurfaceControl native object handle * @param frame The id of the frame being drawn. */ - void onMergeTransaction(long aSurfaceTranactionNativeObj, + boolean onMergeTransaction(long aSurfaceTranactionNativeObj, long aSurfaceControlNativeObj, long frame); } diff --git a/libs/hwui/jni/android_graphics_HardwareRenderer.cpp b/libs/hwui/jni/android_graphics_HardwareRenderer.cpp index 9ff2f461d40d7..4d31cd90d40fc 100644 --- a/libs/hwui/jni/android_graphics_HardwareRenderer.cpp +++ b/libs/hwui/jni/android_graphics_HardwareRenderer.cpp @@ -662,16 +662,18 @@ static void android_view_ThreadedRenderer_setASurfaceTransactionCallback( auto globalCallbackRef = std::make_shared(vm, aSurfaceTransactionCallback); proxy->setASurfaceTransactionCallback( - [globalCallbackRef](int64_t transObj, int64_t scObj, int64_t frameNr) { + [globalCallbackRef](int64_t transObj, int64_t scObj, int64_t frameNr) -> bool { JNIEnv* env = getenv(globalCallbackRef->vm()); jobject localref = env->NewLocalRef(globalCallbackRef->ref()); if (CC_UNLIKELY(!localref)) { - return; + return false; } - env->CallVoidMethod(localref, gASurfaceTransactionCallback.onMergeTransaction, - static_cast(transObj), static_cast(scObj), - static_cast(frameNr)); + jboolean ret = env->CallBooleanMethod( + localref, gASurfaceTransactionCallback.onMergeTransaction, + static_cast(transObj), static_cast(scObj), + static_cast(frameNr)); env->DeleteLocalRef(localref); + return ret; }); } } @@ -1064,7 +1066,7 @@ int register_android_view_ThreadedRenderer(JNIEnv* env) { jclass aSurfaceTransactionCallbackClass = FindClassOrDie(env, "android/graphics/HardwareRenderer$ASurfaceTransactionCallback"); gASurfaceTransactionCallback.onMergeTransaction = - GetMethodIDOrDie(env, aSurfaceTransactionCallbackClass, "onMergeTransaction", "(JJJ)V"); + GetMethodIDOrDie(env, aSurfaceTransactionCallbackClass, "onMergeTransaction", "(JJJ)Z"); jclass prepareSurfaceControlForWebviewCallbackClass = FindClassOrDie( env, "android/graphics/HardwareRenderer$PrepareSurfaceControlForWebviewCallback"); diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp index 44d0038ad47ef..0c9711ba80254 100644 --- a/libs/hwui/renderthread/CanvasContext.cpp +++ b/libs/hwui/renderthread/CanvasContext.cpp @@ -910,9 +910,8 @@ CanvasContext* CanvasContext::getActiveContext() { bool CanvasContext::mergeTransaction(ASurfaceTransaction* transaction, ASurfaceControl* control) { if (!mASurfaceTransactionCallback) return false; - std::invoke(mASurfaceTransactionCallback, reinterpret_cast(transaction), - reinterpret_cast(control), getFrameNumber()); - return true; + return std::invoke(mASurfaceTransactionCallback, reinterpret_cast(transaction), + reinterpret_cast(control), getFrameNumber()); } void CanvasContext::prepareSurfaceControlForWebview() { diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h index a61c2bfd5c014..3279ccb8e5970 100644 --- a/libs/hwui/renderthread/CanvasContext.h +++ b/libs/hwui/renderthread/CanvasContext.h @@ -206,7 +206,7 @@ public: ASurfaceControlStats* stats); void setASurfaceTransactionCallback( - const std::function& callback) { + const std::function& callback) { mASurfaceTransactionCallback = callback; } @@ -317,7 +317,7 @@ private: // If set to true, we expect that callbacks into onSurfaceStatsAvailable bool mExpectSurfaceStats = false; - std::function mASurfaceTransactionCallback; + std::function mASurfaceTransactionCallback; std::function mPrepareSurfaceControlForWebviewCallback; void cleanupResources(); diff --git a/libs/hwui/renderthread/RenderProxy.cpp b/libs/hwui/renderthread/RenderProxy.cpp index c47050c31e9aa..a77b5b5699075 100644 --- a/libs/hwui/renderthread/RenderProxy.cpp +++ b/libs/hwui/renderthread/RenderProxy.cpp @@ -314,7 +314,7 @@ void RenderProxy::setPictureCapturedCallback( } void RenderProxy::setASurfaceTransactionCallback( - const std::function& callback) { + const std::function& callback) { mRenderThread.queue().post( [this, cb = callback]() { mContext->setASurfaceTransactionCallback(cb); }); } diff --git a/libs/hwui/renderthread/RenderProxy.h b/libs/hwui/renderthread/RenderProxy.h index d575aa77e4ab4..1b0f22e75a2de 100644 --- a/libs/hwui/renderthread/RenderProxy.h +++ b/libs/hwui/renderthread/RenderProxy.h @@ -123,7 +123,7 @@ public: void setContentDrawBounds(int left, int top, int right, int bottom); void setPictureCapturedCallback(const std::function&&)>& callback); void setASurfaceTransactionCallback( - const std::function& callback); + const std::function& callback); void setPrepareSurfaceControlForWebviewCallback(const std::function& callback); void setFrameCallback(std::function&& callback); void setFrameCompleteCallback(std::function&& callback);