Merge "Fix an issue where we might load bubbles with an invalid user id" into... (8eac5a89) · Commits · e / os / android_frameworks_base

libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleController.java

+3 −1

Original line number	Diff line number	Diff line
		@@ -1282,7 +1282,9 @@ public class BubbleController implements ConfigurationChangeListener,
		return;
		}
		mOverflowDataLoadNeeded = false;
		mDataRepository.loadBubbles(mCurrentUserId, (bubbles) -> {
		List<UserInfo> users = mUserManager.getAliveUsers();
		List<Integer> userIds = users.stream().map(userInfo -> userInfo.id).toList();
		mDataRepository.loadBubbles(mCurrentUserId, userIds, (bubbles) -> {
		bubbles.forEach(bubble -> {
		if (mBubbleData.hasAnyBubbleWithKey(bubble.getKey())) {
		// if the bubble is already active, there's no need to push it to overflow

libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleDataRepository.kt

+58 −9

Original line number	Diff line number	Diff line
		@@ -17,7 +17,6 @@ package com.android.wm.shell.bubbles

		import android.annotation.SuppressLint
		import android.annotation.UserIdInt
		import android.content.Context
		import android.content.pm.LauncherApps
		import android.content.pm.LauncherApps.ShortcutQuery.FLAG_MATCH_CACHED
		import android.content.pm.LauncherApps.ShortcutQuery.FLAG_MATCH_DYNAMIC
		@@ -25,6 +24,8 @@ import android.content.pm.LauncherApps.ShortcutQuery.FLAG_MATCH_PINNED_BY_ANY_LA
		import android.content.pm.UserInfo
		import android.os.UserHandle
		import android.util.Log
		import android.util.SparseArray
		import com.android.internal.annotations.VisibleForTesting
		import com.android.wm.shell.bubbles.Bubbles.BubbleMetadataFlagListener
		import com.android.wm.shell.bubbles.storage.BubbleEntity
		import com.android.wm.shell.bubbles.storage.BubblePersistentRepository
		@@ -38,13 +39,12 @@ import kotlinx.coroutines.cancelAndJoin
		import kotlinx.coroutines.launch
		import kotlinx.coroutines.yield

		internal class BubbleDataRepository(
		context: Context,
		class BubbleDataRepository(
		private val launcherApps: LauncherApps,
		private val mainExecutor: ShellExecutor
		private val mainExecutor: ShellExecutor,
		private val persistentRepository: BubblePersistentRepository,
		) {
		private val volatileRepository = BubbleVolatileRepository(launcherApps)
		private val persistentRepository = BubblePersistentRepository(context)

		private val coroutineScope = CoroutineScope(Dispatchers.IO + SupervisorJob())
		private var job: Job? = null
		@@ -99,6 +99,43 @@ internal class BubbleDataRepository(
		if (volatileRepository.sanitizeBubbles(userIds)) persistToDisk()
		}

		/**
		* Removes all entities that don't have a user in the activeUsers list, if any entities were
		* removed it persists the new list to disk.
		*/
		private fun filterForActiveUsersAndPersist(
		activeUsers: List<Int>,
		entitiesByUser: SparseArray<List<BubbleEntity>>
		): SparseArray<List<BubbleEntity>> {
		val validEntitiesByUser = SparseArray<List<BubbleEntity>>()
		var entitiesChanged = false
		for (i in 0 until entitiesByUser.size()) {
		val parentUserId = entitiesByUser.keyAt(i)
		if (activeUsers.contains(parentUserId)) {
		val validEntities = mutableListOf<BubbleEntity>()
		// Check if each of the bubbles in the top-level user still has a valid user
		// as it could belong to a profile and have a different id from the parent.
		for (entity in entitiesByUser.get(parentUserId)) {
		if (activeUsers.contains(entity.userId)) {
		validEntities.add(entity)
		} else {
		entitiesChanged = true
		}
		}
		if (validEntities.isNotEmpty()) {
		validEntitiesByUser.put(parentUserId, validEntities)
		}
		} else {
		entitiesChanged = true
		}
		}
		if (entitiesChanged) {
		persistToDisk(validEntitiesByUser)
		return validEntitiesByUser
		}
		return entitiesByUser
		}

		private fun transform(bubbles: List<Bubble>): List<BubbleEntity> {
		return bubbles.mapNotNull { b ->
		BubbleEntity(
		@@ -130,7 +167,9 @@ internal class BubbleDataRepository(
		* Job C resumes and reaches yield() and is then cancelled
		* Job D resumes and performs another blocking I/O
		*/
		private fun persistToDisk() {
		private fun persistToDisk(
		entitiesByUser: SparseArray<List<BubbleEntity>> = volatileRepository.bubbles
		) {
		val prev = job
		job = coroutineScope.launch {
		// if there was an ongoing disk I/O operation, they can be cancelled
		@@ -138,7 +177,7 @@ internal class BubbleDataRepository(
		// check for cancellation before disk I/O
		yield()
		// save to disk
		persistentRepository.persistsToDisk(volatileRepository.bubbles)
		persistentRepository.persistsToDisk(entitiesByUser)
		}
		}

		@@ -149,7 +188,12 @@ internal class BubbleDataRepository(
		* bubbles.
		*/
		@SuppressLint("WrongConstant")
		fun loadBubbles(userId: Int, cb: (List<Bubble>) -> Unit) = coroutineScope.launch {
		@VisibleForTesting
		fun loadBubbles(
		userId: Int,
		currentUsers: List<Int>,
		cb: (List<Bubble>) -> Unit
		) = coroutineScope.launch {
		/**
		* Load BubbleEntity from disk.
		* e.g.
		@@ -160,7 +204,12 @@ internal class BubbleDataRepository(
		* ]
		*/
		val entitiesByUser = persistentRepository.readFromDisk()
		val entities = entitiesByUser.get(userId) ?: return@launch

		// Before doing anything, validate that the entities we loaded are valid & have an existing
		// user.
		val validEntitiesByUser = filterForActiveUsersAndPersist(currentUsers, entitiesByUser)

		val entities = validEntitiesByUser.get(userId) ?: return@launch
		volatileRepository.addBubbles(userId, entities)
		/**
		* Extract userId/packageName from these entities.

libs/WindowManager/Shell/src/com/android/wm/shell/dagger/WMShellModule.java

+3 −1

Original line number	Diff line number	Diff line
		@@ -37,6 +37,7 @@ import com.android.wm.shell.bubbles.BubbleDataRepository;
		import com.android.wm.shell.bubbles.BubbleLogger;
		import com.android.wm.shell.bubbles.BubblePositioner;
		import com.android.wm.shell.bubbles.properties.ProdBubbleProperties;
		import com.android.wm.shell.bubbles.storage.BubblePersistentRepository;
		import com.android.wm.shell.common.DisplayController;
		import com.android.wm.shell.common.DisplayImeController;
		import com.android.wm.shell.common.DisplayInsetsController;
		@@ -184,7 +185,8 @@ public abstract class WMShellModule {
		IWindowManager wmService) {
		return new BubbleController(context, shellInit, shellCommandHandler, shellController, data,
		null /* synchronizer */, floatingContentCoordinator,
		new BubbleDataRepository(context, launcherApps, mainExecutor),
		new BubbleDataRepository(launcherApps, mainExecutor,
		new BubblePersistentRepository(context)),
		statusBarService, windowManager, windowManagerShellWrapper, userManager,
		launcherApps, logger, taskStackListener, organizer, positioner, displayController,
		oneHandedOptional, dragAndDropController, mainExecutor, mainHandler, bgExecutor,

libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/bubbles/BubbleDataRepositoryTest.kt

0 → 100644

+190 −0

Original line number	Diff line number	Diff line
		/*
		* Copyright (C) 2023 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 com.android.wm.shell.bubbles

		import android.app.ActivityTaskManager
		import android.content.pm.LauncherApps
		import android.content.pm.ShortcutInfo
		import android.util.SparseArray
		import com.android.wm.shell.ShellTestCase
		import com.android.wm.shell.bubbles.storage.BubbleEntity
		import com.android.wm.shell.bubbles.storage.BubblePersistentRepository
		import com.android.wm.shell.common.ShellExecutor
		import com.google.common.truth.Truth.assertThat
		import org.junit.After
		import org.junit.Before
		import org.junit.Test
		import org.mockito.Mockito.mock
		import org.mockito.Mockito.spy

		class BubbleDataRepositoryTest : ShellTestCase() {

		private val user0BubbleEntities = listOf(
		BubbleEntity(
		userId = 0,
		packageName = "com.example.messenger",
		shortcutId = "shortcut-1",
		key = "0k1",
		desiredHeight = 120,
		desiredHeightResId = 0,
		title = null,
		taskId = 1,
		locus = null,
		isDismissable = true
		),
		BubbleEntity(
		userId = 10,
		packageName = "com.example.chat",
		shortcutId = "alice and bob",
		key = "0k2",
		desiredHeight = 0,
		desiredHeightResId = 16537428,
		title = "title",
		taskId = 2,
		locus = null
		),
		BubbleEntity(
		userId = 0,
		packageName = "com.example.messenger",
		shortcutId = "shortcut-2",
		key = "0k3",
		desiredHeight = 120,
		desiredHeightResId = 0,
		title = null,
		taskId = ActivityTaskManager.INVALID_TASK_ID,
		locus = null
		)
		)

		private val user1BubbleEntities = listOf(
		BubbleEntity(
		userId = 1,
		packageName = "com.example.messenger",
		shortcutId = "shortcut-1",
		key = "1k1",
		desiredHeight = 120,
		desiredHeightResId = 0,
		title = null,
		taskId = 3,
		locus = null,
		isDismissable = true
		),
		BubbleEntity(
		userId = 12,
		packageName = "com.example.chat",
		shortcutId = "alice and bob",
		key = "1k2",
		desiredHeight = 0,
		desiredHeightResId = 16537428,
		title = "title",
		taskId = 4,
		locus = null
		),
		BubbleEntity(
		userId = 1,
		packageName = "com.example.messenger",
		shortcutId = "shortcut-2",
		key = "1k3",
		desiredHeight = 120,
		desiredHeightResId = 0,
		title = null,
		taskId = ActivityTaskManager.INVALID_TASK_ID,
		locus = null
		),
		BubbleEntity(
		userId = 12,
		packageName = "com.example.chat",
		shortcutId = "alice",
		key = "1k4",
		desiredHeight = 0,
		desiredHeightResId = 16537428,
		title = "title",
		taskId = 5,
		locus = null
		)
		)

		private val mainExecutor = mock(ShellExecutor::class.java)
		private val launcherApps = mock(LauncherApps::class.java)

		private val persistedBubbles = SparseArray<List<BubbleEntity>>()

		private lateinit var dataRepository: BubbleDataRepository
		private lateinit var persistentRepository: BubblePersistentRepository

		@Before
		fun setup() {
		persistentRepository = spy(BubblePersistentRepository(mContext))
		dataRepository = BubbleDataRepository(launcherApps, mainExecutor, persistentRepository)

		// Add the bubbles to the persistent repository
		persistedBubbles.put(0, user0BubbleEntities)
		persistedBubbles.put(1, user1BubbleEntities)
		persistentRepository.persistsToDisk(persistedBubbles)
		}

		@After
		fun teardown() {
		// Clean up any persisted bubbles for the next run
		persistentRepository.persistsToDisk(SparseArray())
		}

		@Test
		fun testLoadBubbles_invalidParent() {
		val activeUserIds = listOf(10, 1, 12) // Missing user 0 in persistedBubbles
		dataRepository.loadBubbles(1, activeUserIds) {
		// Verify that user 0 has been removed from the persisted list
		val entitiesByUser = persistentRepository.readFromDisk()
		assertThat(entitiesByUser.get(0)).isNull()
		}
		}

		@Test
		fun testLoadBubbles_invalidChild() {
		val activeUserIds = listOf(0, 10, 1) // Missing user 1's child user 12
		dataRepository.loadBubbles(1, activeUserIds) {
		// Build a list to compare against
		val user1BubblesWithoutUser12 = mutableListOf<Bubble>()
		val user1EntitiesWithoutUser12 = mutableListOf<BubbleEntity>()
		for (entity in user1BubbleEntities) {
		if (entity.userId != 12) {
		user1BubblesWithoutUser12.add(entity.toBubble())
		user1EntitiesWithoutUser12.add(entity)
		}
		}

		// Verify that user 12 has been removed from the persisted list
		val entitiesByUser = persistentRepository.readFromDisk()
		assertThat(entitiesByUser.get(1)).isEqualTo(user1EntitiesWithoutUser12)
		}
		}

		private fun BubbleEntity.toBubble(): Bubble {
		return Bubble(
		key,
		mock(ShortcutInfo::class.java),
		desiredHeight,
		desiredHeightResId,
		title,
		taskId,
		locus,
		isDismissable,
		mainExecutor,
		mock(Bubbles.BubbleMetadataFlagListener::class.java)
		)
		}
		}
		No newline at end of file

packages/SystemUI/tests/src/com/android/systemui/wmshell/BubblesTest.java

+3 −2

Original line number	Diff line number	Diff line
		@@ -35,6 +35,7 @@ import static org.junit.Assert.assertTrue;
		import static org.mockito.ArgumentMatchers.any;
		import static org.mockito.ArgumentMatchers.anyBoolean;
		import static org.mockito.ArgumentMatchers.anyInt;
		import static org.mockito.ArgumentMatchers.anyList;
		import static org.mockito.ArgumentMatchers.anyString;
		import static org.mockito.ArgumentMatchers.eq;
		import static org.mockito.Mockito.atLeastOnce;
		@@ -1131,7 +1132,7 @@ public class BubblesTest extends SysuiTestCase {
		assertThat(mBubbleData.getOverflowBubbleWithKey(mBubbleEntry2User11.getKey())).isNotNull();

		// Would have loaded bubbles twice because of user switch
		verify(mDataRepository, times(2)).loadBubbles(anyInt(), any());
		verify(mDataRepository, times(2)).loadBubbles(anyInt(), anyList(), any());
		}

		@Test
		@@ -1187,7 +1188,7 @@ public class BubblesTest extends SysuiTestCase {
		mEntryListener.onEntryRemoved(mRow2, REASON_APP_CANCEL);
		assertThat(mBubbleData.getOverflowBubbles()).isEmpty();

		verify(mDataRepository, times(1)).loadBubbles(anyInt(), any());
		verify(mDataRepository, times(1)).loadBubbles(anyInt(), anyList(), any());
		}

		/**