Merge "Rework battery saver synchronization logic" into pi-dev

am: 869c6f559a

Change-Id: Ibd889ec4d997969959e301eb0e23e1a3c58908da
This commit is contained in:
Makoto Onuki
2018-05-14 08:50:51 -07:00
committed by android-build-merger
9 changed files with 147 additions and 115 deletions

View File

@@ -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");
}
}
}

View File

@@ -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) {
/**
* Called by {@link PowerManagerService#systemReady}, *with no lock held.*
*/
public void systemReady() {
ConcurrentUtils.wtfIfLockHeld(TAG, mLock);
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);
acm.addAccessibilityStateChangeListener((enabled) -> {
synchronized (mLock) {
mAccessibilityEnabled = enabled;
}
refreshSettings();
});
final boolean enabled = acm.isEnabled();
synchronized (mLock) {
mContext = context;
mContentResolver = context.getContentResolver();
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);
mAccessibilityManager.addAccessibilityStateChangeListener((enabled) -> {
refreshSettings();
});
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;
}
}
}

View File

@@ -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

View File

@@ -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);

View File

@@ -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(

View File

@@ -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) {

View File

@@ -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);
}

View File

@@ -125,7 +125,7 @@ public class BatterySaverStateMachineTest {
*/
private class TestableBatterySaverStateMachine extends BatterySaverStateMachine {
public TestableBatterySaverStateMachine() {
super(mMockContext, mMockBatterySaverController);
super(new Object(), mMockContext, mMockBatterySaverController);
}
@Override

View File

@@ -53,7 +53,7 @@ public class BatterySavingStatsTest {
private int mBatteryLevel = 1_000_000_000;
private BatterySavingStatsTestable() {
super(mMetricsLogger);
super(new Object(), mMetricsLogger);
}
@Override