From c39b8b11850964bc47154f2b750257be7962bb4f Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 4 Feb 2021 22:50:43 +0000 Subject: [PATCH] Revert "Put broadcast timekeeping on its own thread" This reverts commit 9c2775572ead41a8006e2da768900d663f904ceb. Reason for revert: Unanticipated performance degradation observed in user-switching scenarios; investigation will follow. Change-Id: Ie06e909a92be45a24cc46b458a372eadf302e6c4 --- core/tests/coretests/AndroidManifest.xml | 5 +-- .../android/app/activity/BroadcastTest.java | 39 +++++------------- .../app/activity/LaunchpadActivity.java | 40 +++++++------------ .../app/activity/LocalDeniedReceiver.java | 2 +- .../app/activity/RemoteDeniedReceiver.java | 2 +- .../server/am/ActivityManagerService.java | 15 ++----- 6 files changed, 32 insertions(+), 71 deletions(-) diff --git a/core/tests/coretests/AndroidManifest.xml b/core/tests/coretests/AndroidManifest.xml index 151a320494b46..bb826deb4eff4 100644 --- a/core/tests/coretests/AndroidManifest.xml +++ b/core/tests/coretests/AndroidManifest.xml @@ -35,8 +35,6 @@ android:label="@string/permlab_testDenied" android:description="@string/permdesc_testDenied" /> - - @@ -78,6 +76,7 @@ + @@ -1456,7 +1455,6 @@ @@ -1464,7 +1462,6 @@ diff --git a/core/tests/coretests/src/android/app/activity/BroadcastTest.java b/core/tests/coretests/src/android/app/activity/BroadcastTest.java index d79c2fe19ce28..0f81896692c00 100644 --- a/core/tests/coretests/src/android/app/activity/BroadcastTest.java +++ b/core/tests/coretests/src/android/app/activity/BroadcastTest.java @@ -56,8 +56,6 @@ public class BroadcastTest extends ActivityTestsBase { "com.android.frameworks.coretests.activity.BROADCAST_MULTI"; public static final String BROADCAST_ABORT = "com.android.frameworks.coretests.activity.BROADCAST_ABORT"; - public static final String BROADCAST_RESULT = - "com.android.frameworks.coretests.activity.BROADCAST_RESULT"; public static final String BROADCAST_STICKY1 = "com.android.frameworks.coretests.activity.BROADCAST_STICKY1"; @@ -108,14 +106,7 @@ public class BroadcastTest extends ActivityTestsBase { } public Intent makeBroadcastIntent(String action) { - return makeBroadcastIntent(action, false); - } - - public Intent makeBroadcastIntent(String action, boolean makeImplicit) { Intent intent = new Intent(action, null); - if (makeImplicit) { - intent.addFlags(intent.FLAG_RECEIVER_INCLUDE_BACKGROUND); - } intent.putExtra("caller", mCallTarget); return intent; } @@ -286,7 +277,7 @@ public class BroadcastTest extends ActivityTestsBase { map.putString("foo", "you"); map.putString("remove", "me"); getContext().sendOrderedBroadcast( - makeBroadcastIntent(BROADCAST_RESULT, true), + new Intent("com.android.frameworks.coretests.activity.BROADCAST_RESULT"), null, broadcastReceiver, null, 1, "foo", map); while (!broadcastReceiver.mHaveResult) { try { @@ -433,13 +424,10 @@ public class BroadcastTest extends ActivityTestsBase { public void testLocalReceivePermissionGranted() throws Exception { setExpectedReceivers(new String[]{RECEIVER_LOCAL}); - getContext().sendBroadcast(makeBroadcastIntent(BROADCAST_LOCAL_GRANTED, true)); + getContext().sendBroadcast(makeBroadcastIntent(BROADCAST_LOCAL_GRANTED)); waitForResultOrThrow(BROADCAST_TIMEOUT); } - /* - // TODO: multi-package test b/c self-target broadcasts are always allowed - // even when gated on ungranted permissions public void testLocalReceivePermissionDenied() throws Exception { setExpectedReceivers(new String[]{RECEIVER_RESULTS}); @@ -450,17 +438,16 @@ public class BroadcastTest extends ActivityTestsBase { }; getContext().sendOrderedBroadcast( - makeBroadcastIntent(BROADCAST_LOCAL_DENIED, true), + makeBroadcastIntent(BROADCAST_LOCAL_DENIED), null, finish, null, Activity.RESULT_CANCELED, null, null); waitForResultOrThrow(BROADCAST_TIMEOUT); } - */ public void testLocalBroadcastPermissionGranted() throws Exception { setExpectedReceivers(new String[]{RECEIVER_LOCAL}); getContext().sendBroadcast( - makeBroadcastIntent(BROADCAST_LOCAL, true), + makeBroadcastIntent(BROADCAST_LOCAL), PERMISSION_GRANTED); waitForResultOrThrow(BROADCAST_TIMEOUT); } @@ -475,7 +462,7 @@ public class BroadcastTest extends ActivityTestsBase { }; getContext().sendOrderedBroadcast( - makeBroadcastIntent(BROADCAST_LOCAL, true), + makeBroadcastIntent(BROADCAST_LOCAL), PERMISSION_DENIED, finish, null, Activity.RESULT_CANCELED, null, null); waitForResultOrThrow(BROADCAST_TIMEOUT); @@ -483,13 +470,10 @@ public class BroadcastTest extends ActivityTestsBase { public void testRemoteReceivePermissionGranted() throws Exception { setExpectedReceivers(new String[]{RECEIVER_REMOTE}); - getContext().sendBroadcast(makeBroadcastIntent(BROADCAST_REMOTE_GRANTED, true)); + getContext().sendBroadcast(makeBroadcastIntent(BROADCAST_REMOTE_GRANTED)); waitForResultOrThrow(BROADCAST_TIMEOUT); } - /* - // TODO: multi-package test b/c self-target broadcasts are always allowed - // even when gated on ungranted permissions public void testRemoteReceivePermissionDenied() throws Exception { setExpectedReceivers(new String[]{RECEIVER_RESULTS}); @@ -500,17 +484,16 @@ public class BroadcastTest extends ActivityTestsBase { }; getContext().sendOrderedBroadcast( - makeBroadcastIntent(BROADCAST_REMOTE_DENIED, true), + makeBroadcastIntent(BROADCAST_REMOTE_DENIED), null, finish, null, Activity.RESULT_CANCELED, null, null); waitForResultOrThrow(BROADCAST_TIMEOUT); } - */ public void testRemoteBroadcastPermissionGranted() throws Exception { setExpectedReceivers(new String[]{RECEIVER_REMOTE}); getContext().sendBroadcast( - makeBroadcastIntent(BROADCAST_REMOTE, true), + makeBroadcastIntent(BROADCAST_REMOTE), PERMISSION_GRANTED); waitForResultOrThrow(BROADCAST_TIMEOUT); } @@ -525,7 +508,7 @@ public class BroadcastTest extends ActivityTestsBase { }; getContext().sendOrderedBroadcast( - makeBroadcastIntent(BROADCAST_REMOTE, true), + makeBroadcastIntent(BROADCAST_REMOTE), PERMISSION_DENIED, finish, null, Activity.RESULT_CANCELED, null, null); waitForResultOrThrow(BROADCAST_TIMEOUT); @@ -533,13 +516,13 @@ public class BroadcastTest extends ActivityTestsBase { public void testReceiverCanNotRegister() throws Exception { setExpectedReceivers(new String[]{RECEIVER_LOCAL}); - getContext().sendBroadcast(makeBroadcastIntent(BROADCAST_FAIL_REGISTER, true)); + getContext().sendBroadcast(makeBroadcastIntent(BROADCAST_FAIL_REGISTER)); waitForResultOrThrow(BROADCAST_TIMEOUT); } public void testReceiverCanNotBind() throws Exception { setExpectedReceivers(new String[]{RECEIVER_LOCAL}); - getContext().sendBroadcast(makeBroadcastIntent(BROADCAST_FAIL_BIND, true)); + getContext().sendBroadcast(makeBroadcastIntent(BROADCAST_FAIL_BIND)); waitForResultOrThrow(BROADCAST_TIMEOUT); } diff --git a/core/tests/coretests/src/android/app/activity/LaunchpadActivity.java b/core/tests/coretests/src/android/app/activity/LaunchpadActivity.java index 0b21fa90c1c4d..766245600d13b 100644 --- a/core/tests/coretests/src/android/app/activity/LaunchpadActivity.java +++ b/core/tests/coretests/src/android/app/activity/LaunchpadActivity.java @@ -253,16 +253,16 @@ public class LaunchpadActivity extends Activity { sendBroadcast(makeBroadcastIntent(BROADCAST_REGISTERED)); } else if (BROADCAST_LOCAL.equals(action)) { setExpectedReceivers(new String[]{RECEIVER_LOCAL}); - sendBroadcast(makeBroadcastIntent(BROADCAST_LOCAL, true)); + sendBroadcast(makeBroadcastIntent(BROADCAST_LOCAL)); } else if (BROADCAST_REMOTE.equals(action)) { setExpectedReceivers(new String[]{RECEIVER_REMOTE}); - sendBroadcast(makeBroadcastIntent(BROADCAST_REMOTE, true)); + sendBroadcast(makeBroadcastIntent(BROADCAST_REMOTE)); } else if (BROADCAST_ALL.equals(action)) { setExpectedReceivers(new String[]{ RECEIVER_REMOTE, RECEIVER_REG, RECEIVER_LOCAL}); registerMyReceiver(new IntentFilter(BROADCAST_ALL)); sCallingTest.addIntermediate("after-register"); - sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_ALL, true), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_ALL), null); } else if (BROADCAST_MULTI.equals(action)) { setExpectedReceivers(new String[]{ RECEIVER_REMOTE, RECEIVER_REG, RECEIVER_LOCAL, @@ -277,26 +277,23 @@ public class LaunchpadActivity extends Activity { RECEIVER_REMOTE, RECEIVER_LOCAL}); registerMyReceiver(new IntentFilter(BROADCAST_ALL)); sCallingTest.addIntermediate("after-register"); - final Intent allIntent = makeBroadcastIntent(BROADCAST_ALL, true); - final Intent localIntent = makeBroadcastIntent(BROADCAST_LOCAL, true); - final Intent remoteIntent = makeBroadcastIntent(BROADCAST_REMOTE, true); - sendOrderedBroadcast(allIntent, null); - sendOrderedBroadcast(allIntent, null); - sendOrderedBroadcast(allIntent, null); - sendOrderedBroadcast(localIntent, null); - sendOrderedBroadcast(remoteIntent, null); - sendOrderedBroadcast(localIntent, null); - sendOrderedBroadcast(remoteIntent, null); - sendOrderedBroadcast(allIntent, null); - sendOrderedBroadcast(allIntent, null); - sendOrderedBroadcast(allIntent, null); - sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_REPEAT, true), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_ALL), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_ALL), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_ALL), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_LOCAL), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_REMOTE), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_LOCAL), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_REMOTE), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_ALL), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_ALL), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_ALL), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_REPEAT), null); } else if (BROADCAST_ABORT.equals(action)) { setExpectedReceivers(new String[]{ RECEIVER_REMOTE, RECEIVER_ABORT}); registerMyReceiver(new IntentFilter(BROADCAST_ABORT)); sCallingTest.addIntermediate("after-register"); - sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_ABORT, true), null); + sendOrderedBroadcast(makeBroadcastIntent(BROADCAST_ABORT), null); } else if (BROADCAST_STICKY1.equals(action)) { setExpectedReceivers(new String[]{RECEIVER_REG}); setExpectedData(new String[]{DATA_1}); @@ -439,14 +436,7 @@ public class LaunchpadActivity extends Activity { } private Intent makeBroadcastIntent(String action) { - return makeBroadcastIntent(action, false); - } - - private Intent makeBroadcastIntent(String action, boolean makeImplicit) { Intent intent = new Intent(action, null); - if (makeImplicit) { - intent.addFlags(Intent.FLAG_RECEIVER_INCLUDE_BACKGROUND); - } intent.putExtra("caller", mCallTarget); return intent; } diff --git a/core/tests/coretests/src/android/app/activity/LocalDeniedReceiver.java b/core/tests/coretests/src/android/app/activity/LocalDeniedReceiver.java index 16ea73f309376..2120a1db463c0 100644 --- a/core/tests/coretests/src/android/app/activity/LocalDeniedReceiver.java +++ b/core/tests/coretests/src/android/app/activity/LocalDeniedReceiver.java @@ -23,7 +23,7 @@ import android.os.RemoteException; import android.os.IBinder; import android.os.Parcel; -public class LocalDeniedReceiver extends BroadcastReceiver { +class LocalDeniedReceiver extends BroadcastReceiver { public LocalDeniedReceiver() { } diff --git a/core/tests/coretests/src/android/app/activity/RemoteDeniedReceiver.java b/core/tests/coretests/src/android/app/activity/RemoteDeniedReceiver.java index 5c1ded93e1c34..7c89346e820df 100644 --- a/core/tests/coretests/src/android/app/activity/RemoteDeniedReceiver.java +++ b/core/tests/coretests/src/android/app/activity/RemoteDeniedReceiver.java @@ -23,7 +23,7 @@ import android.os.RemoteException; import android.os.IBinder; import android.os.Parcel; -public class RemoteDeniedReceiver extends BroadcastReceiver { +class RemoteDeniedReceiver extends BroadcastReceiver { public RemoteDeniedReceiver() { } diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 1396654e37c83..05f2fa096e4d0 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -237,7 +237,6 @@ import android.os.DropBoxManager; import android.os.FactoryTest; import android.os.FileUtils; import android.os.Handler; -import android.os.HandlerThread; import android.os.IBinder; import android.os.IDeviceIdentifiersPolicyService; import android.os.IPermissionController; @@ -2090,19 +2089,11 @@ public class ActivityManagerService extends IActivityManager.Stub mEnableOffloadQueue = SystemProperties.getBoolean( "persist.device_config.activity_manager_native_boot.offload_queue_enabled", false); - // Decouple broadcast-related timing operations from other OS activity by - // using a dedicated thread. Sharing this thread between queues is safe - // because we know the nature of the activity on it and can't stall - // unexpectedly. - HandlerThread broadcastThread = new HandlerThread("broadcast"); - broadcastThread.start(); - Handler broadcastHandler = broadcastThread.getThreadHandler(); - - mFgBroadcastQueue = new BroadcastQueue(this, broadcastHandler, + mFgBroadcastQueue = new BroadcastQueue(this, mHandler, "foreground", foreConstants, false); - mBgBroadcastQueue = new BroadcastQueue(this, broadcastHandler, + mBgBroadcastQueue = new BroadcastQueue(this, mHandler, "background", backConstants, true); - mOffloadBroadcastQueue = new BroadcastQueue(this, broadcastHandler, + mOffloadBroadcastQueue = new BroadcastQueue(this, mHandler, "offload", offloadConstants, true); mBroadcastQueues[0] = mFgBroadcastQueue; mBroadcastQueues[1] = mBgBroadcastQueue;