Rework battery saver synchronization logic

- Stop using individual lock instances and use use the power manager lock.
- Make sure not to access external components with the lock held,
*except for settings provider*, which is already touched by the power manager
and the battery service

(I'm planning to re-organize things for Q: b/79580230)

Fixes: 79486713

Test: atest /android/pi-dev/frameworks/base/services/tests/servicestests/src/com/android/server/power/batterysaver/Battery*Test.java /android/pi-dev/frameworks/base/services/tests/servicestests/src/com/android/server/power/*Test.java
Test: Manual test with ./development/scripts/battery_simulator.py with toggling
battery saver on and off and then watch
adb logcat -b all | grep -P '(battery_saving_stats|battery_saver_mode)'

Change-Id: I8ecdfefe978de348ee979d765c82e3d8f0280082

9 files changed, 145 insertions, 113 deletions

diff --git a/core/java/com/android/internal/util/ConcurrentUtils.java b/core/java/com/android/internal/util/ConcurrentUtils.java
index e08eb587ab97..8023500b00e4 100644
--- a/core/java/com/android/internal/util/ConcurrentUtils.java
+++ b/core/java/com/android/internal/util/ConcurrentUtils.java
@@ -17,6 +17,7 @@
 package com.android.internal.util;
 
 import android.os.Process;
+import android.util.Slog;
 
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
@@ -111,4 +112,22 @@ public class ConcurrentUtils {
             throw new IllegalStateException(description + " interrupted.");
         }
     }
+
+    /**
+     * Calls {@link Slog#wtf} if a given lock is held.
+     */
+    public static void wtfIfLockHeld(String tag, Object lock) {
+        if (Thread.holdsLock(lock)) {
+            Slog.wtf(tag, "Lock mustn't be held");
+        }
+    }
+
+    /**
+     * Calls {@link Slog#wtf} if a given lock is not held.
+     */
+    public static void wtfIfLockNotHeld(String tag, Object lock) {
+        if (!Thread.holdsLock(lock)) {
+            Slog.wtf(tag, "Lock must be held");
+        }
+    }
 }
diff --git a/services/core/java/com/android/server/power/BatterySaverPolicy.java b/services/core/java/com/android/server/power/BatterySaverPolicy.java
index c04c1fb4f786..91ef3d4a576e 100644
--- a/services/core/java/com/android/server/power/BatterySaverPolicy.java
+++ b/services/core/java/com/android/server/power/BatterySaverPolicy.java
@@ -34,6 +34,8 @@ import android.view.accessibility.AccessibilityManager;
 import com.android.internal.R;
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.os.BackgroundThread;
+import com.android.internal.util.ConcurrentUtils;
 import com.android.server.power.batterysaver.BatterySavingStats;
 import com.android.server.power.batterysaver.CpuFrequencies;
 
@@ -44,6 +46,9 @@ import java.util.List;
 /**
  * Class to decide whether to turn on battery saver mode for specific service
  *
+ * IMPORTANT: This class shares the power manager lock, which is very low in the lock hierarchy.
+ * Do not call out with the lock held, such as AccessibilityManager. (Settings provider is okay.)
+ *
  * Test:
  atest ${ANDROID_BUILD_TOP}/frameworks/base/services/tests/servicestests/src/com/android/server/power/BatterySaverPolicyTest.java
  */
