From ea203cdbdf271f4c5e57aa0b27255e82ac5ddcbc Mon Sep 17 00:00:00 2001 From: Louis Chang Date: Mon, 1 Jul 2019 12:39:31 +0800 Subject: Avoid making post state to RESUMED for a PAUSING activity Make sure the client post execution lifecycle state to RESUMED only if the activity was RESUMED while delivering new intent. Bug: 135715788 Test: making skype calls Test: atest ActivityThreadTest Change-Id: I1e3054e1d1611aecf6ddf6d482abf2cb3ebdf9a4 --- .../app/servertransaction/NewIntentItem.java | 15 +++++++--- .../android/app/activity/ActivityThreadTest.java | 35 ++++++++++++++++++++++ .../app/servertransaction/ObjectPoolTests.java | 6 ++-- .../servertransaction/TransactionParcelTests.java | 2 +- .../java/com/android/server/wm/ActivityRecord.java | 5 +++- .../java/com/android/server/wm/ActivityStack.java | 3 +- 6 files changed, 56 insertions(+), 10 deletions(-) diff --git a/core/java/android/app/servertransaction/NewIntentItem.java b/core/java/android/app/servertransaction/NewIntentItem.java index 2d1883836d02..bb775fc4a5fb 100644 --- a/core/java/android/app/servertransaction/NewIntentItem.java +++ b/core/java/android/app/servertransaction/NewIntentItem.java @@ -17,6 +17,7 @@ package android.app.servertransaction; import static android.app.servertransaction.ActivityLifecycleItem.ON_RESUME; +import static android.app.servertransaction.ActivityLifecycleItem.UNDEFINED; import android.annotation.UnsupportedAppUsage; import android.app.ClientTransactionHandler; @@ -38,10 +39,11 @@ public class NewIntentItem extends ClientTransactionItem { @UnsupportedAppUsage private List mIntents; + private boolean mResume; @Override public int getPostExecutionState() { - return ON_RESUME; + return mResume ? ON_RESUME : UNDEFINED; } @Override @@ -58,12 +60,13 @@ public class NewIntentItem extends ClientTransactionItem { private NewIntentItem() {} /** Obtain an instance initialized with provided params. */ - public static NewIntentItem obtain(List intents) { + public static NewIntentItem obtain(List intents, boolean resume) { NewIntentItem instance = ObjectPool.obtain(NewIntentItem.class); if (instance == null) { instance = new NewIntentItem(); } instance.mIntents = intents; + instance.mResume = resume; return instance; } @@ -71,6 +74,7 @@ public class NewIntentItem extends ClientTransactionItem { @Override public void recycle() { mIntents = null; + mResume = false; ObjectPool.recycle(this); } @@ -80,11 +84,13 @@ public class NewIntentItem extends ClientTransactionItem { /** Write to Parcel. */ @Override public void writeToParcel(Parcel dest, int flags) { + dest.writeBoolean(mResume); dest.writeTypedList(mIntents, flags); } /** Read from Parcel. */ private NewIntentItem(Parcel in) { + mResume = in.readBoolean(); mIntents = in.createTypedArrayList(ReferrerIntent.CREATOR); } @@ -108,18 +114,19 @@ public class NewIntentItem extends ClientTransactionItem { return false; } final NewIntentItem other = (NewIntentItem) o; - return Objects.equals(mIntents, other.mIntents); + return mResume == other.mResume && Objects.equals(mIntents, other.mIntents); } @Override public int hashCode() { int result = 17; + result = 31 * result + (mResume ? 1 : 0); result = 31 * result + mIntents.hashCode(); return result; } @Override public String toString() { - return "NewIntentItem{intents=" + mIntents + "}"; + return "NewIntentItem{intents=" + mIntents + ",resume=" + mResume + "}"; } } diff --git a/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java b/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java index 711eaa7edc2a..c50cbe3773ab 100644 --- a/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java +++ b/core/tests/coretests/src/android/app/activity/ActivityThreadTest.java @@ -19,6 +19,8 @@ package android.app.activity; import static android.content.Intent.ACTION_EDIT; import static android.content.Intent.ACTION_VIEW; +import static com.google.common.truth.Truth.assertThat; + import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; @@ -31,6 +33,7 @@ import android.app.servertransaction.ActivityConfigurationChangeItem; import android.app.servertransaction.ActivityRelaunchItem; import android.app.servertransaction.ClientTransaction; import android.app.servertransaction.ClientTransactionItem; +import android.app.servertransaction.NewIntentItem; import android.app.servertransaction.ResumeActivityItem; import android.app.servertransaction.StopActivityItem; import android.content.Intent; @@ -45,9 +48,13 @@ import androidx.test.filters.MediumTest; import androidx.test.rule.ActivityTestRule; import androidx.test.runner.AndroidJUnit4; +import com.android.internal.content.ReferrerIntent; + import org.junit.Test; import org.junit.runner.RunWith; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.CountDownLatch; /** @@ -307,6 +314,24 @@ public class ActivityThreadTest { assertEquals(400, activity.mConfig.smallestScreenWidthDp); } + @Test + public void testResumeAfterNewIntent() { + final Activity activity = mActivityTestRule.launchActivity(new Intent()); + final ActivityThread activityThread = activity.getActivityThread(); + final ArrayList rIntents = new ArrayList<>(); + rIntents.add(new ReferrerIntent(new Intent(), "android.app.activity")); + + InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { + activityThread.executeTransaction(newNewIntentTransaction(activity, rIntents, false)); + }); + assertThat(activity.isResumed()).isFalse(); + + InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { + activityThread.executeTransaction(newNewIntentTransaction(activity, rIntents, true)); + }); + assertThat(activity.isResumed()).isTrue(); + } + /** * Calls {@link ActivityThread#handleActivityConfigurationChanged(IBinder, Configuration, int)} * to try to push activity configuration to the activity for the given sequence number. @@ -386,6 +411,16 @@ public class ActivityThreadTest { return transaction; } + private static ClientTransaction newNewIntentTransaction(Activity activity, + List intents, boolean resume) { + final NewIntentItem item = NewIntentItem.obtain(intents, resume); + + final ClientTransaction transaction = newTransaction(activity); + transaction.addCallback(item); + + return transaction; + } + private static ClientTransaction newTransaction(Activity activity) { final IApplicationThread appThread = activity.getActivityThread().getApplicationThread(); return ClientTransaction.obtain(appThread, activity.getActivityToken()); diff --git a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java index 1e49c0a7f55d..37d21f0928be 100644 --- a/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/ObjectPoolTests.java @@ -214,15 +214,15 @@ public class ObjectPoolTests { @Test public void testRecycleNewIntentItem() { - NewIntentItem emptyItem = NewIntentItem.obtain(null); - NewIntentItem item = NewIntentItem.obtain(referrerIntentList()); + NewIntentItem emptyItem = NewIntentItem.obtain(null, false); + NewIntentItem item = NewIntentItem.obtain(referrerIntentList(), false); assertNotSame(item, emptyItem); assertFalse(item.equals(emptyItem)); item.recycle(); assertEquals(item, emptyItem); - NewIntentItem item2 = NewIntentItem.obtain(referrerIntentList()); + NewIntentItem item2 = NewIntentItem.obtain(referrerIntentList(), false); assertSame(item, item2); assertFalse(item2.equals(emptyItem)); } diff --git a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java index 36ed88fbe8d5..d2b18cb0bcb8 100644 --- a/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/TransactionParcelTests.java @@ -128,7 +128,7 @@ public class TransactionParcelTests { @Test public void testNewIntent() { // Write to parcel - NewIntentItem item = NewIntentItem.obtain(referrerIntentList()); + NewIntentItem item = NewIntentItem.obtain(referrerIntentList(), false); writeAndPrepareForReading(item); // Read from parcel and assert diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java index 0faea61b9d60..371a94356432 100644 --- a/services/core/java/com/android/server/wm/ActivityRecord.java +++ b/services/core/java/com/android/server/wm/ActivityRecord.java @@ -1616,8 +1616,11 @@ final class ActivityRecord extends ConfigurationContainer { try { ArrayList ar = new ArrayList<>(1); ar.add(rintent); + // Making sure the client state is RESUMED after transaction completed and doing + // so only if activity is currently RESUMED. Otherwise, client may have extra + // life-cycle calls to RESUMED (and PAUSED later). mAtmService.getLifecycleManager().scheduleTransaction(app.getThread(), appToken, - NewIntentItem.obtain(ar)); + NewIntentItem.obtain(ar, mState == RESUMED)); unsent = false; } catch (RemoteException e) { Slog.w(TAG, "Exception thrown sending new intent to " + this, e); diff --git a/services/core/java/com/android/server/wm/ActivityStack.java b/services/core/java/com/android/server/wm/ActivityStack.java index 30e866b8e2a4..6bed46226b42 100644 --- a/services/core/java/com/android/server/wm/ActivityStack.java +++ b/services/core/java/com/android/server/wm/ActivityStack.java @@ -2964,7 +2964,8 @@ class ActivityStack extends ConfigurationContainer { } if (next.newIntents != null) { - transaction.addCallback(NewIntentItem.obtain(next.newIntents)); + transaction.addCallback( + NewIntentItem.obtain(next.newIntents, true /* resume */)); } // Well the app will no longer be stopped. -- cgit v1.2.3-59-g8ed1b