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

Commit 185782dd authored by Mady Mellor's avatar Mady Mellor
Browse files

Fix potential NPE & clean up some logic 

- I had accidentally swapped the values in removeBubblesForUser
  in one place which could lead to a crash, it's actually simpler
  to just do the removal there since it's the parentUser that was
  removed so we can remove all of those entries.
- In other places I've added a null check before performing the
  removeIf to be safe
- Added a test checking bad input

Bug: 232847471
Test: atest BubbleVolatileRepositoryTest
Change-Id: I10f5b24e591db62edbb63ad40e588ba19ab621c1
parent 8c88d030

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

+8 −3
Original line number Original line Diff line number Diff line
@@ -126,7 +126,11 @@ class BubbleVolatileRepository(private val launcherApps: LauncherApps) { 
        @UserIdInt userId: Int,

        @UserIdInt userId: Int,

        @UserIdInt parentUserId: Int

        @UserIdInt parentUserId: Int

    ): Boolean {

    ): Boolean {

        return entitiesByUser.get(parentUserId).removeIf { b: BubbleEntity -> b.userId == userId }

        if (entitiesByUser.get(parentUserId) != null) {

            return entitiesByUser.get(parentUserId).removeIf {

                b: BubbleEntity -> b.userId == userId }

        }

        return false

    }

    }





    /**

    /**
@@ -141,8 +145,9 @@ class BubbleVolatileRepository(private val launcherApps: LauncherApps) { 
            // First check if the user is a parent / top-level user

            // First check if the user is a parent / top-level user

            val parentUserId = entitiesByUser.keyAt(i)

            val parentUserId = entitiesByUser.keyAt(i)

            if (!activeUsers.contains(parentUserId)) {

            if (!activeUsers.contains(parentUserId)) {

                return removeBubblesForUser(parentUserId, -1)

                entitiesByUser.remove(parentUserId)

            } else {

                return true

            } else if (entitiesByUser.get(parentUserId) != null) {

                // Then check if each of the bubbles in the top-level user, still has a valid user

                // Then 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.

                // as it could belong to a profile and have a different id from the parent.

                return entitiesByUser.get(parentUserId).removeIf { b: BubbleEntity ->

                return entitiesByUser.get(parentUserId).removeIf { b: BubbleEntity ->

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

+6 −0
Original line number Original line Diff line number Diff line
@@ -269,6 +269,12 @@ class BubbleVolatileRepositoryTest : ShellTestCase() { 
        assertThat(repository.getEntities(user11.identifier).toList())

        assertThat(repository.getEntities(user11.identifier).toList())

                .isEqualTo(listOf(bubble11, bubble12))

                .isEqualTo(listOf(bubble11, bubble12))

    }

    }



    @Test

    fun testRemoveBubbleForUser_invalidInputDoesntCrash() {

        repository.removeBubblesForUser(-1, 0)

        repository.removeBubblesForUser(-1, -1)

    }

}

}





private const val PKG_MESSENGER = "com.example.messenger"

private const val PKG_MESSENGER = "com.example.messenger"