@@ -75,7 +80,8 @@ public class BatterySaverPolicy extends ContentObserver {
     private static final String KEY_CPU_FREQ_INTERACTIVE = "cpufreq-i";
     private static final String KEY_CPU_FREQ_NONINTERACTIVE = "cpufreq-n";
 
-    private final Object mLock = new Object();
+    private final Object mLock;
+    private final Handler mHandler;
 
     @GuardedBy("mLock")
     private String mSettings;
@@ -227,14 +233,9 @@ public class BatterySaverPolicy extends ContentObserver {
     @GuardedBy("mLock")
     private boolean mSendTronLog;
 
-    @GuardedBy("mLock")
-    private Context mContext;
-
-    @GuardedBy("mLock")
-    private ContentResolver mContentResolver;
-
-    @GuardedBy("mLock")
-    private AccessibilityManager mAccessibilityManager;
+    private final Context mContext;
+    private final ContentResolver mContentResolver;
+    private final BatterySavingStats mBatterySavingStats;
 
     @GuardedBy("mLock")
     private final List<BatterySaverPolicyListener> mListeners = new ArrayList<>();
@@ -267,25 +268,37 @@ public class BatterySaverPolicy extends ContentObserver {
         void onBatterySaverPolicyChanged(BatterySaverPolicy policy);
     }
 
-    public BatterySaverPolicy(Handler handler) {
-        super(handler);
+    public BatterySaverPolicy(Object lock, Context context, BatterySavingStats batterySavingStats) {
+        super(BackgroundThread.getHandler());
+        mLock = lock;
+        mHandler = BackgroundThread.getHandler();
+        mContext = context;
+        mContentResolver = context.getContentResolver();
+        mBatterySavingStats = batterySavingStats;
     }
 
-    public void systemReady(Context context) {
-        synchronized (mLock) {
-            mContext = context;
-            mContentResolver = context.getContentResolver();
+    /**
+     * Called by {@link PowerManagerService#systemReady}, *with no lock held.*
+     */
+    public void systemReady() {
+        ConcurrentUtils.wtfIfLockHeld(TAG, mLock);
 
-            mAccessibilityManager = context.getSystemService(AccessibilityManager.class);
+        mContentResolver.registerContentObserver(Settings.Global.getUriFor(
+                Settings.Global.BATTERY_SAVER_CONSTANTS), false, this);
+        mContentResolver.registerContentObserver(Settings.Global.getUriFor(
+                Global.BATTERY_SAVER_DEVICE_SPECIFIC_CONSTANTS), false, this);
 
-            mContentResolver.registerContentObserver(Settings.Global.getUriFor(
-                    Settings.Global.BATTERY_SAVER_CONSTANTS), false, this);
-            mContentResolver.registerContentObserver(Settings.Global.getUriFor(
-                    Global.BATTERY_SAVER_DEVICE_SPECIFIC_CONSTANTS), false, this);
+        final AccessibilityManager acm = mContext.getSystemService(AccessibilityManager.class);
 
-            mAccessibilityManager.addAccessibilityStateChangeListener((enabled) -> {
-                refreshSettings();
-            });
+        acm.addAccessibilityStateChangeListener((enabled) -> {
+            synchronized (mLock) {
+                mAccessibilityEnabled = enabled;
+            }
+            refreshSettings();
+        });
+        final boolean enabled = acm.isEnabled();
+        synchronized (mLock) {
+            mAccessibilityEnabled = enabled;
         }
         onChange(true, null);
     }
@@ -298,11 +311,7 @@ public class BatterySaverPolicy extends ContentObserver {
 
     @VisibleForTesting
     String getGlobalSetting(String key) {
-        final ContentResolver cr;
-        synchronized (mLock) {
-            cr = mContentResolver;
-        }
-        return Settings.Global.getString(cr, key);
+        return Settings.Global.getString(mContentResolver, key);
     }
 
     @VisibleForTesting
@@ -310,11 +319,6 @@ public class BatterySaverPolicy extends ContentObserver {
         return R.string.config_batterySaverDeviceSpecificConfig;
     }
 
-    @VisibleForTesting
-    boolean isAccessibilityEnabled() {
-        return mAccessibilityManager.isEnabled();
-    }
-
     @Override
     public void onChange(boolean selfChange, Uri uri) {
         refreshSettings();
@@ -347,9 +351,11 @@ public class BatterySaverPolicy extends ContentObserver {
         }
 
         // Notify the listeners.
-        for (BatterySaverPolicyListener listener : listeners) {
-            listener.onBatterySaverPolicyChanged(this);
-        }
+        mHandler.post(() -> {
+            for (BatterySaverPolicyListener listener : listeners) {
+                listener.onBatterySaverPolicyChanged(this);
+            }
+        });
     }
 
     @GuardedBy("mLock")
@@ -408,8 +414,6 @@ public class BatterySaverPolicy extends ContentObserver {
                 parser.getString(KEY_CPU_FREQ_NONINTERACTIVE, "")).toSysFileMap();
 
         // Update the effective policy.
-        mAccessibilityEnabled = isAccessibilityEnabled();
-
         mVibrationDisabledEffective = mVibrationDisabledConfig
                 && !mAccessibilityEnabled; // Don't disable vibration when accessibility is on.
 
@@ -436,7 +440,7 @@ public class BatterySaverPolicy extends ContentObserver {
 
         mEventLogKeys = sb.toString();
 
-        BatterySavingStats.getInstance().setSendTronLog(mSendTronLog);
+        mBatterySavingStats.setSendTronLog(mSendTronLog);
     }
 
     /**
@@ -532,7 +536,7 @@ public class BatterySaverPolicy extends ContentObserver {
     public void dump(PrintWriter pw) {
         synchronized (mLock) {
             pw.println();
-            BatterySavingStats.getInstance().dump(pw, "");
+            mBatterySavingStats.dump(pw, "");
 
             pw.println();
             pw.println("Battery saver policy (*NOTE* they only apply when battery saver is ON):");
@@ -583,4 +587,11 @@ public class BatterySaverPolicy extends ContentObserver {
             pw.println("'");
         }
     }
+
+    @VisibleForTesting
+    public void setAccessibilityEnabledForTest(boolean enabled) {
+        synchronized (mLock) {
+            mAccessibilityEnabled = enabled;
+        }
+    }
 }
diff --git a/services/core/java/com/android/server/power/PowerManagerService.java b/services/core/java/com/android/server/power/PowerManagerService.java
index 2ffc4e77eec5..697801f7787a 100644
--- a/services/core/java/com/android/server/power/PowerManagerService.java
+++ b/services/core/java/com/android/server/power/PowerManagerService.java
@@ -99,6 +99,7 @@ import com.android.server.lights.LightsManager;
 import com.android.server.policy.WindowManagerPolicy;
 import com.android.server.power.batterysaver.BatterySaverController;
 import com.android.server.power.batterysaver.BatterySaverStateMachine;
+import com.android.server.power.batterysaver.BatterySavingStats;
 
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
@@ -228,6 +229,7 @@ public final class PowerManagerService extends SystemService
     private final BatterySaverPolicy mBatterySaverPolicy;
     private final BatterySaverController mBatterySaverController;
     private final BatterySaverStateMachine mBatterySaverStateMachine;
+    private final BatterySavingStats mBatterySavingStats;
 
     private LightsManager mLightsManager;
     private BatteryManagerInternal mBatteryManagerInternal;
@@ -655,10 +657,12 @@ public final class PowerManagerService extends SystemService
         mConstants = new Constants(mHandler);
         mAmbientDisplayConfiguration = new AmbientDisplayConfiguration(mContext);
 
-        mBatterySaverPolicy = new BatterySaverPolicy(mHandler);
-        mBatterySaverController = new BatterySaverController(mContext,
-                BackgroundThread.get().getLooper(), mBatterySaverPolicy);
-        mBatterySaverStateMachine = new BatterySaverStateMachine(mContext, mBatterySaverController);
+        mBatterySavingStats = new BatterySavingStats(mLock);
+        mBatterySaverPolicy = new BatterySaverPolicy(mLock, mContext, mBatterySavingStats);
+        mBatterySaverController = new BatterySaverController(mLock, mContext,
+                BackgroundThread.get().getLooper(), mBatterySaverPolicy, mBatterySavingStats);
+        mBatterySaverStateMachine = new BatterySaverStateMachine(
+                mLock, mContext, mBatterySaverController);
 
         synchronized (mLock) {
             mWakeLockSuspendBlocker = createSuspendBlockerLocked("PowerManagerService.WakeLocks");
@@ -693,10 +697,12 @@ public final class PowerManagerService extends SystemService
         mDisplaySuspendBlocker = null;
         mWakeLockSuspendBlocker = null;
 
+        mBatterySavingStats = new BatterySavingStats(mLock);
         mBatterySaverPolicy = batterySaverPolicy;
-        mBatterySaverController = new BatterySaverController(context,
-                BackgroundThread.getHandler().getLooper(), batterySaverPolicy);
-        mBatterySaverStateMachine = new BatterySaverStateMachine(mContext, mBatterySaverController);
+        mBatterySaverController = new BatterySaverController(mLock, context,
+                BackgroundThread.getHandler().getLooper(), batterySaverPolicy, mBatterySavingStats);
+        mBatterySaverStateMachine = new BatterySaverStateMachine(
+                mLock, mContext, mBatterySaverController);
     }
 
     @Override
@@ -787,7 +793,7 @@ public final class PowerManagerService extends SystemService
         mConstants.start(resolver);
 
         mBatterySaverController.systemReady();
-        mBatterySaverPolicy.systemReady(mContext);
+        mBatterySaverPolicy.systemReady();
 
         // Register for settings changes.
         resolver.registerContentObserver(Settings.Secure.getUriFor(
@@ -2679,10 +2685,6 @@ public final class PowerManagerService extends SystemService
         }
     }
 
-    private boolean isLowPowerModeInternal() {
-        return mBatterySaverController.isEnabled();
-    }
-
     private boolean setLowPowerModeInternal(boolean enabled) {
         synchronized (mLock) {
             if (DEBUG) {
@@ -4334,7 +4336,7 @@ public final class PowerManagerService extends SystemService
         public boolean isPowerSaveMode() {
             final long ident = Binder.clearCallingIdentity();
             try {
-                return isLowPowerModeInternal();
+                return mBatterySaverController.isEnabled();
             } finally {
                 Binder.restoreCallingIdentity(ident);
             }
@@ -4344,10 +4346,8 @@ public final class PowerManagerService extends SystemService
         public PowerSaveState getPowerSaveState(@ServiceType int serviceType) {
             final long ident = Binder.clearCallingIdentity();
             try {
-                synchronized (mLock) {
-                    return mBatterySaverPolicy.getBatterySaverPolicy(
-                            serviceType, isLowPowerModeInternal());
-                }
+                return mBatterySaverPolicy.getBatterySaverPolicy(
+                        serviceType, mBatterySaverController.isEnabled());
             } finally {
                 Binder.restoreCallingIdentity(ident);
             }
@@ -4673,10 +4673,8 @@ public final class PowerManagerService extends SystemService
 
         @Override
         public PowerSaveState getLowPowerState(@ServiceType int serviceType) {
-            synchronized (mLock) {
-                return mBatterySaverPolicy.getBatterySaverPolicy(serviceType,
-                        mBatterySaverController.isEnabled());
-            }
+            return mBatterySaverPolicy.getBatterySaverPolicy(serviceType,
+                    mBatterySaverController.isEnabled());
         }
 
         @Override
diff --git a/services/core/java/com/android/server/power/batterysaver/BatterySaverController.java b/services/core/java/com/android/server/power/batterysaver/BatterySaverController.java
index cb84cf3fa72b..be24c7125969 100644
--- a/services/core/java/com/android/server/power/batterysaver/BatterySaverController.java
+++ b/services/core/java/com/android/server/power/batterysaver/BatterySaverController.java
@@ -50,13 +50,16 @@ import java.util.ArrayList;
 
 /**
  * Responsible for battery saver mode transition logic.
+ *
+ * IMPORTANT: This class shares the power manager lock, which is very low in the lock hierarchy.
+ * Do not call out with the lock held. (Settings provider is okay.)
  */
 public class BatterySaverController implements BatterySaverPolicyListener {
     static final String TAG = "BatterySaverController";
 
     static final boolean DEBUG = BatterySaverPolicy.DEBUG;
 
-    private final Object mLock = new Object();
+    private final Object mLock;
     private final Context mContext;
     private final MyHandler mHandler;
     private final FileUpdater mFileUpdater;
@@ -142,13 +145,15 @@ public class BatterySaverController implements BatterySaverPolicyListener {
     /**
      * Constructor.
      */
-    public BatterySaverController(Context context, Looper looper, BatterySaverPolicy policy) {
+    public BatterySaverController(Object lock, Context context, Looper looper,
+            BatterySaverPolicy policy, BatterySavingStats batterySavingStats) {
+        mLock = lock;
         mContext = context;
         mHandler = new MyHandler(looper);
         mBatterySaverPolicy = policy;
         mBatterySaverPolicy.addListener(this);
         mFileUpdater = new FileUpdater(context);
-        mBatterySavingStats = BatterySavingStats.getInstance();
+        mBatterySavingStats = batterySavingStats;
 
         // Initialize plugins.
         final ArrayList<Plugin> plugins = new ArrayList<>();
@@ -167,7 +172,7 @@ public class BatterySaverController implements BatterySaverPolicyListener {
     }
 
     /**
-     * Called by {@link PowerManagerService} on system ready.
+     * Called by {@link PowerManagerService} on system ready, *with no lock held*.
      */
     public void systemReady() {
         final IntentFilter filter = new IntentFilter(Intent.ACTION_SCREEN_ON);
diff --git a/services/core/java/com/android/server/power/batterysaver/BatterySaverStateMachine.java b/services/core/java/com/android/server/power/batterysaver/BatterySaverStateMachine.java
index b9f31b126f0b..06559df98351 100644
--- a/services/core/java/com/android/server/power/batterysaver/BatterySaverStateMachine.java
+++ b/services/core/java/com/android/server/power/batterysaver/BatterySaverStateMachine.java
@@ -37,14 +37,15 @@ import java.io.PrintWriter;
 /**
  * Decides when to enable / disable battery saver.
  *
- * (n.b. This isn't really implemented as a "state machine" though.)
+ * IMPORTANT: This class shares the power manager lock, which is very low in the lock hierarchy.
+ * Do not call out with the lock held. (Settings provider is okay.)
  *
  * Test:
   atest $ANDROID_BUILD_TOP/frameworks/base/services/tests/servicestests/src/com/android/server/power/batterysaver/BatterySaverStateMachineTest.java
  */
 public class BatterySaverStateMachine {
     private static final String TAG = "BatterySaverStateMachine";
-    private final Object mLock = new Object();
+    private final Object mLock;
 
     private static final boolean DEBUG = BatterySaverPolicy.DEBUG;
 
@@ -118,8 +119,9 @@ public class BatterySaverStateMachine {
         }
     };
 
-    public BatterySaverStateMachine(
+    public BatterySaverStateMachine(Object lock,
             Context context, BatterySaverController batterySaverController) {
+        mLock = lock;
         mContext = context;
         mBatterySaverController = batterySaverController;
     }
@@ -141,18 +143,18 @@ public class BatterySaverStateMachine {
         }
         // This is called with the power manager lock held. Don't do any
         runOnBgThread(() -> {
-            synchronized (mLock) {
+            final ContentResolver cr = mContext.getContentResolver();
+            cr.registerContentObserver(Settings.Global.getUriFor(
+                    Settings.Global.LOW_POWER_MODE),
+                    false, mSettingsObserver, UserHandle.USER_SYSTEM);
+            cr.registerContentObserver(Settings.Global.getUriFor(
+                    Settings.Global.LOW_POWER_MODE_STICKY),
+                    false, mSettingsObserver, UserHandle.USER_SYSTEM);
+            cr.registerContentObserver(Settings.Global.getUriFor(
+                    Settings.Global.LOW_POWER_MODE_TRIGGER_LEVEL),
+                    false, mSettingsObserver, UserHandle.USER_SYSTEM);
 
-                final ContentResolver cr = mContext.getContentResolver();
-                cr.registerContentObserver(Settings.Global.getUriFor(
-                        Settings.Global.LOW_POWER_MODE),
-                        false, mSettingsObserver, UserHandle.USER_SYSTEM);
-                cr.registerContentObserver(Settings.Global.getUriFor(
-                        Settings.Global.LOW_POWER_MODE_STICKY),
-                        false, mSettingsObserver, UserHandle.USER_SYSTEM);
-                cr.registerContentObserver(Settings.Global.getUriFor(
-                        Settings.Global.LOW_POWER_MODE_TRIGGER_LEVEL),
-                        false, mSettingsObserver, UserHandle.USER_SYSTEM);
+            synchronized (mLock) {
 
                 mBootCompleted = true;
 
@@ -183,8 +185,6 @@ public class BatterySaverStateMachine {
     }
 
     void refreshSettingsLocked() {
-        final ContentResolver cr = mContext.getContentResolver();
-
         final boolean lowPowerModeEnabled = getGlobalSetting(
                 Settings.Global.LOW_POWER_MODE, 0) != 0;
         final boolean lowPowerModeEnabledSticky = getGlobalSetting(
diff --git a/services/core/java/com/android/server/power/batterysaver/BatterySavingStats.java b/services/core/java/com/android/server/power/batterysaver/BatterySavingStats.java
index 05549e70cd1c..f53a5dc6ac16 100644
--- a/services/core/java/com/android/server/power/batterysaver/BatterySavingStats.java
+++ b/services/core/java/com/android/server/power/batterysaver/BatterySavingStats.java
@@ -38,6 +38,9 @@ import java.util.Date;
 /**
  * This class keeps track of battery drain rate.
  *
+ * IMPORTANT: This class shares the power manager lock, which is very low in the lock hierarchy.
+ * Do not call out with the lock held. (Settings provider is okay.)
+ *
  * TODO: The use of the terms "percent" and "level" in this class is not standard. Fix it.
  *
  * Test:
@@ -49,7 +52,7 @@ public class BatterySavingStats {
 
     private static final boolean DEBUG = BatterySaverPolicy.DEBUG;
 
-    private final Object mLock = new Object();
+    private final Object mLock;
 
     /** Whether battery saver is on or off. */
     interface BatterySaverState {
@@ -138,8 +141,6 @@ public class BatterySavingStats {
         }
     }
 
-    private static BatterySavingStats sInstance;
-
     private BatteryManagerInternal mBatteryManagerInternal;
     private final MetricsLogger mMetricsLogger;
 
@@ -177,21 +178,16 @@ public class BatterySavingStats {
     @GuardedBy("mLock")
     private boolean mSendTronLog;
 
-    /**
-     * Don't call it directly -- use {@link #getInstance()}. Not private for testing.
-     * @param metricsLogger
-     */
+    /** Visible for unit tests */
     @VisibleForTesting
-    BatterySavingStats(MetricsLogger metricsLogger) {
+    public BatterySavingStats(Object lock, MetricsLogger metricsLogger) {
+        mLock = lock;
         mBatteryManagerInternal = LocalServices.getService(BatteryManagerInternal.class);
         mMetricsLogger = metricsLogger;
     }
 
-    public static synchronized BatterySavingStats getInstance() {
-        if (sInstance == null) {
-            sInstance = new BatterySavingStats(new MetricsLogger());
-        }
-        return sInstance;
+    public BatterySavingStats(Object lock) {
+        this(lock, new MetricsLogger());
     }
 
     public void setSendTronLog(boolean send) {
diff --git a/services/tests/servicestests/src/com/android/server/power/BatterySaverPolicyTest.java b/services/tests/servicestests/src/com/android/server/power/BatterySaverPolicyTest.java
index 761c1f1ea3a5..54ac6fc7db93 100644
--- a/services/tests/servicestests/src/com/android/server/power/BatterySaverPolicyTest.java
+++ b/services/tests/servicestests/src/com/android/server/power/BatterySaverPolicyTest.java
@@ -15,10 +15,15 @@
  */
 package com.android.server.power;
 
+import static com.google.common.truth.Truth.assertThat;
+
+import static org.mockito.Mockito.mock;
+
+import android.content.Context;
+import android.os.Handler;
 import android.os.PowerManager;
 import android.os.PowerManager.ServiceType;
 import android.os.PowerSaveState;
-import android.os.Handler;
 import android.provider.Settings.Global;
 import android.test.AndroidTestCase;
 import android.test.suitebuilder.annotation.SmallTest;
@@ -26,12 +31,12 @@ import android.util.ArrayMap;
 
 import com.android.frameworks.servicestests.R;
 import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.logging.MetricsLogger;
+import com.android.server.power.batterysaver.BatterySavingStats;
 
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
-import static com.google.common.truth.Truth.assertThat;
-
 /**
  * Tests for {@link com.android.server.power.BatterySaverPolicy}
  */
@@ -57,8 +62,9 @@ public class BatterySaverPolicyTest extends AndroidTestCase {
     private static final String BATTERY_SAVER_INCORRECT_CONSTANTS = "vi*,!=,,true";
 
     private class BatterySaverPolicyForTest extends BatterySaverPolicy {
-        public BatterySaverPolicyForTest(Handler handler) {
-            super(handler);
+        public BatterySaverPolicyForTest(Object lock, Context context,
+                BatterySavingStats batterySavingStats) {
+            super(lock, context, batterySavingStats);
         }
 
         @Override
@@ -71,11 +77,6 @@ public class BatterySaverPolicyTest extends AndroidTestCase {
             return mDeviceSpecificConfigResId;
         }
 
-        @Override
-        boolean isAccessibilityEnabled() {
-            return mMockAccessibilityEnabled;
-        }
-
         @VisibleForTesting
         void onChange() {
             onChange(true, null);
@@ -84,20 +85,22 @@ public class BatterySaverPolicyTest extends AndroidTestCase {
 
     @Mock
     Handler mHandler;
+
+    @Mock
+    MetricsLogger mMetricsLogger = mock(MetricsLogger.class);
+
     private BatterySaverPolicyForTest mBatterySaverPolicy;
 
     private final ArrayMap<String, String> mMockGlobalSettings = new ArrayMap<>();
     private int mDeviceSpecificConfigResId = R.string.config_batterySaverDeviceSpecificConfig_1;
 
-    private boolean mMockAccessibilityEnabled;
-
     public void setUp() throws Exception {
         super.setUp();
         MockitoAnnotations.initMocks(this);
-        mBatterySaverPolicy = new BatterySaverPolicyForTest(mHandler);
-        mBatterySaverPolicy.systemReady(getContext());
-
-        mMockAccessibilityEnabled = false;
+        final Object lock = new Object();
+        mBatterySaverPolicy = new BatterySaverPolicyForTest(lock, getContext(),
+                new BatterySavingStats(lock, mMetricsLogger));
+        mBatterySaverPolicy.systemReady();
     }
 
     @SmallTest
@@ -112,7 +115,7 @@ public class BatterySaverPolicyTest extends AndroidTestCase {
 
     @SmallTest
     public void testGetBatterySaverPolicy_PolicyVibration_WithAccessibilityEnabled() {
-        mMockAccessibilityEnabled = true;
+        mBatterySaverPolicy.setAccessibilityEnabledForTest(true);
         testServiceDefaultValue_unchanged(ServiceType.VIBRATION);
     }
 
diff --git a/services/tests/servicestests/src/com/android/server/power/batterysaver/BatterySaverStateMachineTest.java b/services/tests/servicestests/src/com/android/server/power/batterysaver/BatterySaverStateMachineTest.java
index 62fe6b261715..5b247253fd9c 100644
--- a/services/tests/servicestests/src/com/android/server/power/batterysaver/BatterySaverStateMachineTest.java
+++ b/services/tests/servicestests/src/com/android/server/power/batterysaver/BatterySaverStateMachineTest.java
@@ -125,7 +125,7 @@ public class BatterySaverStateMachineTest {
      */
     private class TestableBatterySaverStateMachine extends BatterySaverStateMachine {
         public TestableBatterySaverStateMachine() {
-            super(mMockContext, mMockBatterySaverController);
+            super(new Object(), mMockContext, mMockBatterySaverController);
         }
 
         @Override
diff --git a/services/tests/servicestests/src/com/android/server/power/batterysaver/BatterySavingStatsTest.java b/services/tests/servicestests/src/com/android/server/power/batterysaver/BatterySavingStatsTest.java
index 0f3ca03fa6d7..9bd4cc34662a 100644
--- a/services/tests/servicestests/src/com/android/server/power/batterysaver/BatterySavingStatsTest.java
+++ b/services/tests/servicestests/src/com/android/server/power/batterysaver/BatterySavingStatsTest.java
@@ -53,7 +53,7 @@ public class BatterySavingStatsTest {
         private int mBatteryLevel = 1_000_000_000;
 
         private BatterySavingStatsTestable() {
-            super(mMetricsLogger);
+            super(new Object(), mMetricsLogger);
         }
 
         @Override