Removed screen on/off callbacks from NPMS.

NetworkPolicyManagerService (NPMS) used to depend on screen on/off
changes to determine if a foreground activity should have network
restrictions, but such check is now redundant since ActivityManager
already changes the proper UID state (like going from TOP to
TOP_SLEEPING) when the screen status is changed.

Removing such code decreases the NPMS lock contention when the screen is
turned on in about 3-5ms.

Change-Id: I2853443efedbf14961ae9a5b2e72689d4d1a646c
BUG: 30785671
(cherry picked from commit 88f40ad9a7)
(cherry picked from commit f8dd7b4e8d)

services/core/java/com/android/server/net/NetworkPolicyManagerService.java

@@ -133,7 +133,6 @@ import android.os.Handler;
 import android.os.HandlerThread;
 import android.os.IDeviceIdleController;
 import android.os.INetworkManagementService;
-import android.os.IPowerManager;
 import android.os.Message;
 import android.os.MessageQueue.IdleHandler;
 import android.os.PowerManager;
@@ -286,7 +285,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
     private static final int MSG_LIMIT_REACHED = 5;
     private static final int MSG_RESTRICT_BACKGROUND_CHANGED = 6;
     private static final int MSG_ADVISE_PERSIST_THRESHOLD = 7;
-    private static final int MSG_SCREEN_ON_CHANGED = 8;
     private static final int MSG_RESTRICT_BACKGROUND_WHITELIST_CHANGED = 9;
     private static final int MSG_UPDATE_INTERFACE_QUOTA = 10;
     private static final int MSG_REMOVE_INTERFACE_QUOTA = 11;
@@ -294,7 +292,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
 
     private final Context mContext;
     private final IActivityManager mActivityManager;
-    private final IPowerManager mPowerManager;
     private final INetworkStatsService mNetworkStats;
     private final INetworkManagementService mNetworkManager;
     private UsageStatsManagerInternal mUsageStats;
@@ -312,7 +309,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
 
     @GuardedBy("allLocks") volatile boolean mSystemReady;
 
-    @GuardedBy("mUidRulesFirstLock") volatile boolean mScreenOn;
     @GuardedBy("mUidRulesFirstLock") volatile boolean mRestrictBackground;
     @GuardedBy("mUidRulesFirstLock") volatile boolean mRestrictPower;
     @GuardedBy("mUidRulesFirstLock") volatile boolean mDeviceIdleMode;
@@ -418,9 +414,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
     // TODO: migrate notifications to SystemUI
 
     public NetworkPolicyManagerService(Context context, IActivityManager activityManager,
-            IPowerManager powerManager, INetworkStatsService networkStats,
+            INetworkStatsService networkStats, INetworkManagementService networkManagement) {
-            INetworkManagementService networkManagement) {
+        this(context, activityManager, networkStats, networkManagement,
-        this(context, activityManager, powerManager, networkStats, networkManagement,
                 NtpTrustedTime.getInstance(context), getSystemDir(), false);
     }
 
@@ -429,12 +424,10 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
     }
 
     public NetworkPolicyManagerService(Context context, IActivityManager activityManager,
-            IPowerManager powerManager, INetworkStatsService networkStats,
+            INetworkStatsService networkStats, INetworkManagementService networkManagement,
-            INetworkManagementService networkManagement, TrustedTime time, File systemDir,
+            TrustedTime time, File systemDir, boolean suppressDefaultPolicy) {
-            boolean suppressDefaultPolicy) {
         mContext = checkNotNull(context, "missing context");
         mActivityManager = checkNotNull(activityManager, "missing activityManager");
-        mPowerManager = checkNotNull(powerManager, "missing powerManager");
         mNetworkStats = checkNotNull(networkStats, "missing networkStats");
         mNetworkManager = checkNotNull(networkManagement, "missing networkManagement");
         mDeviceIdleController = IDeviceIdleController.Stub.asInterface(ServiceManager.getService(
@@ -618,8 +611,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
             }
         }
 
-        updateScreenOn();
-
         try {
             mActivityManager.registerUidObserver(mUidObserver,
                     ActivityManager.UID_OBSERVER_PROCSTATE|ActivityManager.UID_OBSERVER_GONE);
@@ -628,14 +619,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
             // ignored; both services live in system_server
         }
 
-        // TODO: traverse existing processes to know foreground state, or have
-        // activitymanager dispatch current state when new observer attached.
-
-        final IntentFilter screenFilter = new IntentFilter();
-        screenFilter.addAction(Intent.ACTION_SCREEN_ON);
-        screenFilter.addAction(Intent.ACTION_SCREEN_OFF);
-        mContext.registerReceiver(mScreenReceiver, screenFilter);
-
         // listen for changes to power save whitelist
         final IntentFilter whitelistFilter = new IntentFilter(
                 PowerManager.ACTION_POWER_SAVE_WHITELIST_CHANGED);
@@ -734,15 +717,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
         }
     };
 
