Refactor PowerManager to make SystemProperties testable

Introduce an interface through which SystemProperties are accessed.
This interface can be swapped out during tests to make testing
SystemProperty related changes easier.

Bug: 145265927
Test: atest PowerManagerServiceTest
Change-Id: Ic0a99da07c5592810376bdacdb44393f92be0422

3 files changed, 92 insertions, 26 deletions

diff --git a/services/core/java/com/android/server/power/PowerManagerService.java b/services/core/java/com/android/server/power/PowerManagerService.java
index 9cfd09563bfa..e3c889941c06 100644
--- a/services/core/java/com/android/server/power/PowerManagerService.java
+++ b/services/core/java/com/android/server/power/PowerManagerService.java
@@ -203,8 +203,11 @@ public final class PowerManagerService extends SystemService
     // System Property indicating that retail demo mode is currently enabled.
     private static final String SYSTEM_PROPERTY_RETAIL_DEMO_ENABLED = "sys.retaildemo.enabled";
 
-    // Possible reasons for shutting down or reboot for use in REBOOT_PROPERTY(sys.boot.reason)
-    // which is set by bootstat
+    // System property for last reboot reason
+    private static final String SYSTEM_PROPERTY_REBOOT_REASON = "sys.boot.reason";
+
+    // Possible reasons for shutting down or reboot for use in
+    // SYSTEM_PROPERTY_REBOOT_REASON(sys.boot.reason) which is set by bootstat
     private static final String REASON_SHUTDOWN = "shutdown";
     private static final String REASON_REBOOT = "reboot";
     private static final String REASON_USERREQUESTED = "shutdown,userrequested";
@@ -225,9 +228,6 @@ public final class PowerManagerService extends SystemService
     private static final int HALT_MODE_REBOOT = 1;
     private static final int HALT_MODE_REBOOT_SAFE_MODE = 2;
 
-    // property for last reboot reason
-    private static final String REBOOT_PROPERTY = "sys.boot.reason";
-
     private final Context mContext;
     private final ServiceThread mHandlerThread;
     private final PowerManagerHandler mHandler;
@@ -240,6 +240,7 @@ public final class PowerManagerService extends SystemService
     private final BinderService mBinderService;
     private final LocalService mLocalService;
     private final NativeWrapper mNativeWrapper;
+    private final SystemPropertiesWrapper mSystemProperties;
     private final Injector mInjector;
 
     private LightsManager mLightsManager;
@@ -756,6 +757,20 @@ public final class PowerManagerService extends SystemService
         InattentiveSleepWarningController createInattentiveSleepWarningController() {
             return new InattentiveSleepWarningController();
         }
+
+        public SystemPropertiesWrapper createSystemPropertiesWrapper() {
+            return new SystemPropertiesWrapper() {
+                @Override
+                public String get(String key, String def) {
+                    return SystemProperties.get(key, def);
+                }
+
+                @Override
+                public void set(String key, String val) {
+                    SystemProperties.set(key, val);
+                }
+            };
+        }
     }
 
     final Constants mConstants;
@@ -781,6 +796,7 @@ public final class PowerManagerService extends SystemService
         mBinderService = new BinderService();
         mLocalService = new LocalService();
         mNativeWrapper = injector.createNativeWrapper();
+        mSystemProperties = injector.createSystemPropertiesWrapper();
         mInjector = injector;
 
         mHandlerThread = new ServiceThread(TAG,
@@ -816,7 +832,7 @@ public final class PowerManagerService extends SystemService
             mHalInteractiveModeEnabled = true;
 
             mWakefulness = WAKEFULNESS_AWAKE;
-            sQuiescent = SystemProperties.get(SYSTEM_PROPERTY_QUIESCENT, "0").equals("1");
+            sQuiescent = mSystemProperties.get(SYSTEM_PROPERTY_QUIESCENT, "0").equals("1");
 
             mNativeWrapper.nativeInit(this);
             mNativeWrapper.nativeSetAutoSuspend(false);
@@ -1067,8 +1083,9 @@ public final class PowerManagerService extends SystemService
         }
 
         final String retailDemoValue = UserManager.isDeviceInDemoMode(mContext) ? "1" : "0";
