From 9e719d3639d56bd1f47bdce77451582cdc8ffdbd Mon Sep 17 00:00:00 2001 From: Sudheer Shanka Date: Wed, 4 Sep 2024 05:56:08 +0000 Subject: Cache sticky broadcast intents on the client side. When a client makes a sticky broadcast intent query, cache the result on the client side to avoid having to make a call to the system_server process for subsequent queries. Bug: 356148006 Test: atest tests/broadcasts/unit/src/android/app/BroadcastStickyCacheTest.java Flag: android.app.use_sticky_bcast_cache Change-Id: I8a89b42e6696e9b8bc4bbf690a41b52531083b6c --- core/java/android/app/BroadcastStickyCache.java | 175 ++++++++++++++ core/java/android/app/ContextImpl.java | 17 +- core/java/android/app/TEST_MAPPING | 4 + core/java/android/app/activity_manager.aconfig | 11 + .../com/android/server/am/BroadcastController.java | 36 ++- tests/broadcasts/OWNERS | 2 + tests/broadcasts/unit/Android.bp | 44 ++++ tests/broadcasts/unit/AndroidManifest.xml | 27 +++ tests/broadcasts/unit/AndroidTest.xml | 29 +++ tests/broadcasts/unit/TEST_MAPPING | 15 ++ .../src/android/app/BroadcastStickyCacheTest.java | 256 +++++++++++++++++++++ 11 files changed, 609 insertions(+), 7 deletions(-) create mode 100644 core/java/android/app/BroadcastStickyCache.java create mode 100644 tests/broadcasts/OWNERS create mode 100644 tests/broadcasts/unit/Android.bp create mode 100644 tests/broadcasts/unit/AndroidManifest.xml create mode 100644 tests/broadcasts/unit/AndroidTest.xml create mode 100644 tests/broadcasts/unit/TEST_MAPPING create mode 100644 tests/broadcasts/unit/src/android/app/BroadcastStickyCacheTest.java diff --git a/core/java/android/app/BroadcastStickyCache.java b/core/java/android/app/BroadcastStickyCache.java new file mode 100644 index 000000000000..4087df9d8097 --- /dev/null +++ b/core/java/android/app/BroadcastStickyCache.java @@ -0,0 +1,175 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package android.app; + +import android.annotation.IntRange; +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.content.Intent; +import android.content.IntentFilter; +import android.os.SystemProperties; +import android.util.ArrayMap; + +import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; + +import java.util.ArrayList; + +/** @hide */ +public class BroadcastStickyCache { + + @GuardedBy("sCachedStickyBroadcasts") + private static final ArrayList sCachedStickyBroadcasts = + new ArrayList<>(); + + @GuardedBy("sCachedPropertyHandles") + private static final ArrayMap sCachedPropertyHandles = + new ArrayMap<>(); + + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) + public static boolean useCache(@Nullable IntentFilter filter) { + if (!Flags.useStickyBcastCache()) { + return false; + } + if (filter == null || filter.safeCountActions() != 1) { + return false; + } + synchronized (sCachedStickyBroadcasts) { + final CachedStickyBroadcast cachedStickyBroadcast = getValueUncheckedLocked(filter); + if (cachedStickyBroadcast == null) { + return false; + } + final long version = cachedStickyBroadcast.propertyHandle.getLong(-1 /* def */); + return version > 0 && cachedStickyBroadcast.version == version; + } + } + + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) + public static void add(@Nullable IntentFilter filter, @Nullable Intent intent) { + if (!Flags.useStickyBcastCache()) { + return; + } + if (filter == null || filter.safeCountActions() != 1) { + return; + } + synchronized (sCachedStickyBroadcasts) { + CachedStickyBroadcast cachedStickyBroadcast = getValueUncheckedLocked(filter); + if (cachedStickyBroadcast == null) { + final String key = getKey(filter.getAction(0)); + final SystemProperties.Handle handle = SystemProperties.find(key); + final long version = handle == null ? -1 : handle.getLong(-1 /* def */); + if (version == -1) { + return; + } + cachedStickyBroadcast = new CachedStickyBroadcast(filter, handle); + sCachedStickyBroadcasts.add(cachedStickyBroadcast); + cachedStickyBroadcast.intent = intent; + cachedStickyBroadcast.version = version; + } else { + cachedStickyBroadcast.intent = intent; + cachedStickyBroadcast.version = cachedStickyBroadcast.propertyHandle + .getLong(-1 /* def */); + } + } + } + + @VisibleForTesting + @NonNull + public static String getKey(@NonNull String action) { + return "cache_key.system_server.sticky_bcast." + action; + } + + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) + @Nullable + public static Intent getIntentUnchecked(@NonNull IntentFilter filter) { + synchronized (sCachedStickyBroadcasts) { + final CachedStickyBroadcast cachedStickyBroadcast = getValueUncheckedLocked(filter); + return cachedStickyBroadcast.intent; + } + } + + @GuardedBy("sCachedStickyBroadcasts") + @Nullable + private static CachedStickyBroadcast getValueUncheckedLocked(@NonNull IntentFilter filter) { + for (int i = sCachedStickyBroadcasts.size() - 1; i >= 0; --i) { + final CachedStickyBroadcast cachedStickyBroadcast = sCachedStickyBroadcasts.get(i); + if (IntentFilter.filterEquals(filter, cachedStickyBroadcast.filter)) { + return cachedStickyBroadcast; + } + } + return null; + } + + public static void incrementVersion(@NonNull String action) { + if (!Flags.useStickyBcastCache()) { + return; + } + final String key = getKey(action); + synchronized (sCachedPropertyHandles) { + SystemProperties.Handle handle = sCachedPropertyHandles.get(key); + final long version; + if (handle == null) { + handle = SystemProperties.find(key); + if (handle != null) { + sCachedPropertyHandles.put(key, handle); + } + } + version = handle == null ? 0 : handle.getLong(0 /* def */); + SystemProperties.set(key, String.valueOf(version + 1)); + if (handle == null) { + sCachedPropertyHandles.put(key, SystemProperties.find(key)); + } + } + } + + public static void incrementVersionIfExists(@NonNull String action) { + if (!Flags.useStickyBcastCache()) { + return; + } + final String key = getKey(action); + synchronized (sCachedPropertyHandles) { + final SystemProperties.Handle handle = sCachedPropertyHandles.get(key); + if (handle == null) { + return; + } + final long version = handle.getLong(0 /* def */); + SystemProperties.set(key, String.valueOf(version + 1)); + } + } + + @VisibleForTesting + public static void clearForTest() { + synchronized (sCachedStickyBroadcasts) { + sCachedStickyBroadcasts.clear(); + } + synchronized (sCachedPropertyHandles) { + sCachedPropertyHandles.clear(); + } + } + + private static final class CachedStickyBroadcast { + @NonNull public final IntentFilter filter; + @Nullable public Intent intent; + @IntRange(from = 0) public long version; + @NonNull public final SystemProperties.Handle propertyHandle; + + CachedStickyBroadcast(@NonNull IntentFilter filter, + @NonNull SystemProperties.Handle propertyHandle) { + this.filter = filter; + this.propertyHandle = propertyHandle; + } + } +} diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java index 90fba2962a23..ecef0db46bb2 100644 --- a/core/java/android/app/ContextImpl.java +++ b/core/java/android/app/ContextImpl.java @@ -1921,10 +1921,19 @@ class ContextImpl extends Context { } } try { - final Intent intent = ActivityManager.getService().registerReceiverWithFeature( - mMainThread.getApplicationThread(), mBasePackageName, getAttributionTag(), - AppOpsManager.toReceiverId(receiver), rd, filter, broadcastPermission, userId, - flags); + final Intent intent; + if (receiver == null && BroadcastStickyCache.useCache(filter)) { + intent = BroadcastStickyCache.getIntentUnchecked(filter); + } else { + intent = ActivityManager.getService().registerReceiverWithFeature( + mMainThread.getApplicationThread(), mBasePackageName, getAttributionTag(), + AppOpsManager.toReceiverId(receiver), rd, filter, broadcastPermission, + userId, + flags); + if (receiver == null) { + BroadcastStickyCache.add(filter, intent); + } + } if (intent != null) { intent.setExtrasClassLoader(getClassLoader()); // TODO: determine at registration time if caller is diff --git a/core/java/android/app/TEST_MAPPING b/core/java/android/app/TEST_MAPPING index 5ed1f4e35533..637187e01160 100644 --- a/core/java/android/app/TEST_MAPPING +++ b/core/java/android/app/TEST_MAPPING @@ -177,6 +177,10 @@ { "file_patterns": ["(/|^)AppOpsManager.java"], "name": "CtsAppOpsTestCases" + }, + { + "file_patterns": ["(/|^)BroadcastStickyCache.java"], + "name": "BroadcastUnitTests" } ] } diff --git a/core/java/android/app/activity_manager.aconfig b/core/java/android/app/activity_manager.aconfig index 38bd576d607a..56488e7cd296 100644 --- a/core/java/android/app/activity_manager.aconfig +++ b/core/java/android/app/activity_manager.aconfig @@ -147,3 +147,14 @@ flag { purpose: PURPOSE_BUGFIX } } + +flag { + namespace: "backstage_power" + name: "use_sticky_bcast_cache" + description: "Use cache for sticky broadcast intents" + is_fixed_read_only: true + bug: "356148006" + metadata { + purpose: PURPOSE_BUGFIX + } +} diff --git a/services/core/java/com/android/server/am/BroadcastController.java b/services/core/java/com/android/server/am/BroadcastController.java index f7085b4b26b4..934ce6953d14 100644 --- a/services/core/java/com/android/server/am/BroadcastController.java +++ b/services/core/java/com/android/server/am/BroadcastController.java @@ -57,6 +57,7 @@ import android.app.ApplicationExitInfo; import android.app.ApplicationThreadConstants; import android.app.BackgroundStartPrivileges; import android.app.BroadcastOptions; +import android.app.BroadcastStickyCache; import android.app.IApplicationThread; import android.app.compat.CompatChanges; import android.appwidget.AppWidgetManager; @@ -685,6 +686,8 @@ class BroadcastController { boolean serialized, boolean sticky, int userId) { mService.enforceNotIsolatedCaller("broadcastIntent"); + boolean isCallerCore; + int result; synchronized (mService) { intent = verifyBroadcastLocked(intent); @@ -706,7 +709,8 @@ class BroadcastController { final long origId = Binder.clearCallingIdentity(); try { - return broadcastIntentLocked(callerApp, + isCallerCore = UserHandle.isCore(callingUid); + result = broadcastIntentLocked(callerApp, callerApp != null ? callerApp.info.packageName : null, callingFeatureId, intent, resolvedType, resultToApp, resultTo, resultCode, resultData, resultExtras, requiredPermissions, excludedPermissions, excludedPackages, @@ -717,6 +721,10 @@ class BroadcastController { Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); } } + if (sticky && isCallerCore && result == ActivityManager.BROADCAST_SUCCESS) { + BroadcastStickyCache.incrementVersion(intent.getAction()); + } + return result; } // Not the binder call surface @@ -727,6 +735,7 @@ class BroadcastController { boolean serialized, boolean sticky, int userId, BackgroundStartPrivileges backgroundStartPrivileges, @Nullable int[] broadcastAllowList) { + int result; synchronized (mService) { intent = verifyBroadcastLocked(intent); @@ -734,7 +743,7 @@ class BroadcastController { String[] requiredPermissions = requiredPermission == null ? null : new String[] {requiredPermission}; try { - return broadcastIntentLocked(null, packageName, featureId, intent, resolvedType, + result = broadcastIntentLocked(null, packageName, featureId, intent, resolvedType, resultToApp, resultTo, resultCode, resultData, resultExtras, requiredPermissions, null, null, OP_NONE, bOptions, serialized, sticky, -1, uid, realCallingUid, realCallingPid, userId, @@ -744,6 +753,10 @@ class BroadcastController { Binder.restoreCallingIdentity(origId); } } + if (sticky && result == ActivityManager.BROADCAST_SUCCESS) { + BroadcastStickyCache.incrementVersion(intent.getAction()); + } + return result; } @GuardedBy("mService") @@ -1442,6 +1455,7 @@ class BroadcastController { list.add(StickyBroadcast.create(new Intent(intent), deferUntilActive, callingUid, callerAppProcessState, resolvedType)); } + BroadcastStickyCache.incrementVersion(intent.getAction()); } } @@ -1708,6 +1722,7 @@ class BroadcastController { Slog.w(TAG, msg); throw new SecurityException(msg); } + final ArrayList changedStickyBroadcasts = new ArrayList<>(); synchronized (mStickyBroadcasts) { ArrayMap> stickies = mStickyBroadcasts.get(userId); if (stickies != null) { @@ -1724,12 +1739,16 @@ class BroadcastController { if (list.size() <= 0) { stickies.remove(intent.getAction()); } + changedStickyBroadcasts.add(intent.getAction()); } if (stickies.size() <= 0) { mStickyBroadcasts.remove(userId); } } } + for (int i = changedStickyBroadcasts.size() - 1; i >= 0; --i) { + BroadcastStickyCache.incrementVersionIfExists(changedStickyBroadcasts.get(i)); + } } void finishReceiver(IBinder caller, int resultCode, String resultData, @@ -1892,7 +1911,9 @@ class BroadcastController { private void sendPackageBroadcastLocked(int cmd, String[] packages, int userId) { mService.mProcessList.sendPackageBroadcastLocked(cmd, packages, userId); - }private List collectReceiverComponents( + } + + private List collectReceiverComponents( Intent intent, String resolvedType, int callingUid, int callingPid, int[] users, int[] broadcastAllowList) { // TODO: come back and remove this assumption to triage all broadcasts @@ -2108,9 +2129,18 @@ class BroadcastController { } void removeStickyBroadcasts(int userId) { + final ArrayList changedStickyBroadcasts = new ArrayList<>(); synchronized (mStickyBroadcasts) { + final ArrayMap> stickies = + mStickyBroadcasts.get(userId); + if (stickies != null) { + changedStickyBroadcasts.addAll(stickies.keySet()); + } mStickyBroadcasts.remove(userId); } + for (int i = changedStickyBroadcasts.size() - 1; i >= 0; --i) { + BroadcastStickyCache.incrementVersionIfExists(changedStickyBroadcasts.get(i)); + } } @NeverCompile diff --git a/tests/broadcasts/OWNERS b/tests/broadcasts/OWNERS new file mode 100644 index 000000000000..d2e1f815e8dc --- /dev/null +++ b/tests/broadcasts/OWNERS @@ -0,0 +1,2 @@ +# Bug component: 316181 +include platform/frameworks/base:/BROADCASTS_OWNERS diff --git a/tests/broadcasts/unit/Android.bp b/tests/broadcasts/unit/Android.bp new file mode 100644 index 000000000000..069247735cd4 --- /dev/null +++ b/tests/broadcasts/unit/Android.bp @@ -0,0 +1,44 @@ +// +// Copyright (C) 2024 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package { + // See: http://go/android-license-faq + // A large-scale-change added 'default_applicable_licenses' to import + // all of the 'license_kinds' from "frameworks_base_license" + // to get the below license kinds: + // SPDX-license-identifier-Apache-2.0 + default_applicable_licenses: ["frameworks_base_license"], +} + +android_test { + name: "BroadcastUnitTests", + srcs: ["src/**/*.java"], + defaults: [ + "modules-utils-extended-mockito-rule-defaults", + ], + static_libs: [ + "androidx.test.runner", + "androidx.test.rules", + "androidx.test.ext.junit", + "mockito-target-extended-minus-junit4", + "truth", + "flag-junit", + "android.app.flags-aconfig-java", + ], + certificate: "platform", + platform_apis: true, + test_suites: ["device-tests"], +} diff --git a/tests/broadcasts/unit/AndroidManifest.xml b/tests/broadcasts/unit/AndroidManifest.xml new file mode 100644 index 000000000000..e9c5248e4d98 --- /dev/null +++ b/tests/broadcasts/unit/AndroidManifest.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + \ No newline at end of file diff --git a/tests/broadcasts/unit/AndroidTest.xml b/tests/broadcasts/unit/AndroidTest.xml new file mode 100644 index 000000000000..b91e4783b69e --- /dev/null +++ b/tests/broadcasts/unit/AndroidTest.xml @@ -0,0 +1,29 @@ + + + \ No newline at end of file diff --git a/tests/broadcasts/unit/TEST_MAPPING b/tests/broadcasts/unit/TEST_MAPPING new file mode 100644 index 000000000000..0e824c54e92e --- /dev/null +++ b/tests/broadcasts/unit/TEST_MAPPING @@ -0,0 +1,15 @@ +{ + "postsubmit": [ + { + "name": "BroadcastUnitTests", + "options": [ + { + "exclude-annotation": "androidx.test.filters.FlakyTest" + }, + { + "exclude-annotation": "org.junit.Ignore" + } + ] + } + ] +} diff --git a/tests/broadcasts/unit/src/android/app/BroadcastStickyCacheTest.java b/tests/broadcasts/unit/src/android/app/BroadcastStickyCacheTest.java new file mode 100644 index 000000000000..f8fd04bafbf3 --- /dev/null +++ b/tests/broadcasts/unit/src/android/app/BroadcastStickyCacheTest.java @@ -0,0 +1,256 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package android.app; + +import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; + +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; + +import android.content.Intent; +import android.content.IntentFilter; +import android.os.BatteryManager; +import android.os.Bundle; +import android.os.SystemProperties; +import android.platform.test.annotations.EnableFlags; +import android.platform.test.flag.junit.SetFlagsRule; +import android.util.ArrayMap; + +import androidx.annotation.GuardedBy; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.filters.SmallTest; + +import com.android.modules.utils.testing.ExtendedMockitoRule; + +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mockito; + +@EnableFlags(Flags.FLAG_USE_STICKY_BCAST_CACHE) +@RunWith(AndroidJUnit4.class) +@SmallTest +public class BroadcastStickyCacheTest { + @ClassRule + public static final SetFlagsRule.ClassRule mClassRule = new SetFlagsRule.ClassRule(); + @Rule + public final SetFlagsRule mSetFlagsRule = mClassRule.createSetFlagsRule(); + + @Rule + public final ExtendedMockitoRule mExtendedMockitoRule = new ExtendedMockitoRule.Builder(this) + .mockStatic(SystemProperties.class) + .build(); + + private static final String ACTION_TEST_RED = "action.test.red"; + private static final String ACTION_TEST_GREEN = "action.test.green"; + private static final String PROP_TEST_GREEN = BroadcastStickyCache.getKey(ACTION_TEST_GREEN); + + private final TestSystemProps mTestSystemProps = new TestSystemProps(); + + @Before + public void setUp() { + doAnswer(invocation -> { + final String name = invocation.getArgument(0); + final long value = Long.parseLong(invocation.getArgument(1)); + mTestSystemProps.add(name, value); + return null; + }).when(() -> SystemProperties.set(anyString(), anyString())); + doAnswer(invocation -> { + final String name = invocation.getArgument(0); + final TestSystemProps.Handle testHandle = mTestSystemProps.query(name); + if (testHandle == null) { + return null; + } + final SystemProperties.Handle handle = Mockito.mock(SystemProperties.Handle.class); + doAnswer(handleInvocation -> testHandle.getLong(-1)).when(handle).getLong(anyLong()); + return handle; + }).when(() -> SystemProperties.find(anyString())); + } + + @After + public void tearDown() { + mTestSystemProps.clear(); + BroadcastStickyCache.clearForTest(); + } + + @Test + public void testUseCache_nullFilter() { + assertThat(BroadcastStickyCache.useCache(null)).isEqualTo(false); + } + + @Test + public void testUseCache_noActions() { + final IntentFilter filter = new IntentFilter(); + assertThat(BroadcastStickyCache.useCache(filter)).isEqualTo(false); + } + + @Test + public void testUseCache_multipleActions() { + final IntentFilter filter = new IntentFilter(); + filter.addAction(ACTION_TEST_RED); + filter.addAction(ACTION_TEST_GREEN); + assertThat(BroadcastStickyCache.useCache(filter)).isEqualTo(false); + } + + @Test + public void testUseCache_valueNotSet() { + final IntentFilter filter = new IntentFilter(ACTION_TEST_GREEN); + assertThat(BroadcastStickyCache.useCache(filter)).isEqualTo(false); + } + + @Test + public void testUseCache() { + final IntentFilter filter = new IntentFilter(ACTION_TEST_GREEN); + final Intent intent = new Intent(ACTION_TEST_GREEN) + .putExtra(BatteryManager.EXTRA_LEVEL, 90); + BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + BroadcastStickyCache.add(filter, intent); + assertThat(BroadcastStickyCache.useCache(filter)).isEqualTo(true); + } + + @Test + public void testUseCache_versionMismatch() { + final IntentFilter filter = new IntentFilter(ACTION_TEST_GREEN); + final Intent intent = new Intent(ACTION_TEST_GREEN) + .putExtra(BatteryManager.EXTRA_LEVEL, 90); + BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + BroadcastStickyCache.add(filter, intent); + BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + + assertThat(BroadcastStickyCache.useCache(filter)).isEqualTo(false); + } + + @Test + public void testAdd() { + final IntentFilter filter = new IntentFilter(ACTION_TEST_GREEN); + Intent intent = new Intent(ACTION_TEST_GREEN) + .putExtra(BatteryManager.EXTRA_LEVEL, 90); + BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + BroadcastStickyCache.add(filter, intent); + assertThat(BroadcastStickyCache.useCache(filter)).isEqualTo(true); + Intent actualIntent = BroadcastStickyCache.getIntentUnchecked(filter); + assertThat(actualIntent).isNotNull(); + assertEquals(actualIntent, intent); + + intent = new Intent(ACTION_TEST_GREEN) + .putExtra(BatteryManager.EXTRA_LEVEL, 99); + BroadcastStickyCache.add(filter, intent); + actualIntent = BroadcastStickyCache.getIntentUnchecked(filter); + assertThat(actualIntent).isNotNull(); + assertEquals(actualIntent, intent); + } + + @Test + public void testIncrementVersion_propExists() { + SystemProperties.set(PROP_TEST_GREEN, String.valueOf(100)); + + BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(101); + BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(102); + } + + @Test + public void testIncrementVersion_propNotExists() { + // Verify that the property doesn't exist + assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(-1); + + BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(1); + BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(2); + } + + @Test + public void testIncrementVersionIfExists_propExists() { + BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + + BroadcastStickyCache.incrementVersionIfExists(ACTION_TEST_GREEN); + assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(2); + BroadcastStickyCache.incrementVersionIfExists(ACTION_TEST_GREEN); + assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(3); + } + + @Test + public void testIncrementVersionIfExists_propNotExists() { + // Verify that the property doesn't exist + assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(-1); + + BroadcastStickyCache.incrementVersionIfExists(ACTION_TEST_GREEN); + assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(-1); + // Verify that property is not added as part of the querying. + BroadcastStickyCache.incrementVersionIfExists(ACTION_TEST_GREEN); + assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(-1); + } + + private void assertEquals(Intent actualIntent, Intent expectedIntent) { + assertThat(actualIntent.getAction()).isEqualTo(expectedIntent.getAction()); + assertEquals(actualIntent.getExtras(), expectedIntent.getExtras()); + } + + private void assertEquals(Bundle actualExtras, Bundle expectedExtras) { + assertWithMessage("Extras expected=%s, actual=%s", expectedExtras, actualExtras) + .that(actualExtras.kindofEquals(expectedExtras)).isTrue(); + } + + private static final class TestSystemProps { + @GuardedBy("mSysProps") + private final ArrayMap mSysProps = new ArrayMap<>(); + + public void add(String name, long value) { + synchronized (mSysProps) { + mSysProps.put(name, value); + } + } + + public long get(String name, long defaultValue) { + synchronized (mSysProps) { + final int idx = mSysProps.indexOfKey(name); + return idx >= 0 ? mSysProps.valueAt(idx) : defaultValue; + } + } + + public Handle query(String name) { + synchronized (mSysProps) { + return mSysProps.containsKey(name) ? new Handle(name) : null; + } + } + + public void clear() { + synchronized (mSysProps) { + mSysProps.clear(); + } + } + + public class Handle { + private final String mName; + + Handle(String name) { + mName = name; + } + + public long getLong(long defaultValue) { + return get(mName, defaultValue); + } + } + } +} -- cgit v1.2.3-59-g8ed1b From 353ff476a1ce1600bc4fb048288bb95f9d6735b0 Mon Sep 17 00:00:00 2001 From: Sudheer Shanka Date: Mon, 23 Sep 2024 03:42:41 +0000 Subject: Include the hardcoded list of sticky broadcasts to cache. These broadcast actions will be used in determining whether to cache the broadcast intent and whether to modify the system property. Bug: 356148006 Test: atest tests/broadcasts/unit/src/android/app/BroadcastStickyCacheTest.java Flag: android.app.use_sticky_bcast_cache Change-Id: Ia4f4b6ef05016d654e9c9c04967f669c6c57e34b --- core/java/android/app/BroadcastStickyCache.java | 74 ++++++++++++++++++--- .../com/android/server/am/BroadcastController.java | 4 +- .../src/android/app/BroadcastStickyCacheTest.java | 76 +++++++++++----------- 3 files changed, 104 insertions(+), 50 deletions(-) diff --git a/core/java/android/app/BroadcastStickyCache.java b/core/java/android/app/BroadcastStickyCache.java index 4087df9d8097..d6f061be3b00 100644 --- a/core/java/android/app/BroadcastStickyCache.java +++ b/core/java/android/app/BroadcastStickyCache.java @@ -20,17 +20,54 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Intent; import android.content.IntentFilter; +import android.hardware.usb.UsbManager; +import android.media.AudioManager; +import android.net.ConnectivityManager; +import android.net.TetheringManager; +import android.net.nsd.NsdManager; +import android.net.wifi.WifiManager; +import android.net.wifi.p2p.WifiP2pManager; import android.os.SystemProperties; +import android.os.UpdateLock; +import android.telephony.TelephonyManager; import android.util.ArrayMap; +import android.view.WindowManagerPolicyConstants; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.util.ArrayUtils; import java.util.ArrayList; /** @hide */ public class BroadcastStickyCache { + private static final String[] CACHED_BROADCAST_ACTIONS = { + AudioManager.ACTION_HDMI_AUDIO_PLUG, + AudioManager.ACTION_HEADSET_PLUG, + AudioManager.ACTION_SCO_AUDIO_STATE_CHANGED, + AudioManager.ACTION_SCO_AUDIO_STATE_UPDATED, + AudioManager.INTERNAL_RINGER_MODE_CHANGED_ACTION, + AudioManager.RINGER_MODE_CHANGED_ACTION, + ConnectivityManager.CONNECTIVITY_ACTION, + Intent.ACTION_BATTERY_CHANGED, + Intent.ACTION_DEVICE_STORAGE_FULL, + Intent.ACTION_DEVICE_STORAGE_LOW, + Intent.ACTION_SIM_STATE_CHANGED, + NsdManager.ACTION_NSD_STATE_CHANGED, + TelephonyManager.ACTION_SERVICE_PROVIDERS_UPDATED, + TetheringManager.ACTION_TETHER_STATE_CHANGED, + UpdateLock.UPDATE_LOCK_CHANGED, + UsbManager.ACTION_USB_STATE, + WifiManager.ACTION_WIFI_SCAN_AVAILABILITY_CHANGED, + WifiManager.NETWORK_STATE_CHANGED_ACTION, + WifiManager.SUPPLICANT_STATE_CHANGED_ACTION, + WifiManager.WIFI_STATE_CHANGED_ACTION, + WifiP2pManager.WIFI_P2P_STATE_CHANGED_ACTION, + WindowManagerPolicyConstants.ACTION_HDMI_PLUGGED, + "android.net.conn.INET_CONDITION_ACTION" // ConnectivityManager.INET_CONDITION_ACTION + }; + @GuardedBy("sCachedStickyBroadcasts") private static final ArrayList sCachedStickyBroadcasts = new ArrayList<>(); @@ -41,10 +78,7 @@ public class BroadcastStickyCache { @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) public static boolean useCache(@Nullable IntentFilter filter) { - if (!Flags.useStickyBcastCache()) { - return false; - } - if (filter == null || filter.safeCountActions() != 1) { + if (!shouldCache(filter)) { return false; } synchronized (sCachedStickyBroadcasts) { @@ -59,10 +93,7 @@ public class BroadcastStickyCache { @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) public static void add(@Nullable IntentFilter filter, @Nullable Intent intent) { - if (!Flags.useStickyBcastCache()) { - return; - } - if (filter == null || filter.safeCountActions() != 1) { + if (!shouldCache(filter)) { return; } synchronized (sCachedStickyBroadcasts) { @@ -86,6 +117,19 @@ public class BroadcastStickyCache { } } + private static boolean shouldCache(@Nullable IntentFilter filter) { + if (!Flags.useStickyBcastCache()) { + return false; + } + if (filter == null || filter.safeCountActions() != 1) { + return false; + } + if (!ArrayUtils.contains(CACHED_BROADCAST_ACTIONS, filter.getAction(0))) { + return false; + } + return true; + } + @VisibleForTesting @NonNull public static String getKey(@NonNull String action) { @@ -114,7 +158,7 @@ public class BroadcastStickyCache { } public static void incrementVersion(@NonNull String action) { - if (!Flags.useStickyBcastCache()) { + if (!shouldIncrementVersion(action)) { return; } final String key = getKey(action); @@ -136,7 +180,7 @@ public class BroadcastStickyCache { } public static void incrementVersionIfExists(@NonNull String action) { - if (!Flags.useStickyBcastCache()) { + if (!shouldIncrementVersion(action)) { return; } final String key = getKey(action); @@ -150,6 +194,16 @@ public class BroadcastStickyCache { } } + private static boolean shouldIncrementVersion(@NonNull String action) { + if (!Flags.useStickyBcastCache()) { + return false; + } + if (!ArrayUtils.contains(CACHED_BROADCAST_ACTIONS, action)) { + return false; + } + return true; + } + @VisibleForTesting public static void clearForTest() { synchronized (sCachedStickyBroadcasts) { diff --git a/services/core/java/com/android/server/am/BroadcastController.java b/services/core/java/com/android/server/am/BroadcastController.java index 934ce6953d14..15f1085b7125 100644 --- a/services/core/java/com/android/server/am/BroadcastController.java +++ b/services/core/java/com/android/server/am/BroadcastController.java @@ -686,7 +686,6 @@ class BroadcastController { boolean serialized, boolean sticky, int userId) { mService.enforceNotIsolatedCaller("broadcastIntent"); - boolean isCallerCore; int result; synchronized (mService) { intent = verifyBroadcastLocked(intent); @@ -709,7 +708,6 @@ class BroadcastController { final long origId = Binder.clearCallingIdentity(); try { - isCallerCore = UserHandle.isCore(callingUid); result = broadcastIntentLocked(callerApp, callerApp != null ? callerApp.info.packageName : null, callingFeatureId, intent, resolvedType, resultToApp, resultTo, resultCode, resultData, @@ -721,7 +719,7 @@ class BroadcastController { Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER); } } - if (sticky && isCallerCore && result == ActivityManager.BROADCAST_SUCCESS) { + if (sticky && result == ActivityManager.BROADCAST_SUCCESS) { BroadcastStickyCache.incrementVersion(intent.getAction()); } return result; diff --git a/tests/broadcasts/unit/src/android/app/BroadcastStickyCacheTest.java b/tests/broadcasts/unit/src/android/app/BroadcastStickyCacheTest.java index f8fd04bafbf3..b7c412dea999 100644 --- a/tests/broadcasts/unit/src/android/app/BroadcastStickyCacheTest.java +++ b/tests/broadcasts/unit/src/android/app/BroadcastStickyCacheTest.java @@ -15,6 +15,9 @@ */ package android.app; +import static android.content.Intent.ACTION_BATTERY_CHANGED; +import static android.content.Intent.ACTION_DEVICE_STORAGE_LOW; + import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer; import static com.google.common.truth.Truth.assertThat; @@ -60,9 +63,8 @@ public class BroadcastStickyCacheTest { .mockStatic(SystemProperties.class) .build(); - private static final String ACTION_TEST_RED = "action.test.red"; - private static final String ACTION_TEST_GREEN = "action.test.green"; - private static final String PROP_TEST_GREEN = BroadcastStickyCache.getKey(ACTION_TEST_GREEN); + private static final String PROP_KEY_BATTERY_CHANGED = BroadcastStickyCache.getKey( + ACTION_BATTERY_CHANGED); private final TestSystemProps mTestSystemProps = new TestSystemProps(); @@ -106,52 +108,52 @@ public class BroadcastStickyCacheTest { @Test public void testUseCache_multipleActions() { final IntentFilter filter = new IntentFilter(); - filter.addAction(ACTION_TEST_RED); - filter.addAction(ACTION_TEST_GREEN); + filter.addAction(ACTION_DEVICE_STORAGE_LOW); + filter.addAction(ACTION_BATTERY_CHANGED); assertThat(BroadcastStickyCache.useCache(filter)).isEqualTo(false); } @Test public void testUseCache_valueNotSet() { - final IntentFilter filter = new IntentFilter(ACTION_TEST_GREEN); + final IntentFilter filter = new IntentFilter(ACTION_BATTERY_CHANGED); assertThat(BroadcastStickyCache.useCache(filter)).isEqualTo(false); } @Test public void testUseCache() { - final IntentFilter filter = new IntentFilter(ACTION_TEST_GREEN); - final Intent intent = new Intent(ACTION_TEST_GREEN) + final IntentFilter filter = new IntentFilter(ACTION_BATTERY_CHANGED); + final Intent intent = new Intent(ACTION_BATTERY_CHANGED) .putExtra(BatteryManager.EXTRA_LEVEL, 90); - BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + BroadcastStickyCache.incrementVersion(ACTION_BATTERY_CHANGED); BroadcastStickyCache.add(filter, intent); assertThat(BroadcastStickyCache.useCache(filter)).isEqualTo(true); } @Test public void testUseCache_versionMismatch() { - final IntentFilter filter = new IntentFilter(ACTION_TEST_GREEN); - final Intent intent = new Intent(ACTION_TEST_GREEN) + final IntentFilter filter = new IntentFilter(ACTION_BATTERY_CHANGED); + final Intent intent = new Intent(ACTION_BATTERY_CHANGED) .putExtra(BatteryManager.EXTRA_LEVEL, 90); - BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + BroadcastStickyCache.incrementVersion(ACTION_BATTERY_CHANGED); BroadcastStickyCache.add(filter, intent); - BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + BroadcastStickyCache.incrementVersion(ACTION_BATTERY_CHANGED); assertThat(BroadcastStickyCache.useCache(filter)).isEqualTo(false); } @Test public void testAdd() { - final IntentFilter filter = new IntentFilter(ACTION_TEST_GREEN); - Intent intent = new Intent(ACTION_TEST_GREEN) + final IntentFilter filter = new IntentFilter(ACTION_BATTERY_CHANGED); + Intent intent = new Intent(ACTION_BATTERY_CHANGED) .putExtra(BatteryManager.EXTRA_LEVEL, 90); - BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + BroadcastStickyCache.incrementVersion(ACTION_BATTERY_CHANGED); BroadcastStickyCache.add(filter, intent); assertThat(BroadcastStickyCache.useCache(filter)).isEqualTo(true); Intent actualIntent = BroadcastStickyCache.getIntentUnchecked(filter); assertThat(actualIntent).isNotNull(); assertEquals(actualIntent, intent); - intent = new Intent(ACTION_TEST_GREEN) + intent = new Intent(ACTION_BATTERY_CHANGED) .putExtra(BatteryManager.EXTRA_LEVEL, 99); BroadcastStickyCache.add(filter, intent); actualIntent = BroadcastStickyCache.getIntentUnchecked(filter); @@ -161,45 +163,45 @@ public class BroadcastStickyCacheTest { @Test public void testIncrementVersion_propExists() { - SystemProperties.set(PROP_TEST_GREEN, String.valueOf(100)); + SystemProperties.set(PROP_KEY_BATTERY_CHANGED, String.valueOf(100)); - BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); - assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(101); - BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); - assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(102); + BroadcastStickyCache.incrementVersion(ACTION_BATTERY_CHANGED); + assertThat(mTestSystemProps.get(PROP_KEY_BATTERY_CHANGED, -1 /* def */)).isEqualTo(101); + BroadcastStickyCache.incrementVersion(ACTION_BATTERY_CHANGED); + assertThat(mTestSystemProps.get(PROP_KEY_BATTERY_CHANGED, -1 /* def */)).isEqualTo(102); } @Test public void testIncrementVersion_propNotExists() { // Verify that the property doesn't exist - assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(-1); + assertThat(mTestSystemProps.get(PROP_KEY_BATTERY_CHANGED, -1 /* def */)).isEqualTo(-1); - BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); - assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(1); - BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); - assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(2); + BroadcastStickyCache.incrementVersion(ACTION_BATTERY_CHANGED); + assertThat(mTestSystemProps.get(PROP_KEY_BATTERY_CHANGED, -1 /* def */)).isEqualTo(1); + BroadcastStickyCache.incrementVersion(ACTION_BATTERY_CHANGED); + assertThat(mTestSystemProps.get(PROP_KEY_BATTERY_CHANGED, -1 /* def */)).isEqualTo(2); } @Test public void testIncrementVersionIfExists_propExists() { - BroadcastStickyCache.incrementVersion(ACTION_TEST_GREEN); + BroadcastStickyCache.incrementVersion(ACTION_BATTERY_CHANGED); - BroadcastStickyCache.incrementVersionIfExists(ACTION_TEST_GREEN); - assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(2); - BroadcastStickyCache.incrementVersionIfExists(ACTION_TEST_GREEN); - assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(3); + BroadcastStickyCache.incrementVersionIfExists(ACTION_BATTERY_CHANGED); + assertThat(mTestSystemProps.get(PROP_KEY_BATTERY_CHANGED, -1 /* def */)).isEqualTo(2); + BroadcastStickyCache.incrementVersionIfExists(ACTION_BATTERY_CHANGED); + assertThat(mTestSystemProps.get(PROP_KEY_BATTERY_CHANGED, -1 /* def */)).isEqualTo(3); } @Test public void testIncrementVersionIfExists_propNotExists() { // Verify that the property doesn't exist - assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(-1); + assertThat(mTestSystemProps.get(PROP_KEY_BATTERY_CHANGED, -1 /* def */)).isEqualTo(-1); - BroadcastStickyCache.incrementVersionIfExists(ACTION_TEST_GREEN); - assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(-1); + BroadcastStickyCache.incrementVersionIfExists(ACTION_BATTERY_CHANGED); + assertThat(mTestSystemProps.get(PROP_KEY_BATTERY_CHANGED, -1 /* def */)).isEqualTo(-1); // Verify that property is not added as part of the querying. - BroadcastStickyCache.incrementVersionIfExists(ACTION_TEST_GREEN); - assertThat(mTestSystemProps.get(PROP_TEST_GREEN, -1 /* def */)).isEqualTo(-1); + BroadcastStickyCache.incrementVersionIfExists(ACTION_BATTERY_CHANGED); + assertThat(mTestSystemProps.get(PROP_KEY_BATTERY_CHANGED, -1 /* def */)).isEqualTo(-1); } private void assertEquals(Intent actualIntent, Intent expectedIntent) { -- cgit v1.2.3-59-g8ed1b