Fix LetterboxConfigurationPersisterTest when file is deleted

LetterboxConfigurationPersister can use a different file name for testing. test_whenUpdatedWithNewValuesAndDeleted_valuesAreDefaults has been fixed using two different persister instances to avoid the problem of cached values. Also DEFAULT_REACHABILITY_TEST have been used as default values to be different from reachability position constants. Fixes: 290756407 Test: Run `atest WmTests:LetterboxConfigurationPersisterTest` `atest WmTests:LetterboxConfigurationTest` Change-Id: I22388e0a6168b5bce2652a1d52da49c2188dd3c4
2023-07-11 15:51:24 +00:00
parent 0e61b8a103
commit ca6a371aab
3 changed files with 83 additions and 65 deletions
--- a/services/core/java/com/android/server/wm/LetterboxConfiguration.java
+++ b/services/core/java/com/android/server/wm/LetterboxConfiguration.java
@@ -294,16 +294,15 @@ final class LetterboxConfiguration {
    @NonNull private final SynchedDeviceConfig mDeviceConfig;

    LetterboxConfiguration(@NonNull final Context systemUiContext) {
-        this(systemUiContext,
-                new LetterboxConfigurationPersister(systemUiContext,
-                        () -> readLetterboxHorizontalReachabilityPositionFromConfig(
-                                systemUiContext, /* forBookMode */ false),
-                        () -> readLetterboxVerticalReachabilityPositionFromConfig(
-                                systemUiContext, /* forTabletopMode */ false),
-                        () -> readLetterboxHorizontalReachabilityPositionFromConfig(
-                                systemUiContext, /* forBookMode */ true),
-                        () -> readLetterboxVerticalReachabilityPositionFromConfig(
-                                systemUiContext, /* forTabletopMode */ true)));
+        this(systemUiContext, new LetterboxConfigurationPersister(
+                () -> readLetterboxHorizontalReachabilityPositionFromConfig(
+                        systemUiContext, /* forBookMode */ false),
+                () -> readLetterboxVerticalReachabilityPositionFromConfig(
+                        systemUiContext, /* forTabletopMode */ false),
+                () -> readLetterboxHorizontalReachabilityPositionFromConfig(
+                        systemUiContext, /* forBookMode */ true),
+                () -> readLetterboxVerticalReachabilityPositionFromConfig(
+                        systemUiContext, /* forTabletopMode */ true)));
    }

    @VisibleForTesting
--- a/services/core/java/com/android/server/wm/LetterboxConfigurationPersister.java
+++ b/services/core/java/com/android/server/wm/LetterboxConfigurationPersister.java
@@ -23,7 +23,6 @@ import static com.android.server.wm.WindowManagerDebugConfig.TAG_WM;

 import android.annotation.NonNull;
 import android.annotation.Nullable;
-import android.content.Context;
 import android.os.Environment;
 import android.os.StrictMode;
 import android.os.StrictMode.ThreadPolicy;
@@ -53,10 +52,8 @@ class LetterboxConfigurationPersister {
    private static final String TAG =
            TAG_WITH_CLASS_NAME ? "LetterboxConfigurationPersister" : TAG_WM;

-    @VisibleForTesting
-    static final String LETTERBOX_CONFIGURATION_FILENAME = "letterbox_config";
+    private static final String LETTERBOX_CONFIGURATION_FILENAME = "letterbox_config";

-    private final Context mContext;
    private final Supplier<Integer> mDefaultHorizontalReachabilitySupplier;
    private final Supplier<Integer> mDefaultVerticalReachabilitySupplier;
    private final Supplier<Integer> mDefaultBookModeReachabilitySupplier;
@@ -97,36 +94,32 @@ class LetterboxConfigurationPersister {
    @NonNull
    private final PersisterQueue mPersisterQueue;

-    LetterboxConfigurationPersister(Context systemUiContext,
-            Supplier<Integer> defaultHorizontalReachabilitySupplier,
-            Supplier<Integer> defaultVerticalReachabilitySupplier,
-            Supplier<Integer> defaultBookModeReachabilitySupplier,
-            Supplier<Integer> defaultTabletopModeReachabilitySupplier) {
-        this(systemUiContext, defaultHorizontalReachabilitySupplier,
-                defaultVerticalReachabilitySupplier,
-                defaultBookModeReachabilitySupplier,
-                defaultTabletopModeReachabilitySupplier,
+    LetterboxConfigurationPersister(
+            @NonNull Supplier<Integer> defaultHorizontalReachabilitySupplier,
+            @NonNull Supplier<Integer> defaultVerticalReachabilitySupplier,
+            @NonNull Supplier<Integer> defaultBookModeReachabilitySupplier,
+            @NonNull Supplier<Integer> defaultTabletopModeReachabilitySupplier) {
+        this(defaultHorizontalReachabilitySupplier, defaultVerticalReachabilitySupplier,
+                defaultBookModeReachabilitySupplier, defaultTabletopModeReachabilitySupplier,
                Environment.getDataSystemDirectory(), new PersisterQueue(),
-                /* completionCallback */ null);
+                /* completionCallback */ null, LETTERBOX_CONFIGURATION_FILENAME);
    }

    @VisibleForTesting
-    LetterboxConfigurationPersister(Context systemUiContext,
-            Supplier<Integer> defaultHorizontalReachabilitySupplier,
-            Supplier<Integer> defaultVerticalReachabilitySupplier,
-            Supplier<Integer> defaultBookModeReachabilitySupplier,
-            Supplier<Integer> defaultTabletopModeReachabilitySupplier,
-            File configFolder,
-            PersisterQueue persisterQueue, @Nullable Consumer<String> completionCallback) {
-        mContext = systemUiContext.createDeviceProtectedStorageContext();
+    LetterboxConfigurationPersister(
+            @NonNull Supplier<Integer> defaultHorizontalReachabilitySupplier,
+            @NonNull Supplier<Integer> defaultVerticalReachabilitySupplier,
+            @NonNull Supplier<Integer> defaultBookModeReachabilitySupplier,
+            @NonNull Supplier<Integer> defaultTabletopModeReachabilitySupplier,
+            @NonNull File configFolder, @NonNull PersisterQueue persisterQueue,
+            @Nullable Consumer<String> completionCallback,
+            @NonNull String letterboxConfigurationFileName) {
        mDefaultHorizontalReachabilitySupplier = defaultHorizontalReachabilitySupplier;
        mDefaultVerticalReachabilitySupplier = defaultVerticalReachabilitySupplier;
-        mDefaultBookModeReachabilitySupplier =
-                defaultBookModeReachabilitySupplier;
-        mDefaultTabletopModeReachabilitySupplier =
-                defaultTabletopModeReachabilitySupplier;
+        mDefaultBookModeReachabilitySupplier = defaultBookModeReachabilitySupplier;
+        mDefaultTabletopModeReachabilitySupplier = defaultTabletopModeReachabilitySupplier;
        mCompletionCallback = completionCallback;
-        final File prefFiles = new File(configFolder, LETTERBOX_CONFIGURATION_FILENAME);
+        final File prefFiles = new File(configFolder, letterboxConfigurationFileName);
        mConfigurationFile = new AtomicFile(prefFiles);
        mPersisterQueue = persisterQueue;
        runWithDiskReadsThreadPolicy(this::readCurrentConfiguration);
--- a/services/tests/wmtests/src/com/android/server/wm/LetterboxConfigurationPersisterTest.java
+++ b/services/tests/wmtests/src/com/android/server/wm/LetterboxConfigurationPersisterTest.java
@@ -20,7 +20,6 @@ import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentat

 import static com.android.server.wm.LetterboxConfiguration.LETTERBOX_HORIZONTAL_REACHABILITY_POSITION_LEFT;
 import static com.android.server.wm.LetterboxConfiguration.LETTERBOX_VERTICAL_REACHABILITY_POSITION_TOP;
-import static com.android.server.wm.LetterboxConfigurationPersister.LETTERBOX_CONFIGURATION_FILENAME;

 import android.annotation.NonNull;
 import android.annotation.Nullable;
@@ -42,13 +41,26 @@ import org.junit.Test;
 import java.io.File;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Consumer;
+import java.util.function.Supplier;

+/**
+ * Tests for the {@link LetterboxConfigurationPersister} class.
+ *
+ * Build/Install/Run:
+ *  atest WmTests:LetterboxConfigurationPersisterTest
+ */
@SmallTest
@Presubmit
 public class LetterboxConfigurationPersisterTest {

    private static final long TIMEOUT = 2000L; // 2 secs

+    private static final int DEFAULT_REACHABILITY_TEST = -1;
+    private static final Supplier<Integer> DEFAULT_REACHABILITY_SUPPLIER_TEST =
+            () -> DEFAULT_REACHABILITY_TEST;
+
+    private static final String LETTERBOX_CONFIGURATION_TEST_FILENAME = "letterbox_config_test";
+
    private LetterboxConfigurationPersister mLetterboxConfigurationPersister;
    private Context mContext;
    private PersisterQueue mPersisterQueue;
@@ -62,7 +74,7 @@ public class LetterboxConfigurationPersisterTest {
        mConfigFolder = mContext.getFilesDir();
        mPersisterQueue = new PersisterQueue();
        mQueueState = new QueueState();
-        mLetterboxConfigurationPersister = new LetterboxConfigurationPersister(mContext,
+        mLetterboxConfigurationPersister = new LetterboxConfigurationPersister(
                () -> mContext.getResources().getInteger(
                        R.integer.config_letterboxDefaultPositionForHorizontalReachability),
                () -> mContext.getResources().getInteger(
@@ -72,7 +84,8 @@ public class LetterboxConfigurationPersisterTest {
                () -> mContext.getResources().getInteger(
                        R.integer.config_letterboxDefaultPositionForTabletopModeReachability
                ),
-                mConfigFolder, mPersisterQueue, mQueueState);
+                mConfigFolder, mPersisterQueue, mQueueState,
+                LETTERBOX_CONFIGURATION_TEST_FILENAME);
        mQueueListener = queueEmpty -> mQueueState.onItemAdded();
        mPersisterQueue.addListener(mQueueListener);
        mLetterboxConfigurationPersister.start();
@@ -127,8 +140,10 @@ public class LetterboxConfigurationPersisterTest {
    public void test_whenUpdatedWithNewValues_valuesAreReadAfterRestart() {
        final PersisterQueue firstPersisterQueue = new PersisterQueue();
        final LetterboxConfigurationPersister firstPersister = new LetterboxConfigurationPersister(
-                mContext, () -> -1, () -> -1, () -> -1, () -> -1, mContext.getFilesDir(),
-                firstPersisterQueue, mQueueState);
+                DEFAULT_REACHABILITY_SUPPLIER_TEST, DEFAULT_REACHABILITY_SUPPLIER_TEST,
+                DEFAULT_REACHABILITY_SUPPLIER_TEST, DEFAULT_REACHABILITY_SUPPLIER_TEST,
+                mContext.getFilesDir(), firstPersisterQueue, mQueueState,
+                LETTERBOX_CONFIGURATION_TEST_FILENAME);
        firstPersister.start();
        firstPersister.setLetterboxPositionForHorizontalReachability(false,
                LETTERBOX_HORIZONTAL_REACHABILITY_POSITION_LEFT);
@@ -138,8 +153,10 @@ public class LetterboxConfigurationPersisterTest {
        stopPersisterSafe(firstPersisterQueue);
        final PersisterQueue secondPersisterQueue = new PersisterQueue();
        final LetterboxConfigurationPersister secondPersister = new LetterboxConfigurationPersister(
-                mContext, () -> -1, () -> -1, () -> -1, () -> -1, mContext.getFilesDir(),
-                secondPersisterQueue, mQueueState);
+                DEFAULT_REACHABILITY_SUPPLIER_TEST, DEFAULT_REACHABILITY_SUPPLIER_TEST,
+                DEFAULT_REACHABILITY_SUPPLIER_TEST, DEFAULT_REACHABILITY_SUPPLIER_TEST,
+                mContext.getFilesDir(), secondPersisterQueue, mQueueState,
+                LETTERBOX_CONFIGURATION_TEST_FILENAME);
        secondPersister.start();
        final int newPositionForHorizontalReachability =
                secondPersister.getLetterboxPositionForHorizontalReachability(false);
@@ -156,37 +173,46 @@ public class LetterboxConfigurationPersisterTest {

    @Test
    public void test_whenUpdatedWithNewValuesAndDeleted_valuesAreDefaults() {
-        mLetterboxConfigurationPersister.setLetterboxPositionForHorizontalReachability(false,
+        final PersisterQueue firstPersisterQueue = new PersisterQueue();
+        final LetterboxConfigurationPersister firstPersister = new LetterboxConfigurationPersister(
+                DEFAULT_REACHABILITY_SUPPLIER_TEST, DEFAULT_REACHABILITY_SUPPLIER_TEST,
+                DEFAULT_REACHABILITY_SUPPLIER_TEST, DEFAULT_REACHABILITY_SUPPLIER_TEST,
+                mContext.getFilesDir(), firstPersisterQueue, mQueueState,
+                LETTERBOX_CONFIGURATION_TEST_FILENAME);
+        firstPersister.start();
+        firstPersister.setLetterboxPositionForHorizontalReachability(false,
                LETTERBOX_HORIZONTAL_REACHABILITY_POSITION_LEFT);
-        mLetterboxConfigurationPersister.setLetterboxPositionForVerticalReachability(false,
+        firstPersister.setLetterboxPositionForVerticalReachability(false,
                LETTERBOX_VERTICAL_REACHABILITY_POSITION_TOP);
        waitForCompletion(mPersisterQueue);
        final int newPositionForHorizontalReachability =
-                mLetterboxConfigurationPersister.getLetterboxPositionForHorizontalReachability(
-                        false);
+                firstPersister.getLetterboxPositionForHorizontalReachability(false);
        final int newPositionForVerticalReachability =
-                mLetterboxConfigurationPersister.getLetterboxPositionForVerticalReachability(false);
+                firstPersister.getLetterboxPositionForVerticalReachability(false);
        Assert.assertEquals(LETTERBOX_HORIZONTAL_REACHABILITY_POSITION_LEFT,
                newPositionForHorizontalReachability);
        Assert.assertEquals(LETTERBOX_VERTICAL_REACHABILITY_POSITION_TOP,
                newPositionForVerticalReachability);
-        deleteConfiguration(mLetterboxConfigurationPersister, mPersisterQueue);
-        waitForCompletion(mPersisterQueue);
+        deleteConfiguration(firstPersister, firstPersisterQueue);
+        waitForCompletion(firstPersisterQueue);
+        stopPersisterSafe(firstPersisterQueue);
+
+        final PersisterQueue secondPersisterQueue = new PersisterQueue();
+        final LetterboxConfigurationPersister secondPersister = new LetterboxConfigurationPersister(
+                DEFAULT_REACHABILITY_SUPPLIER_TEST, DEFAULT_REACHABILITY_SUPPLIER_TEST,
+                DEFAULT_REACHABILITY_SUPPLIER_TEST, DEFAULT_REACHABILITY_SUPPLIER_TEST,
+                mContext.getFilesDir(), secondPersisterQueue, mQueueState,
+                LETTERBOX_CONFIGURATION_TEST_FILENAME);
+        secondPersister.start();
        final int positionForHorizontalReachability =
-                mLetterboxConfigurationPersister.getLetterboxPositionForHorizontalReachability(
-                        false);
-        final int defaultPositionForHorizontalReachability =
-                mContext.getResources().getInteger(
-                        R.integer.config_letterboxDefaultPositionForHorizontalReachability);
-        Assert.assertEquals(defaultPositionForHorizontalReachability,
-                positionForHorizontalReachability);
+                secondPersister.getLetterboxPositionForHorizontalReachability(false);
        final int positionForVerticalReachability =
-                mLetterboxConfigurationPersister.getLetterboxPositionForVerticalReachability(false);
-        final int defaultPositionForVerticalReachability =
-                mContext.getResources().getInteger(
-                        R.integer.config_letterboxDefaultPositionForVerticalReachability);
-        Assert.assertEquals(defaultPositionForVerticalReachability,
-                positionForVerticalReachability);
+                secondPersister.getLetterboxPositionForVerticalReachability(false);
+        Assert.assertEquals(DEFAULT_REACHABILITY_TEST, positionForHorizontalReachability);
+        Assert.assertEquals(DEFAULT_REACHABILITY_TEST, positionForVerticalReachability);
+        deleteConfiguration(secondPersister, secondPersisterQueue);
+        waitForCompletion(secondPersisterQueue);
+        stopPersisterSafe(secondPersisterQueue);
    }

    private void stopPersisterSafe(PersisterQueue persisterQueue) {
@@ -222,7 +248,7 @@ public class LetterboxConfigurationPersisterTest {
    private void deleteConfiguration(LetterboxConfigurationPersister persister,
            PersisterQueue persisterQueue) {
        final AtomicFile fileToDelete = new AtomicFile(
-                new File(mConfigFolder, LETTERBOX_CONFIGURATION_FILENAME));
+                new File(mConfigFolder, LETTERBOX_CONFIGURATION_TEST_FILENAME));
        persisterQueue.addItem(
                new DeleteFileCommand(fileToDelete, mQueueState.andThen(
                        s -> persister.useDefaultValue())), true);