diff options
7 files changed, 93 insertions, 32 deletions
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index de346f315016..e610ac4a318b 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -1768,7 +1768,14 @@ public final class ActivityThread extends ClientTransactionHandler { (String[]) ((SomeArgs) msg.obj).arg2); break; case EXECUTE_TRANSACTION: - mTransactionExecutor.execute(((ClientTransaction) msg.obj)); + final ClientTransaction transaction = (ClientTransaction) msg.obj; + mTransactionExecutor.execute(transaction); + 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(); + } break; } Object obj = msg.obj; diff --git a/core/java/android/app/servertransaction/ClientTransaction.java b/core/java/android/app/servertransaction/ClientTransaction.java index 7703c6bef364..08ad2f055774 100644 --- a/core/java/android/app/servertransaction/ClientTransaction.java +++ b/core/java/android/app/servertransaction/ClientTransaction.java @@ -56,6 +56,11 @@ public class ClientTransaction implements Parcelable, ObjectPoolItem { /** Target client activity. Might be null if the entire transaction is targeting an app. */ private IBinder mActivityToken; + /** Get the target client of the transaction. */ + public IApplicationThread getClient() { + return mClient; + } + /** * Add a message to the end of the sequence of callbacks. * @param activityCallback A single message that can contain a lifecycle request/callback. diff --git a/core/java/android/app/servertransaction/ObjectPool.java b/core/java/android/app/servertransaction/ObjectPool.java index 98121253f486..2fec30a0dde7 100644 --- a/core/java/android/app/servertransaction/ObjectPool.java +++ b/core/java/android/app/servertransaction/ObjectPool.java @@ -16,8 +16,8 @@ package android.app.servertransaction; +import java.util.ArrayList; import java.util.HashMap; -import java.util.LinkedList; import java.util.Map; /** @@ -27,7 +27,7 @@ import java.util.Map; class ObjectPool { private static final Object sPoolSync = new Object(); - private static final Map<Class, LinkedList<? extends ObjectPoolItem>> sPoolMap = + private static final Map<Class, ArrayList<? extends ObjectPoolItem>> sPoolMap = new HashMap<>(); private static final int MAX_POOL_SIZE = 50; @@ -40,9 +40,9 @@ class ObjectPool { public static <T extends ObjectPoolItem> T obtain(Class<T> itemClass) { synchronized (sPoolSync) { @SuppressWarnings("unchecked") - LinkedList<T> itemPool = (LinkedList<T>) sPoolMap.get(itemClass); + final ArrayList<T> itemPool = (ArrayList<T>) sPoolMap.get(itemClass); if (itemPool != null && !itemPool.isEmpty()) { - return itemPool.poll(); + return itemPool.remove(itemPool.size() - 1); } return null; } @@ -56,16 +56,20 @@ class ObjectPool { public static <T extends ObjectPoolItem> void recycle(T item) { synchronized (sPoolSync) { @SuppressWarnings("unchecked") - LinkedList<T> itemPool = (LinkedList<T>) sPoolMap.get(item.getClass()); + ArrayList<T> itemPool = (ArrayList<T>) sPoolMap.get(item.getClass()); if (itemPool == null) { - itemPool = new LinkedList<>(); + itemPool = new ArrayList<>(); sPoolMap.put(item.getClass(), itemPool); } - if (itemPool.contains(item)) { - throw new IllegalStateException("Trying to recycle already recycled item"); + // Check if the item is already in the pool + final int size = itemPool.size(); + for (int i = 0; i < size; i++) { + if (itemPool.get(i) == item) { + throw new IllegalStateException("Trying to recycle already recycled item"); + } } - if (itemPool.size() < MAX_POOL_SIZE) { + if (size < MAX_POOL_SIZE) { itemPool.add(item); } } diff --git a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java index c19a343957c0..aefc47e95512 100644 --- a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java @@ -42,8 +42,7 @@ import org.junit.runner.RunWith; @RunWith(AndroidJUnit4.class) @SmallTest -// TODO: b/70616950 -//@Presubmit +@Presubmit public class ObjectPoolTests { // 1. Check if two obtained objects from pool are not the same. diff --git a/services/core/java/com/android/server/am/ClientLifecycleManager.java b/services/core/java/com/android/server/am/ClientLifecycleManager.java index 014f7086efa3..ae8d9fc104bb 100644 --- a/services/core/java/com/android/server/am/ClientLifecycleManager.java +++ b/services/core/java/com/android/server/am/ClientLifecycleManager.java @@ -21,6 +21,7 @@ import android.app.IApplicationThread; import android.app.servertransaction.ClientTransaction; import android.app.servertransaction.ClientTransactionItem; import android.app.servertransaction.ActivityLifecycleItem; +import android.os.Binder; import android.os.IBinder; import android.os.RemoteException; @@ -42,9 +43,14 @@ class ClientLifecycleManager { * @see ClientTransaction */ void scheduleTransaction(ClientTransaction transaction) throws RemoteException { + final IApplicationThread client = transaction.getClient(); transaction.schedule(); - // TODO: b/70616950 - //transaction.recycle(); + 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(); + } } /** diff --git a/services/tests/servicestests/src/com/android/server/am/ActivityRecordTests.java b/services/tests/servicestests/src/com/android/server/am/ActivityRecordTests.java index 1dba39f5345a..b38a4136c94a 100644 --- a/services/tests/servicestests/src/com/android/server/am/ActivityRecordTests.java +++ b/services/tests/servicestests/src/com/android/server/am/ActivityRecordTests.java @@ -35,24 +35,23 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyInt; -import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; - import android.app.servertransaction.ClientTransaction; import android.app.servertransaction.PauseActivityItem; import android.graphics.Rect; import android.platform.test.annotations.Presubmit; import android.support.test.filters.MediumTest; import android.support.test.runner.AndroidJUnit4; +import android.util.MutableBoolean; import org.junit.runner.RunWith; import org.junit.Before; import org.junit.Test; - -import org.mockito.ArgumentCaptor; +import org.mockito.invocation.InvocationOnMock; /** * Tests for the {@link ActivityRecord} class. @@ -110,23 +109,20 @@ public class ActivityRecordTests extends ActivityTestsBase { @Test public void testPausingWhenVisibleFromStopped() throws Exception { + final MutableBoolean pauseFound = new MutableBoolean(false); + doAnswer((InvocationOnMock invocationOnMock) -> { + final ClientTransaction transaction = invocationOnMock.getArgument(0); + if (transaction.getLifecycleStateRequest() instanceof PauseActivityItem) { + pauseFound.value = true; + } + return null; + }).when(mActivity.app.thread).scheduleTransaction(any()); mActivity.state = STOPPED; - mActivity.makeVisibleIfNeeded(null /* starting */); - assertEquals(mActivity.state, PAUSING); - - final ArgumentCaptor<ClientTransaction> transaction = - ArgumentCaptor.forClass(ClientTransaction.class); - verify(mActivity.app.thread, atLeast(1)).scheduleTransaction(transaction.capture()); - boolean pauseFound = false; - - for (ClientTransaction targetTransaction : transaction.getAllValues()) { - if (targetTransaction.getLifecycleStateRequest() instanceof PauseActivityItem) { - pauseFound = true; - } - } + mActivity.makeVisibleIfNeeded(null /* starting */); - assertTrue(pauseFound); + assertEquals(mActivity.state, PAUSING); + assertTrue(pauseFound.value); } @Test diff --git a/services/tests/servicestests/src/com/android/server/am/ClientLifecycleManagerTests.java b/services/tests/servicestests/src/com/android/server/am/ClientLifecycleManagerTests.java new file mode 100644 index 000000000000..ef6d5e81b472 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/am/ClientLifecycleManagerTests.java @@ -0,0 +1,44 @@ +package com.android.server.am; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import android.app.IApplicationThread; +import android.app.servertransaction.ClientTransaction; +import android.os.Binder; +import android.platform.test.annotations.Presubmit; +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; + +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +@SmallTest +@Presubmit +public class ClientLifecycleManagerTests { + + @Test + public void testScheduleAndRecycleBinderClientTransaction() throws Exception { + ClientTransaction item = spy(ClientTransaction.obtain(mock(IApplicationThread.class), + new Binder())); + + ClientLifecycleManager clientLifecycleManager = new ClientLifecycleManager(); + clientLifecycleManager.scheduleTransaction(item); + + verify(item, times(1)).recycle(); + } + + @Test + public void testScheduleNoRecycleNonBinderClientTransaction() throws Exception { + ClientTransaction item = spy(ClientTransaction.obtain(mock(IApplicationThread.Stub.class), + new Binder())); + + ClientLifecycleManager clientLifecycleManager = new ClientLifecycleManager(); + clientLifecycleManager.scheduleTransaction(item); + + verify(item, times(0)).recycle(); + } +} |