From 25fa3a704357b86bff6c0c74966a76e1993c022c Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Fri, 12 Jul 2019 17:00:17 -0700 Subject: [PATCH] Fix Progressive Battery Saver's Data Saver toggle. Make sure that Data Saver can be enabled properly with Progressive Battery Saver. Before this change, NetworkPolicyManagerService would look at the global battery saver enabled flag, which meant that Data Saver would only be enabled if BatterySaverPolicy.advertiseIsEnabled and BatterySaverPolicy.enableDataSaver were both set to true. Now it only requires the latter. Bug: 119261320 Test: atest com.android.server.net.NetworkPolicyManagerServiceTest Change-Id: I0f538129c4ed6a586316426a35e767d2d6b6ba1d --- .../net/NetworkPolicyManagerService.java | 28 ++++--- .../net/NetworkPolicyManagerServiceTest.java | 78 ++++++++++++++++++- 2 files changed, 90 insertions(+), 16 deletions(-) diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java index d3346cddf3d09..5530b51042e33 100644 --- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java +++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java @@ -403,8 +403,10 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { private IConnectivityManager mConnManager; private PowerManagerInternal mPowerManagerInternal; private IDeviceIdleController mDeviceIdleController; + + /** Current cached value of the current Battery Saver mode's setting for restrict background. */ @GuardedBy("mUidRulesFirstLock") - private PowerSaveState mRestrictBackgroundPowerState; + private boolean mRestrictBackgroundLowPowerMode; // Store the status of restrict background before turning on battery saver. // Used to restore mRestrictBackground when battery saver is turned off. @@ -771,11 +773,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { // Update the restrictBackground if battery saver is turned on mRestrictBackgroundBeforeBsm = mLoadedRestrictBackground; - mRestrictBackgroundPowerState = mPowerManagerInternal - .getLowPowerState(ServiceType.DATA_SAVER); - final boolean localRestrictBackground = - mRestrictBackgroundPowerState.batterySaverEnabled; - if (localRestrictBackground && !mLoadedRestrictBackground) { + mRestrictBackgroundLowPowerMode = mPowerManagerInternal + .getLowPowerState(ServiceType.DATA_SAVER).batterySaverEnabled; + if (mRestrictBackgroundLowPowerMode && !mLoadedRestrictBackground) { mLoadedRestrictBackground = true; } mPowerManagerInternal.registerLowPowerModeObserver( @@ -2900,7 +2900,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { sendRestrictBackgroundChangedMsg(); mLogger.restrictBackgroundChanged(oldRestrictBackground, mRestrictBackground); - if (mRestrictBackgroundPowerState.globalBatterySaverEnabled) { + if (mRestrictBackgroundLowPowerMode) { mRestrictBackgroundChangedInBsm = true; } synchronized (mNetworkPoliciesSecondLock) { @@ -4907,17 +4907,21 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { @GuardedBy("mUidRulesFirstLock") @VisibleForTesting void updateRestrictBackgroundByLowPowerModeUL(final PowerSaveState result) { - mRestrictBackgroundPowerState = result; + if (mRestrictBackgroundLowPowerMode == result.batterySaverEnabled) { + // Nothing changed. Nothing to do. + return; + } + mRestrictBackgroundLowPowerMode = result.batterySaverEnabled; - boolean restrictBackground = result.batterySaverEnabled; + boolean restrictBackground = mRestrictBackgroundLowPowerMode; boolean shouldInvokeRestrictBackground; - // store the temporary mRestrictBackgroundChangedInBsm and update it at last + // store the temporary mRestrictBackgroundChangedInBsm and update it at the end. boolean localRestrictBgChangedInBsm = mRestrictBackgroundChangedInBsm; - if (result.globalBatterySaverEnabled) { + if (mRestrictBackgroundLowPowerMode) { // Try to turn on restrictBackground if (1) it is off and (2) batter saver need to // turn it on. - shouldInvokeRestrictBackground = !mRestrictBackground && result.batterySaverEnabled; + shouldInvokeRestrictBackground = !mRestrictBackground; mRestrictBackgroundBeforeBsm = mRestrictBackground; localRestrictBgChangedInBsm = false; } else { diff --git a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java index ce1edcd584f69..09ae3a2628bf7 100644 --- a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java @@ -559,7 +559,25 @@ public class NetworkPolicyManagerServiceTest { .build(); mService.updateRestrictBackgroundByLowPowerModeUL(stateOff); - // RestrictBackground should be on, following its previous state + // RestrictBackground should be on, as before. + assertTrue(mService.getRestrictBackground()); + + stateOn = new PowerSaveState.Builder() + .setGlobalBatterySaverEnabled(true) + .setBatterySaverEnabled(true) + .build(); + mService.updateRestrictBackgroundByLowPowerModeUL(stateOn); + + // RestrictBackground should be on. + assertTrue(mService.getRestrictBackground()); + + stateOff = new PowerSaveState.Builder() + .setGlobalBatterySaverEnabled(false) + .setBatterySaverEnabled(false) + .build(); + mService.updateRestrictBackgroundByLowPowerModeUL(stateOff); + + // RestrictBackground should be on, as it was enabled manually before battery saver. assertTrue(mService.getRestrictBackground()); } @@ -585,6 +603,20 @@ public class NetworkPolicyManagerServiceTest { // RestrictBackground should be off, following its previous state assertFalse(mService.getRestrictBackground()); + + PowerSaveState stateOnRestrictOff = new PowerSaveState.Builder() + .setGlobalBatterySaverEnabled(true) + .setBatterySaverEnabled(false) + .build(); + + mService.updateRestrictBackgroundByLowPowerModeUL(stateOnRestrictOff); + + assertFalse(mService.getRestrictBackground()); + + mService.updateRestrictBackgroundByLowPowerModeUL(stateOff); + + // RestrictBackground should still be off. + assertFalse(mService.getRestrictBackground()); } @Test @@ -602,11 +634,49 @@ public class NetworkPolicyManagerServiceTest { // User turns off RestrictBackground manually setRestrictBackground(false); - PowerSaveState stateOff = new PowerSaveState.Builder().setBatterySaverEnabled( - false).build(); + // RestrictBackground should be off because user changed it manually + assertFalse(mService.getRestrictBackground()); + + PowerSaveState stateOff = new PowerSaveState.Builder() + .setGlobalBatterySaverEnabled(false) + .setBatterySaverEnabled(false) + .build(); mService.updateRestrictBackgroundByLowPowerModeUL(stateOff); - // RestrictBackground should be off because user changes it manually + // RestrictBackground should remain off. + assertFalse(mService.getRestrictBackground()); + } + + @Test + public void updateRestrictBackgroundByLowPowerMode_RestrictOnWithGlobalOff() + throws Exception { + setRestrictBackground(false); + PowerSaveState stateOn = new PowerSaveState.Builder() + .setGlobalBatterySaverEnabled(false) + .setBatterySaverEnabled(true) + .build(); + + mService.updateRestrictBackgroundByLowPowerModeUL(stateOn); + + // RestrictBackground should be turned on because of battery saver. + assertTrue(mService.getRestrictBackground()); + + PowerSaveState stateRestrictOff = new PowerSaveState.Builder() + .setGlobalBatterySaverEnabled(true) + .setBatterySaverEnabled(false) + .build(); + mService.updateRestrictBackgroundByLowPowerModeUL(stateRestrictOff); + + // RestrictBackground should be off, returning to its state before battery saver's change. + assertFalse(mService.getRestrictBackground()); + + PowerSaveState stateOff = new PowerSaveState.Builder() + .setGlobalBatterySaverEnabled(false) + .setBatterySaverEnabled(false) + .build(); + mService.updateRestrictBackgroundByLowPowerModeUL(stateOff); + + // RestrictBackground should still be off, back in its pre-battery saver state. assertFalse(mService.getRestrictBackground()); }