-    final private BroadcastReceiver mScreenReceiver = new BroadcastReceiver() {
-        @Override
-        public void onReceive(Context context, Intent intent) {
-            // screen-related broadcasts are protected by system, no need
-            // for permissions check.
-            mHandler.obtainMessage(MSG_SCREEN_ON_CHANGED).sendToTarget();
-        }
-    };
-
     final private BroadcastReceiver mPackageReceiver = new BroadcastReceiver() {
         @Override
         public void onReceive(Context context, Intent intent) {
@@ -2520,7 +2494,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
 
     private boolean isUidStateForegroundUL(int state) {
         // only really in foreground when screen is also on
-        return mScreenOn && state <= ActivityManager.PROCESS_STATE_TOP;
+        return state <= ActivityManager.PROCESS_STATE_TOP;
     }
 
     /**
@@ -2591,31 +2565,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
         }
     }
 
-    private void updateScreenOn() {
-        synchronized (mUidRulesFirstLock) {
-            try {
-                mScreenOn = mPowerManager.isInteractive();
-            } catch (RemoteException e) {
-                // ignored; service lives in system_server
-            }
-            updateRulesForScreenUL();
-        }
-    }
-
-    /**
-     * Update rules that might be changed by {@link #mScreenOn} value.
-     */
-    private void updateRulesForScreenUL() {
-        // only update rules for anyone with foreground activities
-        final int size = mUidState.size();
-        for (int i = 0; i < size; i++) {
-            if (mUidState.valueAt(i) <= ActivityManager.PROCESS_STATE_FOREGROUND_SERVICE) {
-                final int uid = mUidState.keyAt(i);
-                updateRestrictionRulesForUidUL(uid);
-            }
-        }
-    }
-
     static boolean isProcStateAllowedWhileIdleOrPowerSaveMode(int procState) {
         return procState <= ActivityManager.PROCESS_STATE_FOREGROUND_SERVICE;
     }
@@ -2997,12 +2946,8 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
             mUidRules.put(uid, newUidRules);
         }
 
-        boolean changed = false;
-
         // Second step: apply bw changes based on change of state.
         if (newRule != oldRule) {
-            changed = true;
-
             if ((newRule & RULE_TEMPORARY_ALLOW_METERED) != 0) {
                 // Temporarily whitelist foreground app, removing from blacklist if necessary
                 // (since bw_penalty_box prevails over bw_happy_box).
@@ -3082,7 +3027,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
 
         final boolean isIdle = isUidIdle(uid);
         final boolean restrictMode = isIdle || mRestrictPower || mDeviceIdleMode;
-        final int uidPolicy = mUidPolicy.get(uid, POLICY_NONE);
         final int oldUidRules = mUidRules.get(uid, RULE_NONE);
         final boolean isForeground = isUidForegroundOnRestrictPowerUL(uid);
 
@@ -3105,7 +3049,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
         final int newUidRules = (oldUidRules & MASK_METERED_NETWORKS) | newRule;
 
         if (LOGV) {
-            Log.v(TAG, "updateRulesForNonMeteredNetworksUL(" + uid + ")"
+            Log.v(TAG, "updateRulesForPowerRestrictionsUL(" + uid + ")"
                     + ", isIdle: " + isIdle
                     + ", mRestrictPower: " + mRestrictPower
                     + ", mDeviceIdleMode: " + mDeviceIdleMode
@@ -3347,10 +3291,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub {
                     }
                     return true;
                 }
-                case MSG_SCREEN_ON_CHANGED: {
-                    updateScreenOn();
-                    return true;
-                }
                 case MSG_UPDATE_INTERFACE_QUOTA: {
                     removeInterfaceQuota((String) msg.obj);
                     // int params need to be stitched back into a long

services/java/com/android/server/SystemServer.java

@@ -809,10 +809,8 @@ public final class SystemServer {
 
                 traceBeginAndSlog("StartNetworkPolicyManagerService");
                 try {
-                    networkPolicy = new NetworkPolicyManagerService(
+                    networkPolicy = new NetworkPolicyManagerService(context,
-                            context, mActivityManagerService,
+                            mActivityManagerService, networkStats, networkManagement);
-                            (IPowerManager)ServiceManager.getService(Context.POWER_SERVICE),
-                            networkStats, networkManagement);
                     ServiceManager.addService(Context.NETWORK_POLICY_SERVICE, networkPolicy);
                 } catch (Throwable e) {
                     reportWtf("starting NetworkPolicy Service", e);

services/tests/servicestests/src/com/android/server/NetworkPolicyManagerServiceTest.java

@@ -69,7 +69,6 @@ import android.net.NetworkStats;
 import android.net.NetworkTemplate;
 import android.os.Binder;
 import android.os.INetworkManagementService;
-import android.os.IPowerManager;
 import android.os.MessageQueue.IdleHandler;
 import android.os.UserHandle;
 import android.test.AndroidTestCase;
@@ -115,7 +114,6 @@ public class NetworkPolicyManagerServiceTest extends AndroidTestCase {
     private File mPolicyDir;
 
     private IActivityManager mActivityManager;
-    private IPowerManager mPowerManager;
     private INetworkStatsService mStatsService;
     private INetworkManagementService mNetworkManager;
     private INetworkPolicyListener mPolicyListener;
@@ -187,7 +185,6 @@ public class NetworkPolicyManagerServiceTest extends AndroidTestCase {
         }
 
         mActivityManager = createMock(IActivityManager.class);
-        mPowerManager = createMock(IPowerManager.class);
         mStatsService = createMock(INetworkStatsService.class);
         mNetworkManager = createMock(INetworkManagementService.class);
         mPolicyListener = createMock(INetworkPolicyListener.class);
@@ -195,7 +192,7 @@ public class NetworkPolicyManagerServiceTest extends AndroidTestCase {
         mConnManager = createMock(IConnectivityManager.class);
         mNotifManager = createMock(INotificationManager.class);
 
-        mService = new NetworkPolicyManagerService(mServiceContext, mActivityManager, mPowerManager,
+        mService = new NetworkPolicyManagerService(mServiceContext, mActivityManager,
                 mStatsService, mNetworkManager, mTime, mPolicyDir, true);
         mService.bindConnectivityManager(mConnManager);
         mService.bindNotificationManager(mNotifManager);
@@ -217,8 +214,6 @@ public class NetworkPolicyManagerServiceTest extends AndroidTestCase {
         mNetworkManager.registerObserver(capture(networkObserver));
         expectLastCall().atLeastOnce();
 
-        // expect to answer screen status during systemReady()
-        expect(mPowerManager.isInteractive()).andReturn(true).atLeastOnce();
         expect(mNetworkManager.isBandwidthControlEnabled()).andReturn(true).atLeastOnce();
         expectCurrentTime();
 
@@ -240,7 +235,6 @@ public class NetworkPolicyManagerServiceTest extends AndroidTestCase {
         mPolicyDir = null;
 
         mActivityManager = null;
-        mPowerManager = null;
         mStatsService = null;
         mPolicyListener = null;
         mTime = null;
@@ -312,48 +306,6 @@ public class NetworkPolicyManagerServiceTest extends AndroidTestCase {
         assertFalse(mService.isUidForeground(UID_A));
     }
 
-    @Suppress
-    public void testScreenChangesRules() throws Exception {
-        Future<Void> future;
-
-        expectSetUidMeteredNetworkBlacklist(UID_A, false);
-        expectSetUidForeground(UID_A, true);
-        future = expectRulesChanged(UID_A, RULE_ALLOW_ALL);
-        replay();
-        mProcessObserver.onForegroundActivitiesChanged(PID_1, UID_A, true);
-        future.get();
-        verifyAndReset();
-
-        // push strict policy for foreground uid, verify ALLOW rule
-        expectSetUidMeteredNetworkBlacklist(UID_A, false);
-        expectSetUidForeground(UID_A, true);
-        future = expectRulesChanged(UID_A, RULE_ALLOW_ALL);
-        replay();
-        mService.setUidPolicy(APP_ID_A, POLICY_REJECT_METERED_BACKGROUND);
-        future.get();
-        verifyAndReset();
-
-        // now turn screen off and verify REJECT rule
-        expect(mPowerManager.isInteractive()).andReturn(false).atLeastOnce();
-        expectSetUidMeteredNetworkBlacklist(UID_A, true);
-        expectSetUidForeground(UID_A, false);
-        future = expectRulesChanged(UID_A, RULE_REJECT_METERED);
-        replay();
-        mServiceContext.sendBroadcast(new Intent(Intent.ACTION_SCREEN_OFF));
-        future.get();
-        verifyAndReset();
-
-        // and turn screen back on, verify ALLOW rule restored
-        expect(mPowerManager.isInteractive()).andReturn(true).atLeastOnce();
-        expectSetUidMeteredNetworkBlacklist(UID_A, false);
-        expectSetUidForeground(UID_A, true);
-        future = expectRulesChanged(UID_A, RULE_ALLOW_ALL);
-        replay();
-        mServiceContext.sendBroadcast(new Intent(Intent.ACTION_SCREEN_ON));
-        future.get();
-        verifyAndReset();
-    }
-
     @Suppress
     public void testPolicyNone() throws Exception {
         Future<Void> future;
@@ -1049,14 +1001,14 @@ public class NetworkPolicyManagerServiceTest extends AndroidTestCase {
     }
 
     private void replay() {
-        EasyMock.replay(mActivityManager, mPowerManager, mStatsService, mPolicyListener,
+        EasyMock.replay(mActivityManager, mStatsService, mPolicyListener, mNetworkManager, mTime,
-                mNetworkManager, mTime, mConnManager, mNotifManager);
+                mConnManager, mNotifManager);
     }
 
     private void verifyAndReset() {
-        EasyMock.verify(mActivityManager, mPowerManager, mStatsService, mPolicyListener,
+        EasyMock.verify(mActivityManager, mStatsService, mPolicyListener, mNetworkManager, mTime,
-                mNetworkManager, mTime, mConnManager, mNotifManager);
+                mConnManager, mNotifManager);
-        EasyMock.reset(mActivityManager, mPowerManager, mStatsService, mPolicyListener,
+        EasyMock.reset(mActivityManager, mStatsService, mPolicyListener, mNetworkManager, mTime,
-                mNetworkManager, mTime, mConnManager, mNotifManager);
+                mConnManager, mNotifManager);
     }
 }