From a80aed942934ea582e9f90d1cce6ffbec7a0525f Mon Sep 17 00:00:00 2001 From: fbaron Date: Wed, 15 Jan 2025 14:45:04 -0800 Subject: [PATCH] Fix hotseat issue on grid migration where extra items get removed Flag: EXEMPT bugfix Test: n/a Fix: 333849747 Change-Id: I6bf3b291de33930764ca8e4fac82c59c328adbde --- .../model/GridSizeMigrationDBController.java | 20 ++- .../launcher3/model/GridSizeMigrationLogic.kt | 33 +++- .../launcher3/model/GridSizeMigrationTest.kt | 170 +++++++++++++++++- .../ValidGridMigrationUnitTest.kt | 2 + 4 files changed, 201 insertions(+), 24 deletions(-) diff --git a/src/com/android/launcher3/model/GridSizeMigrationDBController.java b/src/com/android/launcher3/model/GridSizeMigrationDBController.java index 8cbe764c96..9aff90e6e9 100644 --- a/src/com/android/launcher3/model/GridSizeMigrationDBController.java +++ b/src/com/android/launcher3/model/GridSizeMigrationDBController.java @@ -159,8 +159,8 @@ public class GridSizeMigrationDBController { DbReader destReader = new DbReader(t.getDb(), TABLE_NAME, context); Point targetSize = new Point(destDeviceState.getColumns(), destDeviceState.getRows()); - migrate(target, srcReader, destReader, destDeviceState.getNumHotseat(), - targetSize, srcDeviceState, destDeviceState); + migrate(target, srcReader, destReader, srcDeviceState.getNumHotseat(), + destDeviceState.getNumHotseat(), targetSize, srcDeviceState, destDeviceState); dropTable(t.getDb(), TMP_TABLE); t.commit(); return true; @@ -181,19 +181,26 @@ public class GridSizeMigrationDBController { public static boolean migrate( @NonNull DatabaseHelper helper, @NonNull final DbReader srcReader, @NonNull final DbReader destReader, - final int destHotseatSize, @NonNull final Point targetSize, + final int srcHotseatSize, final int destHotseatSize, @NonNull final Point targetSize, @NonNull final DeviceGridState srcDeviceState, @NonNull final DeviceGridState destDeviceState) { final List srcHotseatItems = srcReader.loadHotseatEntries(); final List srcWorkspaceItems = srcReader.loadAllWorkspaceEntries(); final List dstHotseatItems = destReader.loadHotseatEntries(); + // We want to filter out the hotseat items that are placed beyond the size of the source + // grid as we always want to keep those extra items from the destination grid. + List filteredDstHotseatItems = dstHotseatItems; + if (srcHotseatSize < destHotseatSize) { + filteredDstHotseatItems = filteredDstHotseatItems.stream() + .filter(entry -> entry.screenId < srcHotseatSize).toList(); + } final List dstWorkspaceItems = destReader.loadAllWorkspaceEntries(); final List hotseatToBeAdded = new ArrayList<>(1); final List workspaceToBeAdded = new ArrayList<>(1); final IntArray toBeRemoved = new IntArray(); - calcDiff(srcHotseatItems, dstHotseatItems, hotseatToBeAdded, toBeRemoved); + calcDiff(srcHotseatItems, filteredDstHotseatItems, hotseatToBeAdded, toBeRemoved); calcDiff(srcWorkspaceItems, dstWorkspaceItems, workspaceToBeAdded, toBeRemoved); final int trgX = targetSize.x; @@ -421,12 +428,13 @@ public class GridSizeMigrationDBController { } private static void solveHotseatPlacement( - @NonNull final DatabaseHelper helper, final int hotseatSize, + @NonNull final DatabaseHelper helper, + final int dstHotseatSize, @NonNull final DbReader srcReader, @NonNull final DbReader destReader, @NonNull final List placedHotseatItems, @NonNull final List itemsToPlace, List idsInUse) { - final boolean[] occupied = new boolean[hotseatSize]; + final boolean[] occupied = new boolean[dstHotseatSize]; for (DbEntry entry : placedHotseatItems) { occupied[entry.screenId] = true; } diff --git a/src/com/android/launcher3/model/GridSizeMigrationLogic.kt b/src/com/android/launcher3/model/GridSizeMigrationLogic.kt index dfda8e4ab6..fe99fb15e0 100644 --- a/src/com/android/launcher3/model/GridSizeMigrationLogic.kt +++ b/src/com/android/launcher3/model/GridSizeMigrationLogic.kt @@ -107,7 +107,14 @@ class GridSizeMigrationLogic { val idsInUse = mutableListOf() // Migrate hotseat. - migrateHotseat(destDeviceState.numHotseat, srcReader, destReader, target, idsInUse) + migrateHotseat( + srcDeviceState.numHotseat, + destDeviceState.numHotseat, + srcReader, + destReader, + target, + idsInUse, + ) // Migrate workspace. migrateWorkspace(srcReader, destReader, target, targetSize, idsInUse) @@ -134,6 +141,7 @@ class GridSizeMigrationLogic { /** Handles hotseat migration. */ @VisibleForTesting fun migrateHotseat( + srcHotseatSize: Int, destHotseatSize: Int, srcReader: DbReader, destReader: DbReader, @@ -143,17 +151,24 @@ class GridSizeMigrationLogic { val srcHotseatItems = srcReader.loadHotseatEntries() val dstHotseatItems = destReader.loadHotseatEntries() - val hotseatToBeAdded = getItemsToBeAdded(srcHotseatItems, dstHotseatItems) - val toBeRemoved = IntArray() - toBeRemoved.addAll(getItemsToBeRemoved(srcHotseatItems, dstHotseatItems)) + // We want to filter out the hotseat items that are placed beyond the size of the source + // grid as we always want to keep those extra items from the destination grid. + var filteredDstHotseatItems = dstHotseatItems + if (srcHotseatSize < destHotseatSize) { + filteredDstHotseatItems = + filteredDstHotseatItems.filter { entry -> entry.screenId < srcHotseatSize } + } + + val itemsToBeAdded = getItemsToBeAdded(srcHotseatItems, filteredDstHotseatItems) + val itemsToBeRemoved = getItemsToBeRemoved(srcHotseatItems, filteredDstHotseatItems) if (DEBUG) { Log.d( TAG, """Start hotseat migration: - |Removing Hotseat Items: [${dstHotseatItems.filter { toBeRemoved.contains(it.id) } + |Removing Hotseat Items: [${filteredDstHotseatItems.filter { itemsToBeRemoved.contains(it.id) } .joinToString(",\n") { it.toString() }}] - |Adding Hotseat Items: [${hotseatToBeAdded + |Adding Hotseat Items: [${itemsToBeAdded .joinToString(",\n") { it.toString() }}] |""" .trimMargin(), @@ -161,16 +176,16 @@ class GridSizeMigrationLogic { } // Removes the items that we need to remove from the destination DB. - if (!toBeRemoved.isEmpty) { + if (!itemsToBeRemoved.isEmpty) { GridSizeMigrationDBController.removeEntryFromDb( destReader.mDb, destReader.mTableName, - toBeRemoved, + itemsToBeRemoved, ) } placeHotseatItems( - hotseatToBeAdded, + itemsToBeAdded, dstHotseatItems, destHotseatSize, helper, diff --git a/tests/multivalentTests/src/com/android/launcher3/model/GridSizeMigrationTest.kt b/tests/multivalentTests/src/com/android/launcher3/model/GridSizeMigrationTest.kt index eee6191f15..adf38fea93 100644 --- a/tests/multivalentTests/src/com/android/launcher3/model/GridSizeMigrationTest.kt +++ b/tests/multivalentTests/src/com/android/launcher3/model/GridSizeMigrationTest.kt @@ -134,6 +134,7 @@ class GridSizeMigrationTest { var gridSizeMigrationLogic = GridSizeMigrationLogic() val idsInUse = mutableListOf() gridSizeMigrationLogic.migrateHotseat( + 5, idp.numDatabaseHotseatIcons, srcReader, destReader, @@ -152,6 +153,7 @@ class GridSizeMigrationTest { dbHelper, srcReader, destReader, + 5, idp.numDatabaseHotseatIcons, Point(idp.numColumns, idp.numRows), DeviceGridState(context), @@ -277,6 +279,7 @@ class GridSizeMigrationTest { var gridSizeMigrationLogic = GridSizeMigrationLogic() val idsInUse = mutableListOf() gridSizeMigrationLogic.migrateHotseat( + 5, idp.numDatabaseHotseatIcons, readerGridA, readerGridB, @@ -295,6 +298,7 @@ class GridSizeMigrationTest { dbHelper, readerGridA, readerGridB, + 5, idp.numDatabaseHotseatIcons, Point(idp.numColumns, idp.numRows), DeviceGridState(context), @@ -317,8 +321,8 @@ class GridSizeMigrationTest { // 2 1 3 4 verifyHotseat( c, - idp, mutableListOf(testPackage2, testPackage1, testPackage3, testPackage4).toList(), + 4, ) // Check workspace items in grid B @@ -348,7 +352,7 @@ class GridSizeMigrationTest { addItem(ITEM_TYPE_APPLICATION, 0, CONTAINER_DESKTOP, 0, 2, testPackage9) // migrate from B -> A - migrateGrid(dbHelper, readerGridB, readerGridA, 5, 5, 5) + migrateGrid(dbHelper, readerGridB, readerGridA, 4, 5, 5, 5) // Check hotseat items in grid A c = @@ -362,11 +366,12 @@ class GridSizeMigrationTest { null, ) ?: throw IllegalStateException() // Expected hotseat items in grid A - // 1 2 _ 3 4 + // 1 2 4 3 4 verifyHotseat( c, - idp, - mutableListOf(testPackage1, testPackage2, null, testPackage3, testPackage4).toList(), + mutableListOf(testPackage1, testPackage2, testPackage4, testPackage3, testPackage4) + .toList(), + 5, ) // Check workspace items in grid A @@ -404,6 +409,7 @@ class GridSizeMigrationTest { dbHelper, readerGridA, readerGridB, + 5, idp.numDatabaseHotseatIcons, idp.numColumns, idp.numRows, @@ -424,8 +430,8 @@ class GridSizeMigrationTest { // 2 1 3 4 verifyHotseat( c, - idp, mutableListOf(testPackage2, testPackage1, testPackage3, testPackage4).toList(), + 4, ) // Check workspace items in grid B @@ -452,10 +458,150 @@ class GridSizeMigrationTest { assertThat(locMap[testPackage9]).isEqualTo(Triple(0, 0, 2)) } + @Test + @Throws(Exception::class) + @EnableFlags(Flags.FLAG_GRID_MIGRATION_REFACTOR) + fun testHotseatMigrationToSmallerGridBackAndForthFlagOn() { + testHotseatMigrationToSmallerGridBackAndForth() + } + + @Test + @Throws(Exception::class) + @DisableFlags(Flags.FLAG_GRID_MIGRATION_REFACTOR) + fun testHotseatMigrationToSmallerGridBackAndForthFlagOff() { + testHotseatMigrationToSmallerGridBackAndForth() + } + + /** Old migration logic, should be modified once is not needed anymore */ + @Throws(Exception::class) + fun testHotseatMigrationToSmallerGridBackAndForth() { + // Hotseat items in grid A + // 1 2 3 4 5 + addItem(ITEM_TYPE_APPLICATION, 0, CONTAINER_HOTSEAT, 0, 0, testPackage1, 1, TMP_TABLE) + addItem(ITEM_TYPE_DEEP_SHORTCUT, 1, CONTAINER_HOTSEAT, 0, 0, testPackage2, 2, TMP_TABLE) + addItem(ITEM_TYPE_DEEP_SHORTCUT, 2, CONTAINER_HOTSEAT, 0, 0, testPackage3, 3, TMP_TABLE) + addItem(ITEM_TYPE_APPLICATION, 3, CONTAINER_HOTSEAT, 0, 0, testPackage4, 4, TMP_TABLE) + addItem(ITEM_TYPE_APPLICATION, 4, CONTAINER_HOTSEAT, 0, 0, testPackage5, 5, TMP_TABLE) + + // Hotseat items in grid B + // 2 _ _ _ + addItem(ITEM_TYPE_DEEP_SHORTCUT, 0, CONTAINER_HOTSEAT, 0, 0, testPackage2) + + idp.numDatabaseHotseatIcons = 4 + idp.numColumns = 4 + idp.numRows = 4 + val readerGridA = DbReader(db, TMP_TABLE, context) + val readerGridB = DbReader(db, TABLE_NAME, context) + // migrate from A -> B + if (Flags.gridMigrationRefactor()) { + var gridSizeMigrationLogic = GridSizeMigrationLogic() + val idsInUse = mutableListOf() + gridSizeMigrationLogic.migrateHotseat( + 5, + idp.numDatabaseHotseatIcons, + readerGridA, + readerGridB, + dbHelper, + idsInUse, + ) + gridSizeMigrationLogic.migrateWorkspace( + readerGridA, + readerGridB, + dbHelper, + Point(idp.numColumns, idp.numRows), + idsInUse, + ) + } else { + GridSizeMigrationDBController.migrate( + dbHelper, + readerGridA, + readerGridB, + 5, + idp.numDatabaseHotseatIcons, + Point(idp.numColumns, idp.numRows), + DeviceGridState(context), + DeviceGridState(idp), + ) + } + + // Check hotseat items in grid B + var c = + db.query( + TABLE_NAME, + arrayOf(SCREEN, INTENT), + "container=$CONTAINER_HOTSEAT", + null, + SCREEN, + null, + null, + ) ?: throw IllegalStateException() + // Expected hotseat items in grid B + // 2 1 3 4 + verifyHotseat( + c, + mutableListOf(testPackage2, testPackage1, testPackage3, testPackage4).toList(), + 4, + ) + + // migrate from B -> A + migrateGrid(dbHelper, readerGridB, readerGridA, idp.numDatabaseHotseatIcons, 5, 5, 5) + + // Check hotseat items in grid A + c = + db.query( + TMP_TABLE, + arrayOf(SCREEN, INTENT), + "container=$CONTAINER_HOTSEAT", + null, + SCREEN, + null, + null, + ) ?: throw IllegalStateException() + // Expected hotseat items in grid A + // 1 2 3 4 5 + verifyHotseat( + c, + mutableListOf(testPackage1, testPackage2, testPackage3, testPackage4, testPackage5) + .toList(), + 5, + ) + + // migrate from A -> B + migrateGrid( + dbHelper, + readerGridA, + readerGridB, + 5, + idp.numDatabaseHotseatIcons, + idp.numColumns, + idp.numRows, + ) + + // Check hotseat items in grid B + c = + db.query( + TABLE_NAME, + arrayOf(SCREEN, INTENT), + "container=$CONTAINER_HOTSEAT", + null, + SCREEN, + null, + null, + ) ?: throw IllegalStateException() + // Expected hotseat items in grid B + // 2 1 3 4 + verifyHotseat( + c, + mutableListOf(testPackage2, testPackage1, testPackage3, testPackage4).toList(), + 4, + ) + } + private fun migrateGrid( dbHelper: DatabaseHelper, srcReader: DbReader, destReader: DbReader, + srcHotseatSize: Int, destHotseatSize: Int, pointX: Int, pointY: Int, @@ -464,7 +610,8 @@ class GridSizeMigrationTest { var gridSizeMigrationLogic = GridSizeMigrationLogic() val idsInUse = mutableListOf() gridSizeMigrationLogic.migrateHotseat( - idp.numDatabaseHotseatIcons, + srcHotseatSize, + destHotseatSize, srcReader, destReader, dbHelper, @@ -482,6 +629,7 @@ class GridSizeMigrationTest { dbHelper, srcReader, destReader, + srcHotseatSize, destHotseatSize, Point(pointX, pointY), DeviceGridState(idp), @@ -490,8 +638,8 @@ class GridSizeMigrationTest { } } - private fun verifyHotseat(c: Cursor, idp: InvariantDeviceProfile, expected: List) { - assertThat(c.count).isEqualTo(idp.numDatabaseHotseatIcons) + private fun verifyHotseat(c: Cursor, expected: List, expectedCount: Int) { + assertThat(c.count).isEqualTo(expectedCount) val screenIndex = c.getColumnIndex(SCREEN) val intentIndex = c.getColumnIndex(INTENT) expected.forEachIndexed { idx, pkg -> @@ -584,6 +732,7 @@ class GridSizeMigrationTest { dbHelper, srcReader, destReader, + 4, idp.numDatabaseHotseatIcons, idp.numColumns, idp.numRows, @@ -651,6 +800,7 @@ class GridSizeMigrationTest { dbHelper, srcReader, destReader, + 6, idp.numDatabaseHotseatIcons, idp.numColumns, idp.numRows, @@ -729,6 +879,7 @@ class GridSizeMigrationTest { dbHelper, srcReader, destReader, + 2, idp.numDatabaseHotseatIcons, idp.numColumns, idp.numRows, @@ -801,6 +952,7 @@ class GridSizeMigrationTest { dbHelper, srcReader, destReader, + 5, idp.numDatabaseHotseatIcons, idp.numColumns, idp.numRows, diff --git a/tests/multivalentTests/src/com/android/launcher3/model/gridmigration/ValidGridMigrationUnitTest.kt b/tests/multivalentTests/src/com/android/launcher3/model/gridmigration/ValidGridMigrationUnitTest.kt index b96dbcddba..1d1e7ebd1b 100644 --- a/tests/multivalentTests/src/com/android/launcher3/model/gridmigration/ValidGridMigrationUnitTest.kt +++ b/tests/multivalentTests/src/com/android/launcher3/model/gridmigration/ValidGridMigrationUnitTest.kt @@ -138,6 +138,7 @@ class ValidGridMigrationUnitTest { val gridSizeMigrationLogic = GridSizeMigrationLogic() val idsInUse = mutableListOf() gridSizeMigrationLogic.migrateHotseat( + srcGrid.size.x, dstGrid.size.x, GridSizeMigrationDBController.DbReader(it.db, srcGrid.tableName, context), GridSizeMigrationDBController.DbReader(it.db, dstGrid.tableName, context), @@ -156,6 +157,7 @@ class ValidGridMigrationUnitTest { dbHelper, GridSizeMigrationDBController.DbReader(it.db, srcGrid.tableName, context), GridSizeMigrationDBController.DbReader(it.db, dstGrid.tableName, context), + srcGrid.size.x, dstGrid.size.x, dstGrid.size, srcGrid.toGridState(),