From 3d04c1f3fc02272a17f99a91efb6b2d6f6a69802 Mon Sep 17 00:00:00 2001 From: Sebastian Franco Date: Mon, 25 Mar 2024 10:43:57 -0700 Subject: [PATCH] Fix widget dissapearing because of change of appWidgetId When migrating the widgets might dissapear because the appWidgetId can change and when migrating it's no longer valid. Bug: 324931424 Bug: 330583877 Test: ValidGridMigrationUnitTest Test: GridMigrationTest Flag: NA Change-Id: Ie41a01470814b252f6bb7e4409d3382951a1d70d --- .../model/GridSizeMigrationUtil.java | 23 ++- .../ValidGridMigrationTestCaseGenerator.kt | 41 ++++-- .../ValidGridMigrationUnitTest.kt | 136 +++++++++++------- 3 files changed, 133 insertions(+), 67 deletions(-) diff --git a/src/com/android/launcher3/model/GridSizeMigrationUtil.java b/src/com/android/launcher3/model/GridSizeMigrationUtil.java index 299c952d2a..f24a7c12e9 100644 --- a/src/com/android/launcher3/model/GridSizeMigrationUtil.java +++ b/src/com/android/launcher3/model/GridSizeMigrationUtil.java @@ -49,6 +49,7 @@ import com.android.launcher3.config.FeatureFlags; import com.android.launcher3.model.data.ItemInfo; import com.android.launcher3.pm.InstallSessionHelper; import com.android.launcher3.provider.LauncherDbUtils.SQLiteTransaction; +import com.android.launcher3.util.ContentWriter; import com.android.launcher3.util.GridOccupancy; import com.android.launcher3.util.IntArray; import com.android.launcher3.widget.LauncherAppWidgetProviderInfo; @@ -674,6 +675,11 @@ public class GridSizeMigrationUtil { private String mProvider; private Map> mFolderItems = new HashMap<>(); + /** + * Id of the specific widget. + */ + public int appWidgetId = NO_ID; + /** Comparator according to the reading order */ @Override public int compareTo(DbEntry another) { @@ -707,6 +713,18 @@ public class GridSizeMigrationUtil { values.put(LauncherSettings.Favorites.SPANY, spanY); } + @Override + public void writeToValues(@NonNull ContentWriter writer) { + super.writeToValues(writer); + writer.put(LauncherSettings.Favorites.APPWIDGET_ID, appWidgetId); + } + + @Override + public void readFromValues(@NonNull ContentValues values) { + super.readFromValues(values); + appWidgetId = values.getAsInteger(LauncherSettings.Favorites.APPWIDGET_ID); + } + /** This id is not used in the DB is only used while doing the migration and it identifies * an entry on each workspace. For example two calculator icons would have the same * migration id even thought they have different database ids. @@ -717,7 +735,10 @@ public class GridSizeMigrationUtil { case LauncherSettings.Favorites.ITEM_TYPE_APP_PAIR: return getFolderMigrationId(); case LauncherSettings.Favorites.ITEM_TYPE_APPWIDGET: - return mProvider; + // mProvider is the app the widget belongs to and appWidgetId it's the unique + // is of the widget, we need both because if you remove a widget and then add it + // again, then it can change and the WidgetProvider would not know the widget. + return mProvider + appWidgetId; case LauncherSettings.Favorites.ITEM_TYPE_APPLICATION: final String intentStr = cleanIntentString(mIntent); try { diff --git a/tests/src/com/android/launcher3/celllayout/testgenerator/ValidGridMigrationTestCaseGenerator.kt b/tests/src/com/android/launcher3/celllayout/testgenerator/ValidGridMigrationTestCaseGenerator.kt index e773a86a7a..a006fd7463 100644 --- a/tests/src/com/android/launcher3/celllayout/testgenerator/ValidGridMigrationTestCaseGenerator.kt +++ b/tests/src/com/android/launcher3/celllayout/testgenerator/ValidGridMigrationTestCaseGenerator.kt @@ -31,16 +31,17 @@ import java.util.concurrent.atomic.AtomicInteger * account for cases where the user have the same item multiple times. */ fun generateItemsForTest( - testCase: GridMigrationUnitTestCase, - repeatAfter: Int + boards: List, + repeatAfterRange: Point ): List { val id = AtomicInteger(0) val widgetId = AtomicInteger(LauncherAppWidgetInfo.CUSTOM_WIDGET_ID - 1) - val boards = testCase.boards // Repeat the same appWidgetProvider and intent to have repeating widgets and icons and test // that case too - val getIntent = { i: Int -> "Intent ${i % repeatAfter}" } - val getProvider = { i: Int -> "com.test/test.Provider${i % repeatAfter}" } + val getIntent = { i: Int -> "Intent ${(i + repeatAfterRange.x) % repeatAfterRange.y}" } + val getProvider = { i: Int -> + "com.test/test.Provider${(i + repeatAfterRange.x) % repeatAfterRange.y }" + } val hotseatEntries = (0 until boards[0].width).map { WorkspaceItem( @@ -97,11 +98,12 @@ fun generateItemsForTest( container = LauncherSettings.Favorites.CONTAINER_DESKTOP ) } - return widgetEntries + hotseatEntries // + iconEntries + return widgetEntries + hotseatEntries + iconEntries } data class GridMigrationUnitTestCase( val boards: List, + val destBoards: List, val srcSize: Point, val targetSize: Point, val seed: Long @@ -135,11 +137,27 @@ class ValidGridMigrationTestCaseGenerator(private val generator: Random) : return boards } - fun generateTestCase(): GridMigrationUnitTestCase { - var seed = generator.nextLong() + fun generateTestCase(isDestEmpty: Boolean): GridMigrationUnitTestCase { + val seed = generator.nextLong() val randomBoardGenerator = RandomBoardGenerator(Random(seed)) val width = randomBoardGenerator.getRandom(3, MAX_BOARD_SIZE) val height = randomBoardGenerator.getRandom(3, MAX_BOARD_SIZE) + val targetSize = + Point( + randomBoardGenerator.getRandom(3, MAX_BOARD_SIZE), + randomBoardGenerator.getRandom(3, MAX_BOARD_SIZE) + ) + val destBoards = + if (isDestEmpty) { + listOf() + } else { + generateBoards( + boardGenerator = randomBoardGenerator, + width = targetSize.x, + height = targetSize.y, + boardCount = randomBoardGenerator.getRandom(3, MAX_BOARD_COUNT) + ) + } return GridMigrationUnitTestCase( boards = generateBoards( @@ -148,12 +166,9 @@ class ValidGridMigrationTestCaseGenerator(private val generator: Random) : height = height, boardCount = randomBoardGenerator.getRandom(3, MAX_BOARD_COUNT) ), + destBoards = destBoards, srcSize = Point(width, height), - targetSize = - Point( - randomBoardGenerator.getRandom(3, MAX_BOARD_SIZE), - randomBoardGenerator.getRandom(3, MAX_BOARD_SIZE) - ), + targetSize = targetSize, seed = seed ) } diff --git a/tests/src/com/android/launcher3/model/gridmigration/ValidGridMigrationUnitTest.kt b/tests/src/com/android/launcher3/model/gridmigration/ValidGridMigrationUnitTest.kt index 100297608b..58b915f0ca 100644 --- a/tests/src/com/android/launcher3/model/gridmigration/ValidGridMigrationUnitTest.kt +++ b/tests/src/com/android/launcher3/model/gridmigration/ValidGridMigrationUnitTest.kt @@ -40,14 +40,22 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +private data class Grid(val tableName: String, val size: Point, val items: List) { + fun toGridState(): DeviceGridState = + DeviceGridState(size.x, size.y, size.x, InvariantDeviceProfile.TYPE_PHONE, tableName) +} + @SmallTest @RunWith(AndroidJUnit4::class) class ValidGridMigrationUnitTest { companion object { const val SEED = 1044542 - const val REPEAT_AFTER = 10 + val REPEAT_AFTER = Point(0, 10) + val REPEAT_AFTER_DST = Point(6, 15) const val TAG = "ValidGridMigrationUnitTest" + const val SMALL_TEST_SIZE = 60 + const val LARGE_TEST_SIZE = 1000 } private lateinit var context: Context @@ -57,14 +65,10 @@ class ValidGridMigrationUnitTest { context = InstrumentationRegistry.getInstrumentation().targetContext } - private fun validate( - srcItems: List, - dstItems: List, - destinationSize: Point - ) { + private fun validate(srcGrid: Grid, dstGrid: Grid, resultItems: List) { // This returns a map with the number of repeated elements // ex { calculatorIcon : 6, weatherWidget : 2 } - val itemsToSet = { it: List -> + val itemsToMap = { it: List -> it.filter { it.container != Favorites.CONTAINER_HOTSEAT } .groupingBy { when (it.type) { @@ -77,34 +81,36 @@ class ValidGridMigrationUnitTest { } .eachCount() } - for (it in dstItems) { - assert((it.x in 0..destinationSize.x) && (it.y in 0..destinationSize.y)) { - "Item outside of the board size. Size = $destinationSize Item = $it" + resultItems.forEach { + assert((it.x in 0..dstGrid.size.x) && (it.y in 0..dstGrid.size.y)) { + "Item outside of the board size. Size = ${dstGrid.size} Item = $it" } assert( - (it.x + it.spanX in 0..destinationSize.x) && - (it.y + it.spanY in 0..destinationSize.y) + (it.x + it.spanX in 0..dstGrid.size.x) && (it.y + it.spanY in 0..dstGrid.size.y) ) { - "Item doesn't fit in the grid. Size = $destinationSize Item = $it" + "Item doesn't fit in the grid. Size = ${dstGrid.size} Item = $it" } } - assert(itemsToSet(srcItems) == itemsToSet(dstItems)) { - "The srcItems do not match the dstItems src = $srcItems dst = $dstItems" + val srcCountMap = itemsToMap(srcGrid.items) + val resultCountMap = itemsToMap(resultItems) + val diff = resultCountMap - srcCountMap + + diff.forEach { (k, count) -> + assert(count >= 0) { "Source item $k not present on the result" } } } - private fun addItemsToDb(db: SQLiteDatabase, tableName: String, items: List) { + private fun addItemsToDb(db: SQLiteDatabase, grid: Grid) { LauncherDbUtils.SQLiteTransaction(db).use { transaction -> - items.forEach { insertIntoDb(tableName, it, transaction.db) } + grid.items.forEach { insertIntoDb(grid.tableName, it, transaction.db) } transaction.commit() } } private fun migrate( - srcItems: List, - srcSize: Point, - targetSize: Point + srcGrid: Grid, + dstGrid: Grid, ): List { val userSerial = UserCache.INSTANCE[context].getSerialNumberForUser(Process.myUserHandle()) val dbHelper = @@ -114,46 +120,64 @@ class ValidGridMigrationUnitTest { { UserCache.INSTANCE.get(context).getSerialNumberForUser(it) }, {} ) - val srcTableName = Favorites.TMP_TABLE - val dstTableName = Favorites.TABLE_NAME - Favorites.addTableToDb(dbHelper.writableDatabase, userSerial, false, srcTableName) - addItemsToDb(dbHelper.writableDatabase, srcTableName, srcItems) + + Favorites.addTableToDb(dbHelper.writableDatabase, userSerial, false, srcGrid.tableName) + + addItemsToDb(dbHelper.writableDatabase, srcGrid) + addItemsToDb(dbHelper.writableDatabase, dstGrid) + LauncherDbUtils.SQLiteTransaction(dbHelper.writableDatabase).use { GridSizeMigrationUtil.migrate( dbHelper, - GridSizeMigrationUtil.DbReader(it.db, srcTableName, context, MockSet(1)), - GridSizeMigrationUtil.DbReader(it.db, dstTableName, context, MockSet(1)), - targetSize.x, - targetSize, - DeviceGridState( - srcSize.x, - srcSize.y, - srcSize.x, - InvariantDeviceProfile.TYPE_PHONE, - srcTableName - ), - DeviceGridState( - targetSize.x, - targetSize.y, - targetSize.x, - InvariantDeviceProfile.TYPE_PHONE, - dstTableName - ) + GridSizeMigrationUtil.DbReader(it.db, srcGrid.tableName, context, MockSet(1)), + GridSizeMigrationUtil.DbReader(it.db, dstGrid.tableName, context, MockSet(1)), + dstGrid.size.x, + dstGrid.size, + srcGrid.toGridState(), + dstGrid.toGridState() ) it.commit() } - return readDb(dstTableName, dbHelper.readableDatabase) + return readDb(dstGrid.tableName, dbHelper.readableDatabase) } @Test fun runTestCase() { val caseGenerator = ValidGridMigrationTestCaseGenerator(Random(SEED.toLong())) - for (i in 0..50) { - val testCase = caseGenerator.generateTestCase() + for (i in 0..SMALL_TEST_SIZE) { + val testCase = caseGenerator.generateTestCase(isDestEmpty = true) Log.d(TAG, "Test case = $testCase") - val srcItemList = generateItemsForTest(testCase, REPEAT_AFTER) - val dstItemList = migrate(srcItemList, testCase.srcSize, testCase.targetSize) - validate(srcItemList, dstItemList, testCase.targetSize) + val srcGrid = + Grid( + tableName = Favorites.TMP_TABLE, + size = testCase.srcSize, + items = generateItemsForTest(testCase.boards, REPEAT_AFTER) + ) + val dstGrid = + Grid(tableName = Favorites.TABLE_NAME, size = testCase.targetSize, items = listOf()) + validate(srcGrid, dstGrid, migrate(srcGrid, dstGrid)) + } + } + + @Test + fun mergeBoards() { + val caseGenerator = ValidGridMigrationTestCaseGenerator(Random(SEED.toLong())) + for (i in 0..SMALL_TEST_SIZE) { + val testCase = caseGenerator.generateTestCase(isDestEmpty = false) + Log.d(TAG, "Test case = $testCase") + val srcGrid = + Grid( + tableName = Favorites.TMP_TABLE, + size = testCase.srcSize, + items = generateItemsForTest(testCase.boards, REPEAT_AFTER) + ) + val dstGrid = + Grid( + tableName = Favorites.TABLE_NAME, + size = testCase.targetSize, + items = generateItemsForTest(testCase.destBoards, REPEAT_AFTER_DST) + ) + validate(srcGrid, dstGrid, migrate(srcGrid, dstGrid)) } } @@ -162,12 +186,18 @@ class ValidGridMigrationUnitTest { @Test fun runExtensiveTestCases() { val caseGenerator = ValidGridMigrationTestCaseGenerator(Random(SEED.toLong())) - for (i in 0..1000) { - val testCase = caseGenerator.generateTestCase() + for (i in 0..LARGE_TEST_SIZE) { + val testCase = caseGenerator.generateTestCase(isDestEmpty = true) Log.d(TAG, "Test case = $testCase") - val srcItemList = generateItemsForTest(testCase, REPEAT_AFTER) - val dstItemList = migrate(srcItemList, testCase.srcSize, testCase.targetSize) - validate(srcItemList, dstItemList, testCase.targetSize) + val srcGrid = + Grid( + tableName = Favorites.TMP_TABLE, + size = testCase.srcSize, + items = generateItemsForTest(testCase.boards, REPEAT_AFTER) + ) + val dstGrid = + Grid(tableName = Favorites.TABLE_NAME, size = testCase.targetSize, items = listOf()) + validate(srcGrid, dstGrid, migrate(srcGrid, dstGrid)) } } }