From bd670dd9d5d6774fffb469c64ade55ac9c4dbf79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Pomini?= Date: Wed, 21 Jun 2023 12:54:30 +0000 Subject: [PATCH] Replace WallpaperData File fields with methods WallpaperData objects do not need to store File objects directly. There are 4 wallpaper files per user, and since File objects are immutable in java we can keep a global reference of them. WallpaperData objects can retrieve the right files using their mWhich and userId fields. Doing so prevents ending up in weird states, e.g. if the mWhich flag changes but not the wallpaperFile or cropFile. Test: atest WallpaperManagerTest Test: atest WallpaperManagerServiceTests Bug: 286909680 Change-Id: I4cf9fc6311238d94213c4af3c6fb8beed9fc9023 --- .../server/wallpaper/WallpaperCropper.java | 21 +++++----- .../server/wallpaper/WallpaperData.java | 41 ++++++++++++------- .../server/wallpaper/WallpaperDataParser.java | 10 ++--- .../wallpaper/WallpaperManagerService.java | 35 ++++++++-------- .../WallpaperManagerServiceTests.java | 15 +++---- 5 files changed, 68 insertions(+), 54 deletions(-) diff --git a/services/core/java/com/android/server/wallpaper/WallpaperCropper.java b/services/core/java/com/android/server/wallpaper/WallpaperCropper.java index 49b125cd49e8b..b773ade09b890 100644 --- a/services/core/java/com/android/server/wallpaper/WallpaperCropper.java +++ b/services/core/java/com/android/server/wallpaper/WallpaperCropper.java @@ -81,7 +81,7 @@ class WallpaperCropper { if (DEBUG) { Slog.v(TAG, "Generating crop for new wallpaper(s): 0x" + Integer.toHexString(wallpaper.mWhich) - + " to " + wallpaper.cropFile.getName() + + " to " + wallpaper.getCropFile().getName() + " crop=(" + cropHint.width() + 'x' + cropHint.height() + ") dim=(" + wpData.mWidth + 'x' + wpData.mHeight + ')'); } @@ -89,7 +89,7 @@ class WallpaperCropper { // Analyse the source; needed in multiple cases BitmapFactory.Options options = new BitmapFactory.Options(); options.inJustDecodeBounds = true; - BitmapFactory.decodeFile(wallpaper.wallpaperFile.getAbsolutePath(), options); + BitmapFactory.decodeFile(wallpaper.getWallpaperFile().getAbsolutePath(), options); if (options.outWidth <= 0 || options.outHeight <= 0) { Slog.w(TAG, "Invalid wallpaper data"); success = false; @@ -154,11 +154,10 @@ class WallpaperCropper { // may be we can try to remove this optimized way in the future, // that means, we will always go into the 'else' block. - success = FileUtils.copyFile(wallpaper.wallpaperFile, wallpaper.cropFile); + success = FileUtils.copyFile(wallpaper.getWallpaperFile(), wallpaper.getCropFile()); if (!success) { - wallpaper.cropFile.delete(); - // TODO: fall back to default wallpaper in this case + wallpaper.getCropFile().delete(); } if (DEBUG) { @@ -226,7 +225,7 @@ class WallpaperCropper { //Create a record file and will delete if ImageDecoder work well. final String recordName = - (wallpaper.wallpaperFile.getName().equals(WALLPAPER) + (wallpaper.getWallpaperFile().getName().equals(WALLPAPER) ? RECORD_FILE : RECORD_LOCK_FILE); final File record = new File(getWallpaperDir(wallpaper.userId), recordName); record.createNewFile(); @@ -234,7 +233,7 @@ class WallpaperCropper { + ", record name =" + record.getName()); final ImageDecoder.Source srcData = - ImageDecoder.createSource(wallpaper.wallpaperFile); + ImageDecoder.createSource(wallpaper.getWallpaperFile()); final int sampleSize = scale; Bitmap cropped = ImageDecoder.decodeBitmap(srcData, (decoder, info, src) -> { decoder.setTargetSampleSize(sampleSize); @@ -257,7 +256,7 @@ class WallpaperCropper { + " h=" + finalCrop.getHeight()); } - f = new FileOutputStream(wallpaper.cropFile); + f = new FileOutputStream(wallpaper.getCropFile()); bos = new BufferedOutputStream(f, 32 * 1024); finalCrop.compress(Bitmap.CompressFormat.PNG, 100, bos); // don't rely on the implicit flush-at-close when noting success @@ -277,11 +276,11 @@ class WallpaperCropper { if (!success) { Slog.e(TAG, "Unable to apply new wallpaper"); - wallpaper.cropFile.delete(); + wallpaper.getCropFile().delete(); } - if (wallpaper.cropFile.exists()) { - boolean didRestorecon = SELinux.restorecon(wallpaper.cropFile.getAbsoluteFile()); + if (wallpaper.getCropFile().exists()) { + boolean didRestorecon = SELinux.restorecon(wallpaper.getCropFile().getAbsoluteFile()); if (DEBUG) { Slog.v(TAG, "restorecon() of crop file returned " + didRestorecon); } diff --git a/services/core/java/com/android/server/wallpaper/WallpaperData.java b/services/core/java/com/android/server/wallpaper/WallpaperData.java index d87fca4d3c716..b0b66cfa247f9 100644 --- a/services/core/java/com/android/server/wallpaper/WallpaperData.java +++ b/services/core/java/com/android/server/wallpaper/WallpaperData.java @@ -40,10 +40,7 @@ import java.io.File; */ class WallpaperData { - int userId; - - final File wallpaperFile; // source image - final File cropFile; // eventual destination + final int userId; /** * True while the client is writing a new wallpaper @@ -133,14 +130,13 @@ class WallpaperData { */ final Rect cropHint = new Rect(0, 0, 0, 0); + // map of which -> File + private final SparseArray mWallpaperFiles = new SparseArray<>(); + private final SparseArray mCropFiles = new SparseArray<>(); + WallpaperData(int userId, @SetWallpaperFlags int wallpaperType) { this.userId = userId; this.mWhich = wallpaperType; - File wallpaperDir = getWallpaperDir(userId); - String wallpaperFileName = (wallpaperType == FLAG_LOCK) ? WALLPAPER_LOCK_ORIG : WALLPAPER; - String cropFileName = (wallpaperType == FLAG_LOCK) ? WALLPAPER_LOCK_CROP : WALLPAPER_CROP; - this.wallpaperFile = new File(wallpaperDir, wallpaperFileName); - this.cropFile = new File(wallpaperDir, cropFileName); } /** @@ -154,8 +150,6 @@ class WallpaperData { */ WallpaperData(WallpaperData source) { this.userId = source.userId; - this.wallpaperFile = source.wallpaperFile; - this.cropFile = source.cropFile; this.wallpaperComponent = source.wallpaperComponent; this.mWhich = source.mWhich; this.wallpaperId = source.wallpaperId; @@ -169,6 +163,25 @@ class WallpaperData { } } + File getWallpaperFile() { + String fileName = mWhich == FLAG_LOCK ? WALLPAPER_LOCK_ORIG : WALLPAPER; + return getFile(mWallpaperFiles, fileName); + } + + File getCropFile() { + String fileName = mWhich == FLAG_LOCK ? WALLPAPER_LOCK_CROP : WALLPAPER_CROP; + return getFile(mCropFiles, fileName); + } + + private File getFile(SparseArray map, String fileName) { + File result = map.get(mWhich); + if (result == null) { + result = new File(getWallpaperDir(userId), fileName); + map.put(userId, result); + } + return result; + } + @Override public String toString() { StringBuilder out = new StringBuilder(defaultString(this)); @@ -177,7 +190,7 @@ class WallpaperData { out.append(", which: "); out.append(mWhich); out.append(", file mod: "); - out.append(wallpaperFile != null ? wallpaperFile.lastModified() : "null"); + out.append(getWallpaperFile() != null ? getWallpaperFile().lastModified() : "null"); if (connection == null) { out.append(", no connection"); } else { @@ -202,10 +215,10 @@ class WallpaperData { // Called during initialization of a given user's wallpaper bookkeeping boolean cropExists() { - return cropFile.exists(); + return getCropFile().exists(); } boolean sourceExists() { - return wallpaperFile.exists(); + return getWallpaperFile().exists(); } } diff --git a/services/core/java/com/android/server/wallpaper/WallpaperDataParser.java b/services/core/java/com/android/server/wallpaper/WallpaperDataParser.java index 1133dba27ace3..c54e3bdf0d516 100644 --- a/services/core/java/com/android/server/wallpaper/WallpaperDataParser.java +++ b/services/core/java/com/android/server/wallpaper/WallpaperDataParser.java @@ -542,12 +542,12 @@ class WallpaperDataParser { } res = r.openRawResource(resId); - if (wallpaper.wallpaperFile.exists()) { - wallpaper.wallpaperFile.delete(); - wallpaper.cropFile.delete(); + if (wallpaper.getWallpaperFile().exists()) { + wallpaper.getWallpaperFile().delete(); + wallpaper.getCropFile().delete(); } - fos = new FileOutputStream(wallpaper.wallpaperFile); - cos = new FileOutputStream(wallpaper.cropFile); + fos = new FileOutputStream(wallpaper.getWallpaperFile()); + cos = new FileOutputStream(wallpaper.getCropFile()); byte[] buffer = new byte[32768]; int amt; diff --git a/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java b/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java index 3182dcc5078b5..9b2cdd72a338c 100644 --- a/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java +++ b/services/core/java/com/android/server/wallpaper/WallpaperManagerService.java @@ -670,8 +670,8 @@ public class WallpaperManagerService extends IWallpaperManager.Stub // Not having a wallpaperComponent means it's a lock screen wallpaper. final boolean imageWallpaper = mImageWallpaper.equals(wallpaper.wallpaperComponent) || wallpaper.wallpaperComponent == null; - if (imageWallpaper && wallpaper.cropFile != null && wallpaper.cropFile.exists()) { - cropFile = wallpaper.cropFile.getAbsolutePath(); + if (imageWallpaper && wallpaper.getCropFile().exists()) { + cropFile = wallpaper.getCropFile().getAbsolutePath(); } else if (imageWallpaper && !wallpaper.cropExists() && !wallpaper.sourceExists()) { defaultImageWallpaper = true; } @@ -1794,8 +1794,8 @@ public class WallpaperManagerService extends IWallpaperManager.Stub private boolean clearWallpaperBitmaps(WallpaperData wallpaper) { boolean sourceExists = wallpaper.sourceExists(); boolean cropExists = wallpaper.cropExists(); - if (sourceExists) wallpaper.wallpaperFile.delete(); - if (cropExists) wallpaper.cropFile.delete(); + if (sourceExists) wallpaper.getWallpaperFile().delete(); + if (cropExists) wallpaper.getCropFile().delete(); return sourceExists || cropExists; } @@ -2394,13 +2394,13 @@ public class WallpaperManagerService extends IWallpaperManager.Stub wallpaper.callbacks.register(cb); } - File fileToReturn = getCropped ? wallpaper.cropFile : wallpaper.wallpaperFile; + File result = getCropped ? wallpaper.getCropFile() : wallpaper.getWallpaperFile(); - if (!fileToReturn.exists()) { + if (!result.exists()) { return null; } - return ParcelFileDescriptor.open(fileToReturn, MODE_READ_ONLY); + return ParcelFileDescriptor.open(result, MODE_READ_ONLY); } catch (FileNotFoundException e) { /* Shouldn't happen as we check to see if the file exists */ Slog.w(TAG, "Error getting wallpaper", e); @@ -3161,16 +3161,17 @@ public class WallpaperManagerService extends IWallpaperManager.Stub // Migrate the bitmap files outright; no need to copy try { - if (!mIsLockscreenLiveWallpaperEnabled || sysWP.wallpaperFile.exists()) { - Os.rename(sysWP.wallpaperFile.getAbsolutePath(), - lockWP.wallpaperFile.getAbsolutePath()); + if (!mIsLockscreenLiveWallpaperEnabled || sysWP.getWallpaperFile().exists()) { + Os.rename(sysWP.getWallpaperFile().getAbsolutePath(), + lockWP.getWallpaperFile().getAbsolutePath()); } - if (!mIsLockscreenLiveWallpaperEnabled || sysWP.cropFile.exists()) { - Os.rename(sysWP.cropFile.getAbsolutePath(), lockWP.cropFile.getAbsolutePath()); + if (!mIsLockscreenLiveWallpaperEnabled || sysWP.getCropFile().exists()) { + Os.rename(sysWP.getCropFile().getAbsolutePath(), + lockWP.getCropFile().getAbsolutePath()); } mLockWallpaperMap.put(userId, lockWP); if (mIsLockscreenLiveWallpaperEnabled) { - SELinux.restorecon(lockWP.wallpaperFile); + SELinux.restorecon(lockWP.getWallpaperFile()); mLastLockWallpaper = lockWP; } } catch (ErrnoException e) { @@ -3191,11 +3192,11 @@ public class WallpaperManagerService extends IWallpaperManager.Stub FileUtils.S_IRWXU|FileUtils.S_IRWXG|FileUtils.S_IXOTH, -1, -1); } - ParcelFileDescriptor fd = ParcelFileDescriptor.open(wallpaper.wallpaperFile, + ParcelFileDescriptor fd = ParcelFileDescriptor.open(wallpaper.getWallpaperFile(), MODE_CREATE|MODE_READ_WRITE|MODE_TRUNCATE); - if (!SELinux.restorecon(wallpaper.wallpaperFile)) { + if (!SELinux.restorecon(wallpaper.getWallpaperFile())) { Slog.w(TAG, "restorecon failed for wallpaper file: " + - wallpaper.wallpaperFile.getPath()); + wallpaper.getWallpaperFile().getPath()); return null; } wallpaper.name = name; @@ -3206,7 +3207,7 @@ public class WallpaperManagerService extends IWallpaperManager.Stub // Nullify field to require new computation wallpaper.primaryColors = null; Slog.v(TAG, "updateWallpaperBitmapLocked() : id=" + wallpaper.wallpaperId - + " name=" + name + " file=" + wallpaper.wallpaperFile.getName()); + + " name=" + name + " file=" + wallpaper.getWallpaperFile().getName()); return fd; } catch (FileNotFoundException e) { Slog.w(TAG, "Error setting wallpaper", e); diff --git a/services/tests/mockingservicestests/src/com/android/server/wallpaper/WallpaperManagerServiceTests.java b/services/tests/mockingservicestests/src/com/android/server/wallpaper/WallpaperManagerServiceTests.java index bc5e72095a1c5..eefe5af314a67 100644 --- a/services/tests/mockingservicestests/src/com/android/server/wallpaper/WallpaperManagerServiceTests.java +++ b/services/tests/mockingservicestests/src/com/android/server/wallpaper/WallpaperManagerServiceTests.java @@ -31,7 +31,6 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.mock; import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession; import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; import static com.android.server.wallpaper.WallpaperUtils.WALLPAPER; -import static com.android.server.wallpaper.WallpaperUtils.WALLPAPER_CROP; import static org.hamcrest.core.IsNot.not; import static org.junit.Assert.assertEquals; @@ -275,10 +274,10 @@ public class WallpaperManagerServiceTests { assertEquals(testUserId, newWallpaperData.userId); WallpaperData wallpaperData = mService.getWallpaperSafeLocked(testUserId, which); - assertEquals(wallpaperData.cropFile.getAbsolutePath(), - newWallpaperData.cropFile.getAbsolutePath()); - assertEquals(wallpaperData.wallpaperFile.getAbsolutePath(), - newWallpaperData.wallpaperFile.getAbsolutePath()); + assertEquals(wallpaperData.getCropFile().getAbsolutePath(), + newWallpaperData.getCropFile().getAbsolutePath()); + assertEquals(wallpaperData.getWallpaperFile().getAbsolutePath(), + newWallpaperData.getWallpaperFile().getAbsolutePath()); } } @@ -525,7 +524,8 @@ public class WallpaperManagerServiceTests { @Test public void getWallpaperWithFeature_getCropped_returnsCropFile() throws Exception { File cropSystemWallpaperFile = - new File(WallpaperUtils.getWallpaperDir(USER_SYSTEM), WALLPAPER_CROP); + new WallpaperData(USER_SYSTEM, FLAG_SYSTEM).getCropFile(); + cropSystemWallpaperFile.getParentFile().mkdirs(); cropSystemWallpaperFile.createNewFile(); try (FileOutputStream outputStream = new FileOutputStream(cropSystemWallpaperFile)) { outputStream.write("Crop system wallpaper".getBytes()); @@ -547,7 +547,8 @@ public class WallpaperManagerServiceTests { @Test public void getWallpaperWithFeature_notGetCropped_returnsOriginalFile() throws Exception { File originalSystemWallpaperFile = - new File(WallpaperUtils.getWallpaperDir(USER_SYSTEM), WALLPAPER); + new WallpaperData(USER_SYSTEM, FLAG_SYSTEM).getWallpaperFile(); + originalSystemWallpaperFile.getParentFile().mkdirs(); originalSystemWallpaperFile.createNewFile(); try (FileOutputStream outputStream = new FileOutputStream(originalSystemWallpaperFile)) { outputStream.write("Original system wallpaper".getBytes());