-        if (!retailDemoValue.equals(SystemProperties.get(SYSTEM_PROPERTY_RETAIL_DEMO_ENABLED))) {
-            SystemProperties.set(SYSTEM_PROPERTY_RETAIL_DEMO_ENABLED, retailDemoValue);
+        if (!retailDemoValue.equals(
+                mSystemProperties.get(SYSTEM_PROPERTY_RETAIL_DEMO_ENABLED, null))) {
+            mSystemProperties.set(SYSTEM_PROPERTY_RETAIL_DEMO_ENABLED, retailDemoValue);
         }
 
         mScreenBrightnessModeSetting = Settings.System.getIntForUser(resolver,
@@ -4818,7 +4835,7 @@ public final class PowerManagerService extends SystemService
 
             final long ident = Binder.clearCallingIdentity();
             try {
-                return getLastShutdownReasonInternal(REBOOT_PROPERTY);
+                return getLastShutdownReasonInternal();
             } finally {
                 Binder.restoreCallingIdentity(ident);
             }
@@ -5052,12 +5069,8 @@ public final class PowerManagerService extends SystemService
     }
 
     @VisibleForTesting
-    // lastRebootReasonProperty argument to permit testing
-    int getLastShutdownReasonInternal(String lastRebootReasonProperty) {
-        String line = SystemProperties.get(lastRebootReasonProperty);
-        if (line == null) {
-            return PowerManager.SHUTDOWN_REASON_UNKNOWN;
-        }
+    int getLastShutdownReasonInternal() {
+        String line = mSystemProperties.get(SYSTEM_PROPERTY_REBOOT_REASON, null);
         switch (line) {
             case REASON_SHUTDOWN:
                 return PowerManager.SHUTDOWN_REASON_SHUTDOWN;
diff --git a/services/core/java/com/android/server/power/SystemPropertiesWrapper.java b/services/core/java/com/android/server/power/SystemPropertiesWrapper.java
new file mode 100644
index 000000000000..1acf798eb099
--- /dev/null
+++ b/services/core/java/com/android/server/power/SystemPropertiesWrapper.java
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.power;
+
+import android.annotation.NonNull;
+import android.annotation.Nullable;
+import android.os.SystemProperties;
+
+import com.android.internal.annotations.VisibleForTesting;
+
+/**
+ * Wrapper interface to access {@link SystemProperties}.
+ *
+ * @hide
+ */
+@VisibleForTesting
+interface SystemPropertiesWrapper {
+    /**
+     * Get the String value for the given {@code key}.
+     *
+     * @param key the key to lookup
+     * @param def the default value in case the property is not set or empty
+     * @return if the {@code key} isn't found, return {@code def} if it isn't null, or an empty
+     * string otherwise
+     */
+    @NonNull
+    String get(@NonNull String key, @Nullable String def);
+
+    /**
+     * Set the value for the given {@code key} to {@code val}.
+     *
+     * @throws IllegalArgumentException if the {@code val} exceeds 91 characters
+     * @throws RuntimeException if the property cannot be set, for example, if it was blocked by
+     * SELinux. libc will log the underlying reason.
+     */
+    void set(@NonNull String key, @Nullable String val);
+}
diff --git a/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java
index 0ca62e2dcdff..81fb0ec3bf53 100644
--- a/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/power/PowerManagerServiceTest.java
@@ -61,7 +61,6 @@ import android.os.Looper;
 import android.os.PowerManager;
 import android.os.PowerSaveState;
 import android.os.SystemClock;
-import android.os.SystemProperties;
 import android.os.UserHandle;
 import android.provider.Settings;
 import android.test.mock.MockContentResolver;
@@ -97,10 +96,12 @@ import java.util.Map;
  * Tests for {@link com.android.server.power.PowerManagerService}
  */
 public class PowerManagerServiceTest {
+    private static final String SYSTEM_PROPERTY_QUIESCENT = "ro.boot.quiescent";
+    private static final String SYSTEM_PROPERTY_REBOOT_REASON = "sys.boot.reason";
+
     private static final float PRECISION = 0.001f;
     private static final float BRIGHTNESS_FACTOR = 0.7f;
     private static final boolean BATTERY_SAVER_ENABLED = true;
-    private static final String TEST_LAST_REBOOT_PROPERTY = "test.sys.boot.reason";
 
     @Mock private BatterySaverPolicy mBatterySaverPolicyMock;
     @Mock private LightsManager mLightsManagerMock;
@@ -112,6 +113,7 @@ public class PowerManagerServiceTest {
     @Mock private Notifier mNotifierMock;
     @Mock private WirelessChargerDetector mWirelessChargerDetectorMock;
     @Mock private AmbientDisplayConfiguration mAmbientDisplayConfigurationMock;
+    @Mock private SystemPropertiesWrapper mSystemPropertiesMock;
 
     @Mock
     private InattentiveSleepWarningController mInattentiveSleepWarningControllerMock;
@@ -159,6 +161,7 @@ public class PowerManagerServiceTest {
         when(mBatteryManagerInternalMock.isPowered(anyInt())).thenReturn(false);
         when(mInattentiveSleepWarningControllerMock.isShown()).thenReturn(false);
         when(mDisplayManagerInternalMock.requestPowerState(any(), anyBoolean())).thenReturn(true);
+        when(mSystemPropertiesMock.get(eq(SYSTEM_PROPERTY_QUIESCENT), anyString())).thenReturn("");
 
         mDisplayPowerRequest = new DisplayPowerRequest();
         addLocalServiceMock(LightsManager.class, mLightsManagerMock);
@@ -218,6 +221,11 @@ public class PowerManagerServiceTest {
             InattentiveSleepWarningController createInattentiveSleepWarningController() {
                 return mInattentiveSleepWarningControllerMock;
             }
+
+            @Override
+            public SystemPropertiesWrapper createSystemPropertiesWrapper() {
+                return mSystemPropertiesMock;
+            }
         });
         return mService;
     }
@@ -228,12 +236,6 @@ public class PowerManagerServiceTest {
         LocalServices.removeServiceForTest(DisplayManagerInternal.class);
         LocalServices.removeServiceForTest(BatteryManagerInternal.class);
         LocalServices.removeServiceForTest(ActivityManagerInternal.class);
-
-        Settings.Global.putInt(
-                mContextSpy.getContentResolver(), Settings.Global.THEATER_MODE_ON, 0);
-        setAttentiveTimeout(-1);
-        Settings.Global.putInt(mContextSpy.getContentResolver(),
-                Settings.Global.STAY_ON_WHILE_PLUGGED_IN, 0);
     }
 
     /**
@@ -322,10 +324,10 @@ public class PowerManagerServiceTest {
 
     @Test
     public void testGetLastShutdownReasonInternal() {
+        when(mSystemPropertiesMock.get(eq(SYSTEM_PROPERTY_REBOOT_REASON), any())).thenReturn(
+                "shutdown,thermal");
         createService();
-        SystemProperties.set(TEST_LAST_REBOOT_PROPERTY, "shutdown,thermal");
-        int reason = mService.getLastShutdownReasonInternal(TEST_LAST_REBOOT_PROPERTY);
-        SystemProperties.set(TEST_LAST_REBOOT_PROPERTY, "");
+        int reason = mService.getLastShutdownReasonInternal();
         assertThat(reason).isEqualTo(PowerManager.SHUTDOWN_REASON_THERMAL_SHUTDOWN);
     }