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

Commit fb4b51c5 authored by Coco Duan's avatar Coco Duan
Browse files

Fix NPE when removing widgets 

The issue happened when removing a user-selected widget, as
`deleteWidget()` handles removing from both DB and AppWidgetHost.
Then `onAppWidgetRemoved()` callback repetitively calls
`deleteWidget()` trying to remove from DB and widget host again.

- Separate the call to delete a widget from widget host and DB.
  Let CommunalAppWidgetHost manages deleting from DB for both
  uninstalled or user-selected widget, following the deletion
  from widget host.
- In CommunalWidgetDAO#deleteWidgetById, added a null check on
  the entry that is queried. Returns false if no entry exists
  to be deleted.

Bug: b/323057545
Test: atest CommunalWidgetRepositoryImplTest
Test: atest CommunalWidgetDaoTest
Test: atest CommunalEditModeViewModelTest
Flag: ACONFIG com.android.systemui.communal_hub DEVELOPMENT
Change-Id: I6a9b8f8605f22b08a447acf8d7747519fc6e3f81
parent 5ed53c5f

packages/SystemUI/multivalentTests/src/com/android/systemui/communal/data/repository/CommunalWidgetRepositoryImplTest.kt

+11 −2
Original line number Diff line number Diff line
@@ -198,13 +198,22 @@ class CommunalWidgetRepositoryImplTest : SysuiTestCase() { 
        }



    @Test

    fun deleteWidget_removeWidgetId_andDeleteFromDb() =

    fun deleteWidgetFromDb() =

        testScope.runTest {

            val id = 1

            underTest.deleteWidget(id)

            underTest.deleteWidgetFromDb(id)

            runCurrent()



            verify(communalWidgetDao).deleteWidgetById(id)

        }



    @Test

    fun deleteWidgetFromHost() =

        testScope.runTest {

            val id = 1

            underTest.deleteWidgetFromHost(id)

            runCurrent()



            verify(appWidgetHost).deleteAppWidgetId(id)

        }

packages/SystemUI/multivalentTests/src/com/android/systemui/communal/view/viewmodel/CommunalEditModeViewModelTest.kt

