From 543339fe6b01aaaf112b6b093bc34a7a7aa79a7e Mon Sep 17 00:00:00 2001 From: Sudheer Shanka Date: Fri, 28 Jul 2017 15:18:07 -0700 Subject: [PATCH] Update NPMS.setRestrictBackgroundUL to notify listeners. - Move the notifying listeners part to setRestrictBackgroundUL so that we can avoid notifying if the restrict background update fails. - Add tracing code for setRestrictBackgroundUL. - Remove any existing restrict_background_changed messages in the queue before posting a new one so that listeners will be notified only for the latest update. Fixes: 36328002 Test: bit FrameworksServicesTests:com.android.server.NetworkPolicyManagerServiceTest Change-Id: I83c6270aa9f2d38cd418d2d88bfe59394b111692 --- .../net/NetworkPolicyManagerService.java | 102 +++++++++--------- .../NetworkPolicyManagerServiceTest.java | 5 +- 2 files changed, 53 insertions(+), 54 deletions(-) diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java index f70486a8b889a..45b27cce4fb77 100644 --- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java +++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java @@ -353,6 +353,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { // Used to restore mRestrictBackground when battery saver is turned off. private boolean mRestrictBackgroundBeforeBsm; + // Denotes the status of restrict background read from disk. + private boolean mLoadedRestrictBackground; + // See main javadoc for instructions on how to use these locks. final Object mUidRulesFirstLock = new Object(); final Object mNetworkPoliciesSecondLock = new Object(); @@ -680,15 +683,13 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { readPolicyAL(); // Update the restrictBackground if battery saver is turned on - mRestrictBackgroundBeforeBsm = mRestrictBackground; + mRestrictBackgroundBeforeBsm = mLoadedRestrictBackground; mRestrictBackgroundPowerState = mPowerManagerInternal .getLowPowerState(ServiceType.DATA_SAVER); final boolean localRestrictBackground = mRestrictBackgroundPowerState.batterySaverEnabled; - if (localRestrictBackground && localRestrictBackground != mRestrictBackground) { - mRestrictBackground = localRestrictBackground; - mHandler.obtainMessage(MSG_RESTRICT_BACKGROUND_CHANGED, - mRestrictBackground ? 1 : 0, 0).sendToTarget(); + if (localRestrictBackground && !mLoadedRestrictBackground) { + mLoadedRestrictBackground = true; } mPowerManagerInternal.registerLowPowerModeObserver( new PowerManagerInternal.LowPowerModeListener() { @@ -709,7 +710,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { writePolicyAL(); } - setRestrictBackgroundUL(mRestrictBackground); + setRestrictBackgroundUL(mLoadedRestrictBackground); updateRulesForGlobalChangeAL(false); updateNotificationsNL(); } @@ -1782,19 +1783,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { if (TAG_POLICY_LIST.equals(tag)) { final boolean oldValue = mRestrictBackground; version = readIntAttribute(in, ATTR_VERSION); - if (version >= VERSION_ADDED_RESTRICT_BACKGROUND) { - mRestrictBackground = readBooleanAttribute( - in, ATTR_RESTRICT_BACKGROUND); - } else { - mRestrictBackground = false; - } - if (mRestrictBackground != oldValue) { - // Some early services may have read the default value, - // so notify them that it's changed - mHandler.obtainMessage(MSG_RESTRICT_BACKGROUND_CHANGED, - mRestrictBackground ? 1 : 0, 0).sendToTarget(); - } - + mLoadedRestrictBackground = (version >= VERSION_ADDED_RESTRICT_BACKGROUND) + && readBooleanAttribute(in, ATTR_RESTRICT_BACKGROUND); } else if (TAG_NETWORK_POLICY.equals(tag)) { final int networkTemplate = readIntAttribute(in, ATTR_NETWORK_TEMPLATE); final String subscriberId = in.getAttributeValue(null, ATTR_SUBSCRIBER_ID); @@ -1978,7 +1968,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { // usually happens on first boot of a new device and not one that has received an OTA. // Seed from the default value configured for this device. - mRestrictBackground = Settings.Global.getInt( + mLoadedRestrictBackground = Settings.Global.getInt( mContext.getContentResolver(), Global.DEFAULT_RESTRICT_BACKGROUND_DATA, 0) == 1; // NOTE: We used to read the legacy setting here : @@ -2449,54 +2439,64 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { try { maybeRefreshTrustedTime(); synchronized (mUidRulesFirstLock) { - if (restrictBackground == mRestrictBackground) { - // Ideally, UI should never allow this scenario... - Slog.w(TAG, "setRestrictBackground: already " + restrictBackground); - return; - } setRestrictBackgroundUL(restrictBackground); } - } finally { Binder.restoreCallingIdentity(token); } - - mHandler.obtainMessage(MSG_RESTRICT_BACKGROUND_CHANGED, restrictBackground ? 1 : 0, 0) - .sendToTarget(); } finally { Trace.traceEnd(Trace.TRACE_TAG_NETWORK); } } private void setRestrictBackgroundUL(boolean restrictBackground) { - Slog.d(TAG, "setRestrictBackgroundUL(): " + restrictBackground); - final boolean oldRestrictBackground = mRestrictBackground; - mRestrictBackground = restrictBackground; - // Must whitelist foreground apps before turning data saver mode on. - // TODO: there is no need to iterate through all apps here, just those in the foreground, - // so it could call AM to get the UIDs of such apps, and iterate through them instead. - updateRulesForRestrictBackgroundUL(); + Trace.traceBegin(Trace.TRACE_TAG_NETWORK, "setRestrictBackgroundUL"); try { - if (!mNetworkManager.setDataSaverModeEnabled(mRestrictBackground)) { - Slog.e(TAG, "Could not change Data Saver Mode on NMS to " + mRestrictBackground); - mRestrictBackground = oldRestrictBackground; - // TODO: if it knew the foreground apps (see TODO above), it could call - // updateRulesForRestrictBackgroundUL() again to restore state. + if (restrictBackground == mRestrictBackground) { + // Ideally, UI should never allow this scenario... + Slog.w(TAG, "setRestrictBackgroundUL: already " + restrictBackground); return; } - } catch (RemoteException e) { - // ignored; service lives in system_server - } + Slog.d(TAG, "setRestrictBackgroundUL(): " + restrictBackground); + final boolean oldRestrictBackground = mRestrictBackground; + mRestrictBackground = restrictBackground; + // Must whitelist foreground apps before turning data saver mode on. + // TODO: there is no need to iterate through all apps here, just those in the foreground, + // so it could call AM to get the UIDs of such apps, and iterate through them instead. + updateRulesForRestrictBackgroundUL(); + try { + if (!mNetworkManager.setDataSaverModeEnabled(mRestrictBackground)) { + Slog.e(TAG, + "Could not change Data Saver Mode on NMS to " + mRestrictBackground); + mRestrictBackground = oldRestrictBackground; + // TODO: if it knew the foreground apps (see TODO above), it could call + // updateRulesForRestrictBackgroundUL() again to restore state. + return; + } + } catch (RemoteException e) { + // ignored; service lives in system_server + } - if (mRestrictBackgroundPowerState.globalBatterySaverEnabled) { - mRestrictBackgroundChangedInBsm = true; - } - synchronized (mNetworkPoliciesSecondLock) { - updateNotificationsNL(); - writePolicyAL(); + sendRestrictBackgroundChangedMsg(); + + if (mRestrictBackgroundPowerState.globalBatterySaverEnabled) { + mRestrictBackgroundChangedInBsm = true; + } + synchronized (mNetworkPoliciesSecondLock) { + updateNotificationsNL(); + writePolicyAL(); + } + } finally { + Trace.traceEnd(Trace.TRACE_TAG_NETWORK); } } + private void sendRestrictBackgroundChangedMsg() { + mHandler.removeMessages(MSG_RESTRICT_BACKGROUND_CHANGED); + mHandler.obtainMessage(MSG_RESTRICT_BACKGROUND_CHANGED, mRestrictBackground ? 1 : 0, 0) + .sendToTarget(); + } + @Override public int getRestrictBackgroundByCaller() { mContext.enforceCallingOrSelfPermission(ACCESS_NETWORK_STATE, TAG); @@ -4167,7 +4167,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { } if (shouldInvokeRestrictBackground) { - setRestrictBackground(restrictBackground); + setRestrictBackgroundUL(restrictBackground); } // Change it at last so setRestrictBackground() won't affect this variable diff --git a/services/tests/servicestests/src/com/android/server/NetworkPolicyManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/NetworkPolicyManagerServiceTest.java index 8decb968c8fbe..40788299a6c62 100644 --- a/services/tests/servicestests/src/com/android/server/NetworkPolicyManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/NetworkPolicyManagerServiceTest.java @@ -51,6 +51,7 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; @@ -360,6 +361,7 @@ public class NetworkPolicyManagerServiceTest { .thenReturn(new ApplicationInfo()); when(mPackageManager.getPackagesForUid(UID_A)).thenReturn(new String[] {PKG_NAME_A}); when(mNetworkManager.isBandwidthControlEnabled()).thenReturn(true); + when(mNetworkManager.setDataSaverModeEnabled(anyBoolean())).thenReturn(true); expectCurrentTime(); // Prepare NPMS. @@ -520,7 +522,6 @@ public class NetworkPolicyManagerServiceTest { .setBatterySaverEnabled(true) .build(); - doReturn(true).when(mNetworkManager).setDataSaverModeEnabled(true); mService.updateRestrictBackgroundByLowPowerModeUL(stateOn); // RestrictBackground should be turned on because of battery saver @@ -1634,8 +1635,6 @@ public class NetworkPolicyManagerServiceTest { private FutureIntent mRestrictBackgroundChanged; private void setRestrictBackground(boolean flag) throws Exception { - // Must set expectation, otherwise NMPS will reset value to previous one. - doReturn(true).when(mNetworkManager).setDataSaverModeEnabled(flag); mService.setRestrictBackground(flag); // Sanity check. assertEquals("restrictBackground not set", flag, mService.getRestrictBackground());