Make grid size migration less confusing for users.
Grid size migration is a confusing experience for users because the
content of the grid is not predicatable to the user. Part of the reason
was the dedupe logic which merges items from new grid into old grid,
this mean if user removes an item, change to another grid and go back,
user will see that same item re-appears.
This CL keeps the content of target grid the same as source gird, i.e
user will will get the exact same set of app icons/shortcuts/folders and
widgets (if size permits) across grid sizes. The only difference being
their placement in the grid.
Bug: 256859723
Test: atest GridSizeMigrationUtilTest
Change-Id: I1732c91e441ad44bc43e0a943566a83563d12b07
diff --git a/src/com/android/launcher3/model/GridSizeMigrationUtil.java b/src/com/android/launcher3/model/GridSizeMigrationUtil.java
index d63408b..eded5ea 100644
--- a/src/com/android/launcher3/model/GridSizeMigrationUtil.java
+++ b/src/com/android/launcher3/model/GridSizeMigrationUtil.java
@@ -66,7 +66,7 @@
public class GridSizeMigrationUtil {
private static final String TAG = "GridSizeMigrationUtil";
- private static final boolean DEBUG = false;
+ private static final boolean DEBUG = true;
private GridSizeMigrationUtil() {
// Util class should not be instantiated
@@ -188,27 +188,54 @@
@NonNull final DeviceGridState srcDeviceState,
@NonNull final DeviceGridState destDeviceState) {
- final List<DbEntry> hotseatItems = destReader.loadHotseatEntries();
- final List<DbEntry> workspaceItems = destReader.loadAllWorkspaceEntries();
- final List<DbEntry> hotseatDiff =
- calcDiff(srcReader.loadHotseatEntries(), hotseatItems);
- final List<DbEntry> workspaceDiff =
- calcDiff(srcReader.loadAllWorkspaceEntries(), workspaceItems);
+ final List<DbEntry> srcHotseatItems = srcReader.loadHotseatEntries();
+ final List<DbEntry> srcWorkspaceItems = srcReader.loadAllWorkspaceEntries();
+ final List<DbEntry> dstHotseatItems = destReader.loadHotseatEntries();
+ final List<DbEntry> dstWorkspaceItems = destReader.loadAllWorkspaceEntries();
+ final List<DbEntry> hotseatToBeAdded = new ArrayList<>(1);
+ final List<DbEntry> workspaceToBeAdded = new ArrayList<>(1);
+ final IntArray toBeRemoved = new IntArray();
+
+ calcDiff(srcHotseatItems, dstHotseatItems, hotseatToBeAdded, toBeRemoved);
+ calcDiff(srcWorkspaceItems, dstWorkspaceItems, workspaceToBeAdded, toBeRemoved);
final int trgX = targetSize.x;
final int trgY = targetSize.y;
- if (hotseatDiff.isEmpty() && workspaceDiff.isEmpty()) {
+ if (DEBUG) {
+ Log.d(TAG, "Start migration:"
+ + "\n Source Device:"
+ + srcWorkspaceItems.stream().map(DbEntry::toString).collect(
+ Collectors.joining(",\n", "[", "]"))
+ + "\n Target Device:"
+ + dstWorkspaceItems.stream().map(DbEntry::toString).collect(
+ Collectors.joining(",\n", "[", "]"))
+ + "\n Removing Items:"
+ + dstWorkspaceItems.stream().filter(entry ->
+ toBeRemoved.contains(entry.id)).map(DbEntry::toString).collect(
+ Collectors.joining(",\n", "[", "]"))
+ + "\n Adding Workspace Items:"
+ + workspaceToBeAdded.stream().map(DbEntry::toString).collect(
+ Collectors.joining(",\n", "[", "]"))
+ + "\n Adding Hotseat Items:"
+ + hotseatToBeAdded.stream().map(DbEntry::toString).collect(
+ Collectors.joining(",\n", "[", "]"))
+ );
+ }
+ if (!toBeRemoved.isEmpty()) {
+ removeEntryFromDb(destReader.mDb, destReader.mTableName, toBeRemoved);
+ }
+ if (hotseatToBeAdded.isEmpty() && workspaceToBeAdded.isEmpty()) {
return false;
}
// Sort the items by the reading order.
- Collections.sort(hotseatDiff);
- Collections.sort(workspaceDiff);
+ Collections.sort(hotseatToBeAdded);
+ Collections.sort(workspaceToBeAdded);
// Migrate hotseat
solveHotseatPlacement(db, srcReader,
- destReader, context, destHotseatSize, hotseatItems, hotseatDiff);
+ destReader, context, destHotseatSize, dstHotseatItems, hotseatToBeAdded);
// Migrate workspace.
// First we create a collection of the screens
@@ -229,8 +256,8 @@
Log.d(TAG, "Migrating " + screenId);
}
solveGridPlacement(db, srcReader,
- destReader, context, screenId, trgX, trgY, workspaceDiff, false);
- if (workspaceDiff.isEmpty()) {
+ destReader, context, screenId, trgX, trgY, workspaceToBeAdded, false);
+ if (workspaceToBeAdded.isEmpty()) {
break;
}
}
@@ -238,42 +265,37 @@
// In case the new grid is smaller, there might be some leftover items that don't fit on
// any of the screens, in this case we add them to new screens until all of them are placed.
int screenId = destReader.mLastScreenId + 1;
- while (!workspaceDiff.isEmpty()) {
+ while (!workspaceToBeAdded.isEmpty()) {
solveGridPlacement(db, srcReader,
- destReader, context, screenId, trgX, trgY, workspaceDiff, preservePages);
+ destReader, context, screenId, trgX, trgY, workspaceToBeAdded, preservePages);
screenId++;
}
return true;
}
- /** Return what's in the src but not in the dest */
- private static List<DbEntry> calcDiff(List<DbEntry> src, List<DbEntry> dest) {
- Map<String, Integer> destIdSet = new HashMap<>();
- for (DbEntry entry : dest) {
- String entryID = entry.getEntryMigrationId();
- if (destIdSet.containsKey(entryID)) {
- destIdSet.put(entryID, destIdSet.get(entryID) + 1);
- } else {
- destIdSet.put(entryID, 1);
+ /**
+ * Calculate the differences between {@code src} (denoted by A) and {@code dest}
+ * (denoted by B).
+ * All DbEntry in A - B will be added to {@code toBeAdded}
+ * All DbEntry.id in B - A will be added to {@code toBeRemoved}
+ */
+ private static void calcDiff(@NonNull final List<DbEntry> src,
+ @NonNull final List<DbEntry> dest, @NonNull final List<DbEntry> toBeAdded,
+ @NonNull final IntArray toBeRemoved) {
+ src.forEach(entry -> {
+ if (!dest.contains(entry)) {
+ toBeAdded.add(entry);
}
- }
- List<DbEntry> diff = new ArrayList<>();
- for (DbEntry entry : src) {
- String entryID = entry.getEntryMigrationId();
- if (destIdSet.containsKey(entryID)) {
- Integer count = destIdSet.get(entryID);
- if (count <= 0) {
- diff.add(entry);
- destIdSet.remove(entryID);
- } else {
- destIdSet.put(entryID, count - 1);
+ });
+ dest.forEach(entry -> {
+ if (!src.contains(entry)) {
+ toBeRemoved.add(entry.id);
+ if (entry.itemType == LauncherSettings.Favorites.ITEM_TYPE_FOLDER) {
+ entry.mFolderItems.values().forEach(ids -> ids.forEach(toBeRemoved::add));
}
- } else {
- diff.add(entry);
}
- }
- return diff;
+ });
}
private static void insertEntryInDb(SQLiteDatabase db, Context context, DbEntry entry,
@@ -682,12 +704,12 @@
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
DbEntry entry = (DbEntry) o;
- return Objects.equals(mIntent, entry.mIntent);
+ return Objects.equals(getEntryMigrationId(), entry.getEntryMigrationId());
}
@Override
public int hashCode() {
- return Objects.hash(mIntent);
+ return Objects.hash(getEntryMigrationId());
}
public void updateContentValues(ContentValues values) {
diff --git a/tests/src/com/android/launcher3/model/GridSizeMigrationUtilTest.kt b/tests/src/com/android/launcher3/model/GridSizeMigrationUtilTest.kt
index 85d7bf9..76a186b 100644
--- a/tests/src/com/android/launcher3/model/GridSizeMigrationUtilTest.kt
+++ b/tests/src/com/android/launcher3/model/GridSizeMigrationUtilTest.kt
@@ -17,6 +17,7 @@
import android.content.Context
import android.content.Intent
+import android.database.Cursor;
import android.database.sqlite.SQLiteDatabase
import android.graphics.Point
import android.os.Process
@@ -183,15 +184,232 @@
// Expected dest grid icons
// _ _ _ _
// 5 6 7 8
- // 9 _ 10_
+ // 9 _ _ _
// _ _ _ _
- assertThat(locMap.size.toLong()).isEqualTo(6)
+ assertThat(locMap.size.toLong()).isEqualTo(5)
assertThat(locMap[testPackage5]).isEqualTo(Point(0, 1))
assertThat(locMap[testPackage6]).isEqualTo(Point(1, 1))
assertThat(locMap[testPackage7]).isEqualTo(Point(2, 1))
assertThat(locMap[testPackage8]).isEqualTo(Point(3, 1))
assertThat(locMap[testPackage9]).isEqualTo(Point(0, 2))
- assertThat(locMap[testPackage10]).isEqualTo(Point(2, 2))
+ }
+
+ /**
+ * Old migration logic, should be modified once [FeatureFlags.ENABLE_NEW_MIGRATION_LOGIC] is
+ * not needed anymore
+ */
+ @Test
+ @Throws(Exception::class)
+ fun testMigrationBackAndForth() {
+ // Hotseat items in grid A
+ // 1 2 _ 3 4
+ modelHelper.addItem(APP_ICON, 0, HOTSEAT, 0, 0, testPackage1, 1, TMP_CONTENT_URI)
+ modelHelper.addItem(SHORTCUT, 1, HOTSEAT, 0, 0, testPackage2, 2, TMP_CONTENT_URI)
+ modelHelper.addItem(SHORTCUT, 3, HOTSEAT, 0, 0, testPackage3, 3, TMP_CONTENT_URI)
+ modelHelper.addItem(APP_ICON, 4, HOTSEAT, 0, 0, testPackage4, 4, TMP_CONTENT_URI)
+ // Workspace items in grid A
+ // _ _ _ _ _
+ // _ _ _ _ 5
+ // _ _ 6 _ 7
+ // _ _ 8 _ _
+ // _ _ _ _ _
+ modelHelper.addItem(APP_ICON, 0, DESKTOP, 4, 1, testPackage5, 5, TMP_CONTENT_URI)
+ modelHelper.addItem(APP_ICON, 0, DESKTOP, 2, 2, testPackage6, 6, TMP_CONTENT_URI)
+ modelHelper.addItem(APP_ICON, 0, DESKTOP, 4, 2, testPackage7, 7, TMP_CONTENT_URI)
+ modelHelper.addItem(APP_ICON, 0, DESKTOP, 2, 3, testPackage8, 8, TMP_CONTENT_URI)
+
+ // Hotseat items in grid B
+ // 2 _ _ _
+ modelHelper.addItem(SHORTCUT, 0, HOTSEAT, 0, 0, testPackage2)
+ // Workspace items in grid B
+ // _ _ _ _
+ // _ _ _ 10
+ // _ _ _ _
+ // _ _ _ _
+ modelHelper.addItem(APP_ICON, 0, DESKTOP, 1, 3, testPackage10)
+
+ idp.numDatabaseHotseatIcons = 4
+ idp.numColumns = 4
+ idp.numRows = 4
+ val readerGridA = DbReader(db, TMP_TABLE, context, validPackages)
+ val readerGridB = DbReader(db, TABLE_NAME, context, validPackages)
+ // migrate from A -> B
+ GridSizeMigrationUtil.migrate(
+ context,
+ db,
+ readerGridA,
+ readerGridB,
+ idp.numDatabaseHotseatIcons,
+ Point(idp.numColumns, idp.numRows),
+ DeviceGridState(context),
+ DeviceGridState(idp)
+ )
+
+ // Check hotseat items in grid B
+ var c = context.contentResolver.query(
+ CONTENT_URI,
+ arrayOf(SCREEN, INTENT),
+ "container=$CONTAINER_HOTSEAT",
+ null,
+ SCREEN,
+ null
+ ) ?: throw IllegalStateException()
+ // Expected hotseat items in grid B
+ // 2 1 3 4
+ verifyHotseat(c, idp,
+ mutableListOf(testPackage2, testPackage1, testPackage3, testPackage4).toList())
+
+ // Check workspace items in grid B
+ c = context.contentResolver.query(
+ CONTENT_URI,
+ arrayOf(SCREEN, CELLX, CELLY, INTENT),
+ "container=$CONTAINER_DESKTOP",
+ null,
+ null,
+ null
+ ) ?: throw IllegalStateException()
+ var locMap = parseLocMap(context, c)
+ // Expected items in grid B
+ // _ _ _ _
+ // 5 6 7 8
+ // _ _ _ _
+ // _ _ _ _
+ assertThat(locMap.size.toLong()).isEqualTo(4)
+ assertThat(locMap[testPackage5]).isEqualTo(Triple(0, 0, 1))
+ assertThat(locMap[testPackage6]).isEqualTo(Triple(0, 1, 1))
+ assertThat(locMap[testPackage7]).isEqualTo(Triple(0, 2, 1))
+ assertThat(locMap[testPackage8]).isEqualTo(Triple(0, 3, 1))
+
+ // add item in B
+ modelHelper.addItem(APP_ICON, 0, DESKTOP, 0, 2, testPackage9)
+
+ // migrate from B -> A
+ GridSizeMigrationUtil.migrate(
+ context,
+ db,
+ readerGridB,
+ readerGridA,
+ 5,
+ Point(5, 5),
+ DeviceGridState(idp),
+ DeviceGridState(context)
+ )
+ // Check hotseat items in grid A
+ c = context.contentResolver.query(
+ TMP_CONTENT_URI,
+ arrayOf(SCREEN, INTENT),
+ "container=$CONTAINER_HOTSEAT",
+ null,
+ SCREEN,
+ null
+ ) ?: throw IllegalStateException()
+ // Expected hotseat items in grid A
+ // 1 2 _ 3 4
+ verifyHotseat(c, idp, mutableListOf(
+ testPackage1, testPackage2, null, testPackage3, testPackage4).toList())
+
+ // Check workspace items in grid A
+ c = context.contentResolver.query(
+ TMP_CONTENT_URI,
+ arrayOf(SCREEN, CELLX, CELLY, INTENT),
+ "container=$CONTAINER_DESKTOP",
+ null,
+ null,
+ null
+ ) ?: throw IllegalStateException()
+ locMap = parseLocMap(context, c)
+ // Expected workspace items in grid A
+ // _ _ _ _ _
+ // _ _ _ _ 5
+ // 9 _ 6 _ 7
+ // _ _ 8 _ _
+ // _ _ _ _ _
+ assertThat(locMap.size.toLong()).isEqualTo(5)
+ // Verify items that existed in grid A remains in same position
+ assertThat(locMap[testPackage5]).isEqualTo(Triple(0, 4, 1))
+ assertThat(locMap[testPackage6]).isEqualTo(Triple(0, 2, 2))
+ assertThat(locMap[testPackage7]).isEqualTo(Triple(0, 4, 2))
+ assertThat(locMap[testPackage8]).isEqualTo(Triple(0, 2, 3))
+ // Verify items that didn't exist in grid A are added in new screen
+ assertThat(locMap[testPackage9]).isEqualTo(Triple(0, 0, 2))
+
+ // remove item from B
+ modelHelper.deleteItem(7, TMP_TABLE)
+
+ // migrate from A -> B
+ GridSizeMigrationUtil.migrate(
+ context,
+ db,
+ readerGridA,
+ readerGridB,
+ idp.numDatabaseHotseatIcons,
+ Point(idp.numColumns, idp.numRows),
+ DeviceGridState(context),
+ DeviceGridState(idp)
+ )
+
+ // Check hotseat items in grid B
+ c = context.contentResolver.query(
+ CONTENT_URI,
+ arrayOf(SCREEN, INTENT),
+ "container=$CONTAINER_HOTSEAT",
+ null,
+ SCREEN,
+ null
+ ) ?: throw IllegalStateException()
+ // Expected hotseat items in grid B
+ // 2 1 3 4
+ verifyHotseat(c, idp,
+ mutableListOf(testPackage2, testPackage1, testPackage3, testPackage4).toList())
+
+ // Check workspace items in grid B
+ c = context.contentResolver.query(
+ CONTENT_URI,
+ arrayOf(SCREEN, CELLX, CELLY, INTENT),
+ "container=$CONTAINER_DESKTOP",
+ null,
+ null,
+ null
+ ) ?: throw IllegalStateException()
+ locMap = parseLocMap(context, c)
+ // Expected workspace items in grid B
+ // _ _ _ _
+ // 5 6 _ 8
+ // 9 _ _ _
+ // _ _ _ _
+ assertThat(locMap.size.toLong()).isEqualTo(4)
+ assertThat(locMap[testPackage5]).isEqualTo(Triple(0, 0, 1))
+ assertThat(locMap[testPackage6]).isEqualTo(Triple(0, 1, 1))
+ assertThat(locMap[testPackage8]).isEqualTo(Triple(0, 3, 1))
+ assertThat(locMap[testPackage9]).isEqualTo(Triple(0, 0, 2))
+ }
+
+ private fun verifyHotseat(c: Cursor, idp: InvariantDeviceProfile, expected: List<String?>) {
+ assertThat(c.count).isEqualTo(idp.numDatabaseHotseatIcons)
+ val screenIndex = c.getColumnIndex(SCREEN)
+ val intentIndex = c.getColumnIndex(INTENT)
+ expected.forEachIndexed { idx, pkg ->
+ if (pkg == null) return@forEachIndexed
+ c.moveToNext()
+ assertThat(c.getInt(screenIndex).toLong()).isEqualTo(idx)
+ assertThat(c.getString(intentIndex)).contains(pkg)
+ }
+ c.close()
+ }
+
+ private fun parseLocMap(context: Context, c: Cursor): Map<String, Triple<Int, Int, Int>> {
+ // Check workspace items
+ val intentIndex = c.getColumnIndex(INTENT)
+ val screenIndex = c.getColumnIndex(SCREEN)
+ val cellXIndex = c.getColumnIndex(CELLX)
+ val cellYIndex = c.getColumnIndex(CELLY)
+ val locMap = mutableMapOf<String, Triple<Int, Int, Int>>()
+ while (c.moveToNext()) {
+ locMap[Intent.parseUri(c.getString(intentIndex), 0).getPackage()] =
+ Triple(c.getInt(screenIndex), c.getInt(cellXIndex), c.getInt(cellYIndex))
+ }
+ c.close()
+ return locMap.toMap()
}
@Test
diff --git a/tests/src/com/android/launcher3/util/LauncherModelHelper.java b/tests/src/com/android/launcher3/util/LauncherModelHelper.java
index e7e551f..93bf312 100644
--- a/tests/src/com/android/launcher3/util/LauncherModelHelper.java
+++ b/tests/src/com/android/launcher3/util/LauncherModelHelper.java
@@ -362,6 +362,12 @@
sandboxContext.getContentResolver().insert(contentUri, values);
}
+ public void deleteItem(int itemId, @NonNull final String tableName) {
+ final Uri uri = Uri.parse("content://"
+ + LauncherProvider.AUTHORITY + "/" + tableName + "/" + itemId);
+ sandboxContext.getContentResolver().delete(uri, null, null);
+ }
+
public int[][][] createGrid(int[][][] typeArray) {
return createGrid(typeArray, 1);
}