Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 93894bf3 authored by Automerger Merge Worker's avatar Automerger Merge Worker Committed by Android (Google) Code Review
Browse files

Merge "Merge "Fix an issue where we might load bubbles with an invalid user...

Merge "Merge "Fix an issue where we might load bubbles with an invalid user id" into udc-qpr-dev am: d915c1ad am: 3c86b7f3"
parents 669de24d 8eac5a89
Loading
Loading
Loading
Loading
+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
+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.
+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,
+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
+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());
    }

    /**