From e55b009a3d3dbdef759704e0a5fce086d2e3a76e Mon Sep 17 00:00:00 2001 From: Andrii Kulian Date: Thu, 19 Apr 2018 15:29:22 -0700 Subject: [PATCH] Don't require post-execution state for onActivityResult According to the documentation, onActivityResult() should be called immediately before onResume(). This, however, was not always the case in previous releases and fixing this caused some app compatibility issues. This CL removes required post execution state for ActivityResultItem. Bug: 77695691 Test: ActivityLifecycleTests Change-Id: Id8c02e9b49f9758aac616e37948570722d802de8 --- core/java/android/app/Activity.java | 6 +- core/java/android/app/ActivityGroup.java | 8 +-- core/java/android/app/ActivityThread.java | 12 ++-- .../android/app/ClientTransactionHandler.java | 2 +- .../servertransaction/ActivityResultItem.java | 6 +- .../TransactionExecutorTests.java | 58 ++++++++++++++++++- .../android/server/am/EventLogTags.logtags | 2 + 7 files changed, 75 insertions(+), 19 deletions(-) diff --git a/core/java/android/app/Activity.java b/core/java/android/app/Activity.java index 20149dec4dad1..b456b72d8bf72 100644 --- a/core/java/android/app/Activity.java +++ b/core/java/android/app/Activity.java @@ -759,6 +759,7 @@ public class Activity extends ContextThemeWrapper private static final int LOG_AM_ON_STOP_CALLED = 30049; private static final int LOG_AM_ON_RESTART_CALLED = 30058; private static final int LOG_AM_ON_DESTROY_CALLED = 30060; + private static final int LOG_AM_ON_ACTIVITY_RESULT_CALLED = 30062; private static class ManagedDialog { Dialog mDialog; @@ -7438,8 +7439,8 @@ public class Activity extends ContextThemeWrapper } } - void dispatchActivityResult(String who, int requestCode, - int resultCode, Intent data) { + void dispatchActivityResult(String who, int requestCode, int resultCode, Intent data, + String reason) { if (false) Log.v( TAG, "Dispatching result: who=" + who + ", reqCode=" + requestCode + ", resCode=" + resultCode + ", data=" + data); @@ -7475,6 +7476,7 @@ public class Activity extends ContextThemeWrapper frag.onActivityResult(requestCode, resultCode, data); } } + writeEventLog(LOG_AM_ON_ACTIVITY_RESULT_CALLED, reason); } /** diff --git a/core/java/android/app/ActivityGroup.java b/core/java/android/app/ActivityGroup.java index 78a4dfd47b405..228067c3bf759 100644 --- a/core/java/android/app/ActivityGroup.java +++ b/core/java/android/app/ActivityGroup.java @@ -16,11 +16,11 @@ package android.app; -import java.util.HashMap; - import android.content.Intent; import android.os.Bundle; +import java.util.HashMap; + /** * A screen that contains and runs multiple embedded activities. * @@ -109,7 +109,7 @@ public class ActivityGroup extends Activity { @Override void dispatchActivityResult(String who, int requestCode, int resultCode, - Intent data) { + Intent data, String reason) { if (who != null) { Activity act = mLocalActivityManager.getActivity(who); /* @@ -123,7 +123,7 @@ public class ActivityGroup extends Activity { return; } } - super.dispatchActivityResult(who, requestCode, resultCode, data); + super.dispatchActivityResult(who, requestCode, resultCode, data, reason); } } diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java index a41da0eb1ee15..037a87b7b54ef 100644 --- a/core/java/android/app/ActivityThread.java +++ b/core/java/android/app/ActivityThread.java @@ -3730,7 +3730,7 @@ public final class ActivityThread extends ClientTransactionHandler { r.pendingIntents = null; } if (r.pendingResults != null) { - deliverResults(r, r.pendingResults); + deliverResults(r, r.pendingResults, reason); r.pendingResults = null; } r.activity.performResume(r.startsNotResumed, reason); @@ -4299,7 +4299,7 @@ public final class ActivityThread extends ClientTransactionHandler { WindowManagerGlobal.getInstance().reportNewConfiguration(mConfiguration); } - private void deliverResults(ActivityClientRecord r, List results) { + private void deliverResults(ActivityClientRecord r, List results, String reason) { final int N = results.size(); for (int i=0; i results) { + public void handleSendResult(IBinder token, List results, String reason) { ActivityClientRecord r = mActivities.get(token); if (DEBUG_RESULTS) Slog.v(TAG, "Handling send result to " + r); if (r != null) { @@ -4359,9 +4359,9 @@ public final class ActivityThread extends ClientTransactionHandler { } } checkAndBlockForNetworkAccess(); - deliverResults(r, results); + deliverResults(r, results, reason); if (resumed) { - r.activity.performResume(false, "handleSendResult"); + r.activity.performResume(false, reason); r.activity.mTemporaryPause = false; } } diff --git a/core/java/android/app/ClientTransactionHandler.java b/core/java/android/app/ClientTransactionHandler.java index e26d989f09efb..ea0d703a30e3b 100644 --- a/core/java/android/app/ClientTransactionHandler.java +++ b/core/java/android/app/ClientTransactionHandler.java @@ -121,7 +121,7 @@ public abstract class ClientTransactionHandler { Configuration overrideConfig, int displayId); /** Deliver result from another activity. */ - public abstract void handleSendResult(IBinder token, List results); + public abstract void handleSendResult(IBinder token, List results, String reason); /** Deliver multi-window mode change notification. */ public abstract void handleMultiWindowModeChanged(IBinder token, boolean isInMultiWindowMode, diff --git a/core/java/android/app/servertransaction/ActivityResultItem.java b/core/java/android/app/servertransaction/ActivityResultItem.java index 545463c124bba..e57f585d20228 100644 --- a/core/java/android/app/servertransaction/ActivityResultItem.java +++ b/core/java/android/app/servertransaction/ActivityResultItem.java @@ -16,7 +16,6 @@ package android.app.servertransaction; -import static android.app.servertransaction.ActivityLifecycleItem.ON_RESUME; import static android.os.Trace.TRACE_TAG_ACTIVITY_MANAGER; import android.app.ClientTransactionHandler; @@ -37,16 +36,17 @@ public class ActivityResultItem extends ClientTransactionItem { private List mResultInfoList; + /* TODO(b/78294732) @Override public int getPostExecutionState() { return ON_RESUME; - } + }*/ @Override public void execute(ClientTransactionHandler client, IBinder token, PendingTransactionActions pendingActions) { Trace.traceBegin(TRACE_TAG_ACTIVITY_MANAGER, "activityDeliverResult"); - client.handleSendResult(token, mResultInfoList); + client.handleSendResult(token, mResultInfoList, "ACTIVITY_RESULT"); Trace.traceEnd(TRACE_TAG_ACTIVITY_MANAGER); } diff --git a/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java b/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java index 3eefc362ab865..fe58116002f2d 100644 --- a/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java +++ b/core/tests/coretests/src/android/app/servertransaction/TransactionExecutorTests.java @@ -40,7 +40,10 @@ import static org.mockito.Mockito.when; import android.app.ActivityThread.ActivityClientRecord; import android.app.ClientTransactionHandler; +import android.app.servertransaction.ActivityLifecycleItem.LifecycleState; import android.os.IBinder; +import android.os.Parcel; +import android.os.Parcelable; import android.platform.test.annotations.Presubmit; import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; @@ -50,7 +53,6 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InOrder; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -231,12 +233,12 @@ public class TransactionExecutorTests { @Test public void testActivityResultRequiredStateResolution() { - ActivityResultItem activityResultItem = ActivityResultItem.obtain(new ArrayList<>()); + PostExecItem postExecItem = new PostExecItem(ON_RESUME); IBinder token = mock(IBinder.class); ClientTransaction transaction = ClientTransaction.obtain(null /* client */, token /* activityToken */); - transaction.addCallback(activityResultItem); + transaction.addCallback(postExecItem); // Verify resolution that should get to onPause mClientRecord.setState(ON_RESUME); @@ -395,4 +397,54 @@ public class TransactionExecutorTests { return mExecutorHelper.getLifecyclePath(mClientRecord.getLifecycleState(), finish, true /* excludeLastState */).toArray(); } + + /** A transaction item that requires some specific post-execution state. */ + private static class PostExecItem extends StubItem { + + @LifecycleState + private int mPostExecutionState; + + PostExecItem(@LifecycleState int state) { + mPostExecutionState = state; + } + + @Override + public int getPostExecutionState() { + return mPostExecutionState; + } + } + + /** Stub implementation of a transaction item that works as a base class for items in tests. */ + private static class StubItem extends ClientTransactionItem { + + private StubItem() { + } + + private StubItem(Parcel in) { + } + + @Override + public void execute(ClientTransactionHandler client, IBinder token, + PendingTransactionActions pendingActions) { + } + + @Override + public void recycle() { + } + + @Override + public void writeToParcel(Parcel dest, int flags) { + } + + public static final Parcelable.Creator CREATOR = + new Parcelable.Creator() { + public StubItem createFromParcel(Parcel in) { + return new StubItem(in); + } + + public StubItem[] newArray(int size) { + return new StubItem[size]; + } + }; + } } diff --git a/services/core/java/com/android/server/am/EventLogTags.logtags b/services/core/java/com/android/server/am/EventLogTags.logtags index 40b9e4fde874b..ed891dfb0e703 100644 --- a/services/core/java/com/android/server/am/EventLogTags.logtags +++ b/services/core/java/com/android/server/am/EventLogTags.logtags @@ -134,6 +134,8 @@ option java_package com.android.server.am 30059 am_on_start_called (User|1|5),(Component Name|3),(Reason|3) # The activity's onDestroy has been called. 30060 am_on_destroy_called (User|1|5),(Component Name|3),(Reason|3) +# The activity's onActivityResult has been called. +30062 am_on_activity_result_called (User|1|5),(Component Name|3),(Reason|3) # The task is being removed from its parent stack 30061 am_remove_task (Task ID|1|5), (Stack ID|1|5) \ No newline at end of file