diff options
10 files changed, 56 insertions, 101 deletions
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index 6df971a9cea8..4952af353670 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -2636,13 +2636,6 @@ public final class ActivityThread extends ClientTransactionHandler } finally { controller.onClientTransactionFinished(); } - if (isSystem()) { - // Client transactions inside system process are recycled on the client side - // instead of ClientLifecycleManager to avoid being cleared before this - // message is handled. - transaction.recycle(); - } - // TODO(lifecycler): Recycle locally scheduled transactions. break; case RELAUNCH_ACTIVITY: handleRelaunchActivityLocally((IBinder) msg.obj); @@ -3824,7 +3817,7 @@ public final class ActivityThread extends ClientTransactionHandler + " req=" + requestCode + " res=" + resultCode + " data=" + data); final ArrayList<ResultInfo> list = new ArrayList<>(); list.add(new ResultInfo(id, requestCode, resultCode, data, activityToken)); - final ClientTransaction clientTransaction = ClientTransaction.obtain(mAppThread); + final ClientTransaction clientTransaction = new ClientTransaction(mAppThread); final ActivityResultItem activityResultItem = ActivityResultItem.obtain( activityToken, list); clientTransaction.addTransactionItem(activityResultItem); @@ -4620,7 +4613,7 @@ public final class ActivityThread extends ClientTransactionHandler } private void schedulePauseWithUserLeavingHint(ActivityClientRecord r) { - final ClientTransaction transaction = ClientTransaction.obtain(mAppThread); + final ClientTransaction transaction = new ClientTransaction(mAppThread); final PauseActivityItem pauseActivityItem = PauseActivityItem.obtain(r.token, r.activity.isFinishing(), /* userLeaving */ true, /* dontReport */ false, /* autoEnteringPip */ false); @@ -4629,7 +4622,7 @@ public final class ActivityThread extends ClientTransactionHandler } private void scheduleResume(ActivityClientRecord r) { - final ClientTransaction transaction = ClientTransaction.obtain(mAppThread); + final ClientTransaction transaction = new ClientTransaction(mAppThread); final ResumeActivityItem resumeActivityItem = ResumeActivityItem.obtain(r.token, /* isForward */ false, /* shouldSendCompatFakeFocus */ false); transaction.addTransactionItem(resumeActivityItem); @@ -6254,7 +6247,7 @@ public final class ActivityThread extends ClientTransactionHandler final ActivityLifecycleItem lifecycleRequest = TransactionExecutorHelper.getLifecycleRequestForCurrentState(r); // Schedule the transaction. - final ClientTransaction transaction = ClientTransaction.obtain(mAppThread); + final ClientTransaction transaction = new ClientTransaction(mAppThread); transaction.addTransactionItem(activityRelaunchItem); transaction.addTransactionItem(lifecycleRequest); executeTransaction(transaction); diff --git a/core/java/android/app/ClientTransactionHandler.java b/core/java/android/app/ClientTransactionHandler.java index f0c319673ade..f2f5374c9a06 100644 --- a/core/java/android/app/ClientTransactionHandler.java +++ b/core/java/android/app/ClientTransactionHandler.java @@ -42,6 +42,7 @@ import java.util.Map; /** * Defines operations that a {@link android.app.servertransaction.ClientTransaction} or its items * can perform on client. + * * @hide */ public abstract class ClientTransactionHandler { @@ -68,7 +69,6 @@ public abstract class ClientTransactionHandler { getTransactionExecutor().execute(transaction); } finally { mIsExecutingLocalTransaction = false; - transaction.recycle(); } } diff --git a/core/java/android/app/servertransaction/ClientTransaction.java b/core/java/android/app/servertransaction/ClientTransaction.java index 6fb852f50154..6e3e86cfbffc 100644 --- a/core/java/android/app/servertransaction/ClientTransaction.java +++ b/core/java/android/app/servertransaction/ClientTransaction.java @@ -18,6 +18,8 @@ package android.app.servertransaction; import static com.android.internal.annotations.VisibleForTesting.Visibility.PACKAGE; +import static java.util.Objects.requireNonNull; + import android.annotation.NonNull; import android.annotation.Nullable; import android.app.ClientTransactionHandler; @@ -30,7 +32,6 @@ import android.os.Parcelable; import android.os.RemoteException; import com.android.internal.annotations.VisibleForTesting; -import com.android.window.flags.Flags; import java.io.PrintWriter; import java.util.ArrayList; @@ -46,7 +47,7 @@ import java.util.Objects; * @see ActivityLifecycleItem * @hide */ -public class ClientTransaction implements Parcelable, ObjectPoolItem { +public class ClientTransaction implements Parcelable { /** * List of transaction items that should be executed in order. Including both @@ -76,10 +77,39 @@ public class ClientTransaction implements Parcelable, ObjectPoolItem { @Nullable private IBinder mActivityToken; - /** Target client. */ - private IApplicationThread mClient; + /** + * The target client. + * <p> + * This field is null only if the object is: + * - Read from a Parcel on the client side. + * - Constructed for testing purposes. + * <p> + * When created directly on the server, this field represents the server's connection to the + * target client's application thread. It is omitted during parceling and not sent to the + * client. On the client side, this field becomes unnecessary. + */ + @Nullable + private final IApplicationThread mClient; + + @VisibleForTesting + public ClientTransaction() { + mClient = null; + } - /** Get the target client of the transaction. */ + public ClientTransaction(@NonNull IApplicationThread client) { + mClient = requireNonNull(client); + } + + /** + * Gets the target client associated with this transaction. + * <p> + * This method is intended for server-side use only. Calling it from the client side + * will always return {@code null}. + * + * @return the {@link IApplicationThread} representing the target client, or {@code null} if + * called from the client side. + * @see #mClient + */ public IApplicationThread getClient() { return mClient; } @@ -211,51 +241,18 @@ public class ClientTransaction implements Parcelable, ObjectPoolItem { mClient.scheduleTransaction(this); } - - // ObjectPoolItem implementation - - private ClientTransaction() {} - - /** Obtains an instance initialized with provided params. */ - @NonNull - public static ClientTransaction obtain(@Nullable IApplicationThread client) { - ClientTransaction instance = ObjectPool.obtain(ClientTransaction.class); - if (instance == null) { - instance = new ClientTransaction(); - } - instance.mClient = client; - - return instance; - } - - @Override - public void recycle() { - if (Flags.disableObjectPool()) { - return; - } - int size = mTransactionItems.size(); - for (int i = 0; i < size; i++) { - mTransactionItems.get(i).recycle(); - } - mTransactionItems.clear(); - mActivityCallbacks = null; - mLifecycleStateRequest = null; - mClient = null; - mActivityToken = null; - ObjectPool.recycle(this); - } - // Parcelable implementation - /** Write to Parcel. */ + /** Writes to Parcel. */ @SuppressWarnings("AndroidFrameworkEfficientParcelable") // Item class is not final. @Override public void writeToParcel(@NonNull Parcel dest, int flags) { dest.writeParcelableList(mTransactionItems, flags); } - /** Read from Parcel. */ + /** Reads from Parcel. */ private ClientTransaction(@NonNull Parcel in) { + mClient = null; // This field is unnecessary on the client side. in.readParcelableList(mTransactionItems, getClass().getClassLoader(), ClientTransactionItem.class); diff --git a/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java b/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java index 294352e7f928..f5affd39b940 100644 --- a/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java +++ b/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java @@ -78,10 +78,10 @@ import android.window.ActivityWindowInfo; import android.window.WindowContextInfo; import android.window.WindowTokenClientController; +import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.MediumTest; import androidx.test.platform.app.InstrumentationRegistry; import androidx.test.rule.ActivityTestRule; -import androidx.test.runner.AndroidJUnit4; import com.android.internal.content.ReferrerIntent; @@ -1047,7 +1047,7 @@ public class ActivityThreadTest { @NonNull private static ClientTransaction newTransaction(@NonNull ActivityThread activityThread) { - return ClientTransaction.obtain(activityThread.getApplicationThread()); + return new ClientTransaction(activityThread.getApplicationThread()); } // Test activity diff --git a/core/tests/coretests/src/android/app/servertransaction/ClientTransactionTests.java b/core/tests/coretests/src/android/app/servertransaction/ClientTransactionTests.java index e559b05e669a..05392ebc5ade 100644 --- a/core/tests/coretests/src/android/app/servertransaction/ClientTransactionTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/ClientTransactionTests.java @@ -52,7 +52,7 @@ public class ClientTransactionTests { final ClientTransactionHandler clientTransactionHandler = mock(ClientTransactionHandler.class); - final ClientTransaction transaction = ClientTransaction.obtain(null /* client */); + final ClientTransaction transaction = new ClientTransaction(); transaction.addTransactionItem(callback1); transaction.addTransactionItem(callback2); transaction.addTransactionItem(stateRequest); diff --git a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java index e429cfc687bb..da88478efdfb 100644 --- a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java @@ -25,7 +25,6 @@ import static org.junit.Assert.assertNotSame; import android.annotation.NonNull; import android.app.ActivityOptions; -import android.app.IApplicationThread; import android.app.servertransaction.TestUtils.LaunchActivityItemBuilder; import android.content.Intent; import android.content.pm.ActivityInfo; @@ -68,8 +67,6 @@ public class ObjectPoolTests { public final MockitoRule mocks = MockitoJUnit.rule(); @Mock - private IApplicationThread mApplicationThread; - @Mock private IBinder mActivityToken; // 1. Check if two obtained objects from pool are not the same. @@ -184,11 +181,6 @@ public class ObjectPoolTests { testRecycle(() -> StopActivityItem.obtain(mActivityToken)); } - @Test - public void testRecycleClientTransaction() { - testRecycle(() -> ClientTransaction.obtain(mApplicationThread)); - } - private void testRecycle(@NonNull Supplier<? extends ObjectPoolItem> obtain) { // Reuse the same object after recycle. final ObjectPoolItem item = obtain.get(); diff --git a/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java b/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java index 024df1ee2513..ad86e753435c 100644 --- a/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java @@ -248,7 +248,7 @@ public class TransactionExecutorTests { ClientTransactionItem callback2 = mock(ClientTransactionItem.class); when(callback2.getPostExecutionState()).thenReturn(UNDEFINED); - ClientTransaction transaction = ClientTransaction.obtain(null /* client */); + final ClientTransaction transaction = new ClientTransaction(); transaction.addTransactionItem(callback1); transaction.addTransactionItem(callback2); transaction.addTransactionItem(mActivityLifecycleItem); @@ -273,7 +273,7 @@ public class TransactionExecutorTests { // An incoming destroy transaction enters binder thread (preExecute). final IBinder token = mock(IBinder.class); - final ClientTransaction destroyTransaction = ClientTransaction.obtain(null /* client */); + final ClientTransaction destroyTransaction = new ClientTransaction(); destroyTransaction.addTransactionItem( DestroyActivityItem.obtain(token, false /* finished */)); destroyTransaction.preExecute(mTransactionHandler); @@ -281,7 +281,7 @@ public class TransactionExecutorTests { assertEquals(1, activitiesToBeDestroyed.size()); // A previous queued launch transaction runs on main thread (execute). - final ClientTransaction launchTransaction = ClientTransaction.obtain(null /* client */); + final ClientTransaction launchTransaction = new ClientTransaction(); final LaunchActivityItem launchItem = spy(new LaunchActivityItemBuilder(token, new Intent(), new ActivityInfo()).build()); launchTransaction.addTransactionItem(launchItem); @@ -302,7 +302,7 @@ public class TransactionExecutorTests { PostExecItem postExecItem = new PostExecItem(ON_RESUME); - ClientTransaction transaction = ClientTransaction.obtain(null /* client */); + final ClientTransaction transaction = new ClientTransaction(); transaction.addTransactionItem(postExecItem); // Verify resolution that should get to onPause @@ -454,7 +454,7 @@ public class TransactionExecutorTests { final ActivityTransactionItem activityItem = mock(ActivityTransactionItem.class); when(activityItem.getPostExecutionState()).thenReturn(UNDEFINED); final IBinder token = mock(IBinder.class); - final ClientTransaction transaction = ClientTransaction.obtain(null /* client */); + final ClientTransaction transaction = new ClientTransaction(); transaction.addTransactionItem(activityItem); when(mTransactionHandler.getActivityClient(token)).thenReturn(null); @@ -463,7 +463,7 @@ public class TransactionExecutorTests { @Test public void testActivityItemExecute() { - final ClientTransaction transaction = ClientTransaction.obtain(null /* client */); + final ClientTransaction transaction = new ClientTransaction(); final ActivityTransactionItem activityItem = mock(ActivityTransactionItem.class); when(activityItem.getPostExecutionState()).thenReturn(UNDEFINED); when(activityItem.getActivityToken()).thenReturn(mActivityToken); @@ -498,7 +498,7 @@ public class TransactionExecutorTests { private static class PostExecItem extends StubItem { @LifecycleState - private int mPostExecutionState; + private final int mPostExecutionState; PostExecItem(@LifecycleState int state) { mPostExecutionState = state; diff --git a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java index 81291ff577f6..e9950358065a 100644 --- a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java @@ -295,7 +295,7 @@ public class TransactionParcelTests { StopActivityItem lifecycleRequest = StopActivityItem.obtain(mActivityToken); - ClientTransaction transaction = ClientTransaction.obtain(null /* client */); + final ClientTransaction transaction = new ClientTransaction(); transaction.addTransactionItem(callback1); transaction.addTransactionItem(callback2); transaction.addTransactionItem(lifecycleRequest); diff --git a/services/core/java/com/android/server/wm/ClientLifecycleManager.java b/services/core/java/com/android/server/wm/ClientLifecycleManager.java index bf347597ef0b..0acc6610a218 100644 --- a/services/core/java/com/android/server/wm/ClientLifecycleManager.java +++ b/services/core/java/com/android/server/wm/ClientLifecycleManager.java @@ -25,7 +25,6 @@ import android.app.servertransaction.ClientTransactionItem; import android.app.servertransaction.LaunchActivityItem; import android.compat.annotation.ChangeId; import android.compat.annotation.EnabledSince; -import android.os.Binder; import android.os.Build; import android.os.IBinder; import android.os.RemoteException; @@ -81,13 +80,6 @@ class ClientLifecycleManager { Slog.w(TAG, "Failed to deliver transaction for " + client + "\ntransaction=" + transaction); throw e; - } finally { - if (!(client instanceof Binder)) { - // If client is not an instance of Binder - it's a remote call and at this point it - // is safe to recycle the object. All objects used for local calls will be recycled - // after the transaction is executed on client in ActivityThread. - transaction.recycle(); - } } } @@ -99,7 +91,7 @@ class ClientLifecycleManager { */ void scheduleTransactionItemNow(@NonNull IApplicationThread client, @NonNull ClientTransactionItem transactionItem) throws RemoteException { - final ClientTransaction clientTransaction = ClientTransaction.obtain(client); + final ClientTransaction clientTransaction = new ClientTransaction(client); clientTransaction.addTransactionItem(transactionItem); scheduleTransaction(clientTransaction); } @@ -210,7 +202,7 @@ class ClientLifecycleManager { } // Create new transaction if there is no existing. - final ClientTransaction transaction = ClientTransaction.obtain(client); + final ClientTransaction transaction = new ClientTransaction(client); mPendingTransactions.put(clientBinder, transaction); return transaction; } diff --git a/services/tests/wmtests/src/com/android/server/wm/ClientLifecycleManagerTests.java b/services/tests/wmtests/src/com/android/server/wm/ClientLifecycleManagerTests.java index 2bda9500905e..3ddf8da18d16 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ClientLifecycleManagerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ClientLifecycleManagerTests.java @@ -87,24 +87,6 @@ public class ClientLifecycleManagerTests extends SystemServiceTestsBase { } @Test - public void testScheduleTransaction_recycleBinderClientTransaction() throws Exception { - final ClientTransaction item = spy(ClientTransaction.obtain(mClient)); - - mLifecycleManager.scheduleTransaction(item); - - verify(item).recycle(); - } - - @Test - public void testScheduleTransaction_notRecycleNonBinderClientTransaction() throws Exception { - final ClientTransaction item = spy(ClientTransaction.obtain(mNonBinderClient)); - - mLifecycleManager.scheduleTransaction(item); - - verify(item, never()).recycle(); - } - - @Test public void testScheduleTransactionItem() throws RemoteException { spyOn(mWms.mWindowPlacerLocked); doReturn(true).when(mWms.mWindowPlacerLocked).isTraversalScheduled(); @@ -194,7 +176,6 @@ public class ClientLifecycleManagerTests extends SystemServiceTestsBase { assertTrue(mLifecycleManager.mPendingTransactions.isEmpty()); verify(mTransaction).schedule(); - verify(mTransaction).recycle(); } @Test |