+43 −0
Original line number Diff line number Diff line
@@ -141,6 +141,49 @@ class CommunalEditModeViewModelTest : SysuiTestCase() { 
            assertThat(selectedKey).isNull()

        }



    @Test

    fun deleteWidget() =

        testScope.runTest {

            tutorialRepository.setTutorialSettingState(Settings.Secure.HUB_MODE_TUTORIAL_COMPLETED)



            // Widgets available.

            val widgets =

                listOf(

                    CommunalWidgetContentModel(

                        appWidgetId = 0,

                        priority = 30,

                        providerInfo = mock(),

                    ),

                    CommunalWidgetContentModel(

                        appWidgetId = 1,

                        priority = 20,

                        providerInfo = mock(),

                    ),

                )

            widgetRepository.setCommunalWidgets(widgets)



            val communalContent by collectLastValue(underTest.communalContent)



            // Widgets and CTA tile are shown.

            assertThat(communalContent?.size).isEqualTo(3)

            assertThat(communalContent?.get(0))

                .isInstanceOf(CommunalContentModel.Widget::class.java)

            assertThat(communalContent?.get(1))

                .isInstanceOf(CommunalContentModel.Widget::class.java)

            assertThat(communalContent?.get(2))

                .isInstanceOf(CommunalContentModel.CtaTileInEditMode::class.java)



            underTest.onDeleteWidget(widgets.get(0).appWidgetId)



            // Only one widget and CTA tile remain.

            assertThat(communalContent?.size).isEqualTo(2)

            val item = communalContent?.get(0)

            val appWidgetId = if (item is CommunalContentModel.Widget) item.appWidgetId else null

            assertThat(appWidgetId).isEqualTo(widgets.get(1).appWidgetId)

            assertThat(communalContent?.get(1))

                .isInstanceOf(CommunalContentModel.CtaTileInEditMode::class.java)

        }



    @Test

    fun reorderWidget_uiEventLogging_start() {

        underTest.onReorderWidgetStart()

packages/SystemUI/src/com/android/systemui/communal/data/db/CommunalWidgetDao.kt

+10 −4
Original line number Diff line number Diff line
@@ -97,7 +97,7 @@ interface CommunalWidgetDao { 
    fun getWidgets(): Flow<Map<CommunalItemRank, CommunalWidgetItem>>



    @Query("SELECT * FROM communal_widget_table WHERE widget_id = :id")

    fun getWidgetByIdNow(id: Int): CommunalWidgetItem

    fun getWidgetByIdNow(id: Int): CommunalWidgetItem?



    @Delete fun deleteWidgets(vararg widgets: CommunalWidgetItem)
@@ -120,9 +120,11 @@ interface CommunalWidgetDao { 
    fun updateWidgetOrder(widgetIdToPriorityMap: Map<Int, Int>) {

        widgetIdToPriorityMap.forEach { (id, priority) ->

            val widget = getWidgetByIdNow(id)

            if (widget != null) {

                updateItemRank(widget.itemId, priority)

            }

        }

    }



    @Transaction

    fun addWidget(widgetId: Int, provider: ComponentName, priority: Int): Long {
@@ -134,9 +136,13 @@ interface CommunalWidgetDao { 
    }



    @Transaction

    fun deleteWidgetById(widgetId: Int) {

        val widget = getWidgetByIdNow(widgetId)

    fun deleteWidgetById(widgetId: Int): Boolean {

        val widget =

            getWidgetByIdNow(widgetId) ?: // no entry to delete from db

            return false



        deleteItemRankById(widget.itemId)

        deleteWidgets(widget)

        return true

    }

}

packages/SystemUI/src/com/android/systemui/communal/data/repository/CommunalWidgetRepository.kt

+16 −4
Original line number Diff line number Diff line
@@ -54,8 +54,11 @@ interface CommunalWidgetRepository { 
        configurator: WidgetConfigurator? = null

    ) {}



    /** Delete a widget by id from app widget service and the database. */

    fun deleteWidget(widgetId: Int) {}

    /** Delete a widget by id from the database. */

    fun deleteWidgetFromDb(widgetId: Int) {}



    /** Delete a widget by id from app widget host. */

    fun deleteWidgetFromHost(widgetId: Int) {}



    /**

     * Update the order of widgets in the database.
@@ -143,9 +146,18 @@ constructor( 
        }

    }



    override fun deleteWidget(widgetId: Int) {

    override fun deleteWidgetFromDb(widgetId: Int) {

        bgScope.launch {

            if (communalWidgetDao.deleteWidgetById(widgetId)) {

                logger.i("Deleted widget with id $widgetId from DB .")

            } else {

                logger.w("Widget with id $widgetId cannot be deleted from DB.")

            }

        }

    }



    override fun deleteWidgetFromHost(widgetId: Int) {

        bgScope.launch {

            communalWidgetDao.deleteWidgetById(widgetId)

            appWidgetHost.deleteAppWidgetId(widgetId)

            logger.i("Deleted widget with id $widgetId.")

        }

packages/SystemUI/src/com/android/systemui/communal/domain/interactor/CommunalInteractor.kt

+8 −2
Original line number Diff line number Diff line
@@ -170,9 +170,15 @@ constructor( 
        configurator: WidgetConfigurator?,

    ) = widgetRepository.addWidget(componentName, priority, configurator)



    /** Delete a widget by id. */

    fun deleteWidget(id: Int) = widgetRepository.deleteWidget(id)

    /**

     * Delete a widget by id from the database. [CommunalAppWidgetHostStartable] invokes this

     * function to manage the deletion from the database for uninstalled or user-deleted widgets,

     * following the removal of a widget from the host.

     */

    fun deleteWidgetFromDb(id: Int) = widgetRepository.deleteWidgetFromDb(id)



    /** Delete a widget by id from AppWidgetHost. Called when user deletes a widget from the hub */

    fun deleteWidgetFromHost(id: Int) = widgetRepository.deleteWidgetFromHost(id)

    /**

     * Reorder the widgets.

     *