WifiNetworkFactory: Refactor to a new class

Move WifiNetworkFactory & UntrustedWifiNetworkFactory to separate
classes out of ClientModeImpl. Also, simplify the existing auto-join
enable mechanism (WifiConnectivityManager.enable()).

Also added a new |mRunning| flag in WifiConnectivityManager to avoid
multiple calls to start/stop being processed. There was no functional
problem without |mRunning| flag, but it makes the unit tests cleaner.

Bug: 116210817
Test: Unit tests
Test: Device boots up & auto-connects to wifi networks
Change-Id: I24038fe554ab7343e8884ded9ff60d26f2549ff7

5 files changed, 204 insertions, 112 deletions

diff --git a/service/java/com/android/server/wifi/ClientModeImpl.java b/service/java/com/android/server/wifi/ClientModeImpl.java
index a4eb8eaf1..1372e5ee1 100644
--- a/service/java/com/android/server/wifi/ClientModeImpl.java
+++ b/service/java/com/android/server/wifi/ClientModeImpl.java
@@ -41,11 +41,9 @@ import android.net.MacAddress;
 import android.net.Network;
 import android.net.NetworkAgent;
 import android.net.NetworkCapabilities;
-import android.net.NetworkFactory;
 import android.net.NetworkInfo;
 import android.net.NetworkInfo.DetailedState;
 import android.net.NetworkMisc;
-import android.net.NetworkRequest;
 import android.net.NetworkUtils;
 import android.net.RouteInfo;
 import android.net.StaticIpConfiguration;
@@ -85,7 +83,6 @@ import android.util.Pair;
 import android.util.SparseArray;
 
 import com.android.internal.R;
-import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.app.IBatteryStats;
 import com.android.internal.util.AsyncChannel;
@@ -133,7 +130,6 @@ import java.util.concurrent.atomic.AtomicInteger;
 public class ClientModeImpl extends StateMachine {
 
     private static final String NETWORKTYPE = "WIFI";
-    private static final String NETWORKTYPE_UNTRUSTED = "WIFI_UT";
     @VisibleForTesting public static final short NUM_LOG_RECS_NORMAL = 100;
     @VisibleForTesting public static final short NUM_LOG_RECS_VERBOSE_LOW_MEMORY = 200;
     @VisibleForTesting public static final short NUM_LOG_RECS_VERBOSE = 3000;
@@ -387,14 +383,9 @@ public class ClientModeImpl extends StateMachine {
     // Used to initiate a connection with WifiP2pService
     private AsyncChannel mWifiP2pChannel;
 
-    @GuardedBy("mWifiReqCountLock")
-    private int mConnectionReqCount = 0;
     private WifiNetworkFactory mNetworkFactory;
-    @GuardedBy("mWifiReqCountLock")
-    private int mUntrustedReqCount = 0;
     private UntrustedWifiNetworkFactory mUntrustedNetworkFactory;
     private WifiNetworkAgent mNetworkAgent;
-    private final Object mWifiReqCountLock = new Object();
 
     private byte[] mRssiRanges;
 
@@ -3100,95 +3091,20 @@ public class ClientModeImpl extends StateMachine {
         return true;
     }
 
-    private class WifiNetworkFactory extends NetworkFactory {
-        WifiNetworkFactory(Looper l, Context c, String tag, NetworkCapabilities f) {
-            super(l, c, tag, f);
-        }
-
-        @Override
-        protected void needNetworkFor(NetworkRequest networkRequest, int score) {
-            synchronized (mWifiReqCountLock) {
-                if (++mConnectionReqCount == 1) {
-                    if (mUntrustedReqCount == 0) {
-                        mWifiConnectivityManager.enable(true);
-                    }
-                }
-            }
-        }
-
-        @Override
-        protected void releaseNetworkFor(NetworkRequest networkRequest) {
-            synchronized (mWifiReqCountLock) {
-                if (--mConnectionReqCount == 0) {
-                    if (mUntrustedReqCount == 0) {
-                        mWifiConnectivityManager.enable(false);
-                    }
-                }
-            }
-        }
-
-        @Override
-        public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
-            pw.println("mConnectionReqCount " + mConnectionReqCount);
-        }
-
-    }
-
-    private class UntrustedWifiNetworkFactory extends NetworkFactory {
-        UntrustedWifiNetworkFactory(Looper l, Context c, String tag, NetworkCapabilities f) {
-            super(l, c, tag, f);
-        }
-
-        @Override
-        protected void needNetworkFor(NetworkRequest networkRequest, int score) {
-            if (!networkRequest.networkCapabilities.hasCapability(
-                    NetworkCapabilities.NET_CAPABILITY_TRUSTED)) {
-                synchronized (mWifiReqCountLock) {
-                    if (++mUntrustedReqCount == 1) {
-                        if (mConnectionReqCount == 0) {
-                            mWifiConnectivityManager.enable(true);
-                        }
-                        mWifiConnectivityManager.setUntrustedConnectionAllowed(true);
-                    }
-                }
-            }
-        }
-
-        @Override
-        protected void releaseNetworkFor(NetworkRequest networkRequest) {
-            if (!networkRequest.networkCapabilities.hasCapability(
-                    NetworkCapabilities.NET_CAPABILITY_TRUSTED)) {
-                synchronized (mWifiReqCountLock) {
-                    if (--mUntrustedReqCount == 0) {
-                        mWifiConnectivityManager.setUntrustedConnectionAllowed(false);
-                        if (mConnectionReqCount == 0) {
-                            mWifiConnectivityManager.enable(false);
-                        }
-                    }
-                }
-            }
-        }
-
-        @Override
-        public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
-            pw.println("mUntrustedReqCount " + mUntrustedReqCount);
-        }
-    }
-
     void maybeRegisterNetworkFactory() {
         if (mNetworkFactory == null) {
             checkAndSetConnectivityInstance();
             if (mCm != null) {
                 mNetworkFactory = new WifiNetworkFactory(getHandler().getLooper(), mContext,
-                        NETWORKTYPE, mNetworkCapabilitiesFilter);
-                mNetworkFactory.setScoreFilter(60);
+                        mNetworkCapabilitiesFilter, mWifiConnectivityManager);
                 mNetworkFactory.register();
 
                 // We can't filter untrusted network in the capabilities filter because a trusted
                 // network would still satisfy a request that accepts untrusted ones.
+                // We need a second network factory for untrusted network requests because we need a
+                // different score filter for these requests.
                 mUntrustedNetworkFactory = new UntrustedWifiNetworkFactory(getHandler().getLooper(),
-                        mContext, NETWORKTYPE_UNTRUSTED, mNetworkCapabilitiesFilter);
-                mUntrustedNetworkFactory.setScoreFilter(Integer.MAX_VALUE);
+                        mContext, mNetworkCapabilitiesFilter, mWifiConnectivityManager);
                 mUntrustedNetworkFactory.register();
             }
         }
@@ -4129,20 +4045,17 @@ public class ClientModeImpl extends StateMachine {
                     int uid = message.arg2;
                     bssid = (String) message.obj;
 
-                    synchronized (mWifiReqCountLock) {
-                        if (!hasConnectionRequests()) {
-                            if (mNetworkAgent == null) {
-                                loge("CMD_START_CONNECT but no requests and not connected,"
-                                        + " bailing");
-                                break;
-                            } else if (!mWifiPermissionsUtil.checkNetworkSettingsPermission(uid)) {
-                                loge("CMD_START_CONNECT but no requests and connected, but app "
-                                        + "does not have sufficient permissions, bailing");
-                                break;
-                            }
+                    if (!hasConnectionRequests()) {
+                        if (mNetworkAgent == null) {
+                            loge("CMD_START_CONNECT but no requests and not connected,"
+                                    + " bailing");
+                            break;
+                        } else if (!mWifiPermissionsUtil.checkNetworkSettingsPermission(uid)) {
+                            loge("CMD_START_CONNECT but no requests and connected, but app "
+                                    + "does not have sufficient permissions, bailing");
+                            break;
                         }
                     }
-
                     config = mWifiConfigManager.getConfiguredNetworkWithoutMasking(netId);
                     logd("CMD_START_CONNECT sup state "
                             + mSupplicantStateTracker.getSupplicantStateName()
@@ -5874,10 +5787,10 @@ public class ClientModeImpl extends StateMachine {
 
     /**
      * Check if there is any connection request for WiFi network.
-     * Note, caller of this helper function must acquire mWifiReqCountLock.
      */
     private boolean hasConnectionRequests() {
-        return mConnectionReqCount > 0 || mUntrustedReqCount > 0;
+        return mNetworkFactory.hasConnectionRequests()
+                || mUntrustedNetworkFactory.hasConnectionRequests();
     }
 
     /**
diff --git a/service/java/com/android/server/wifi/UntrustedWifiNetworkFactory.java b/service/java/com/android/server/wifi/UntrustedWifiNetworkFactory.java
new file mode 100644
index 000000000..86f3cd87e
--- /dev/null
+++ b/service/java/com/android/server/wifi/UntrustedWifiNetworkFactory.java
@@ -0,0 +1,85 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.wifi;
+
+import android.content.Context;
+import android.net.NetworkCapabilities;
+import android.net.NetworkFactory;
+import android.net.NetworkRequest;
+import android.os.Looper;
+import android.util.Log;
+
+import java.io.FileDescriptor;
+import java.io.PrintWriter;
+
+/**
+ * Network factory to handle untrusted (used for {@link ScoredNetworkEvaluator}) wifi network
+ * requests.
+ */
+public class UntrustedWifiNetworkFactory extends NetworkFactory {
+    private static final String TAG = "UntrustedWifiNetworkFactory";
+    private static final int SCORE_FILTER = Integer.MAX_VALUE;
+
+    private final WifiConnectivityManager mWifiConnectivityManager;
+    private int mConnectionReqCount = 0;
+
+    public UntrustedWifiNetworkFactory(Looper l, Context c, NetworkCapabilities f,
+                                       WifiConnectivityManager connectivityManager) {
+        super(l, c, TAG, f);
+        mWifiConnectivityManager = connectivityManager;
+
+        setScoreFilter(SCORE_FILTER);
+    }
+
+    @Override
+    protected void needNetworkFor(NetworkRequest networkRequest, int score) {
+        if (!networkRequest.networkCapabilities.hasCapability(
+                NetworkCapabilities.NET_CAPABILITY_TRUSTED)) {
+            if (++mConnectionReqCount == 1) {
+                mWifiConnectivityManager.setUntrustedConnectionAllowed(true);
+            }
+        }
+    }
+
+    @Override
+    protected void releaseNetworkFor(NetworkRequest networkRequest) {
+        if (!networkRequest.networkCapabilities.hasCapability(
+                NetworkCapabilities.NET_CAPABILITY_TRUSTED)) {
+            if (mConnectionReqCount == 0) {
+                Log.e(TAG, "No valid network request to release");
+                return;
+            }
+            if (--mConnectionReqCount == 0) {
+                mWifiConnectivityManager.setUntrustedConnectionAllowed(false);
+            }
+        }
+    }
+
+    @Override
+    public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
+        super.dump(fd, pw, args);
+        pw.println(TAG + ": mConnectionReqCount " + mConnectionReqCount);
+    }
+
+    /**
+     * Check if there is at-least one connection request.
+     */
+    public boolean hasConnectionRequests() {
+        return mConnectionReqCount > 0;
+    }
+}
+
diff --git a/service/java/com/android/server/wifi/WifiConnectivityManager.java b/service/java/com/android/server/wifi/WifiConnectivityManager.java
index 07d9cd393..cd3e4c8c2 100644
--- a/service/java/com/android/server/wifi/WifiConnectivityManager.java
+++ b/service/java/com/android/server/wifi/WifiConnectivityManager.java
@@ -153,9 +153,11 @@ public class WifiConnectivityManager {
     private boolean mDbg = false;
     private boolean mWifiEnabled = false;
     private boolean mWifiConnectivityManagerEnabled = false;
+    private boolean mRunning = false;
     private boolean mScreenOn = false;
     private int mWifiState = WIFI_STATE_UNKNOWN;
     private boolean mUntrustedConnectionAllowed = false;
+    private boolean mTrustedConnectionAllowed = false;
     private int mScanRestartCount = 0;
     private int mSingleScanRestartCount = 0;
     private int mTotalConnectivityAttemptsRateLimited = 0;
@@ -577,8 +579,7 @@ public class WifiConnectivityManager {
             WifiLastResortWatchdog wifiLastResortWatchdog, OpenNetworkNotifier openNetworkNotifier,
             CarrierNetworkNotifier carrierNetworkNotifier,
             CarrierNetworkConfig carrierNetworkConfig, WifiMetrics wifiMetrics, Looper looper,
-            Clock clock, LocalLog localLog, boolean enable,
-            SavedNetworkEvaluator savedNetworkEvaluator,
+            Clock clock, LocalLog localLog, SavedNetworkEvaluator savedNetworkEvaluator,
             ScoredNetworkEvaluator scoredNetworkEvaluator,
             PasspointNetworkEvaluator passpointNetworkEvaluator) {
         mStateMachine = stateMachine;
@@ -648,11 +649,6 @@ public class WifiConnectivityManager {
 
         // Listen to WifiConfigManager network update events
         mConfigManager.setOnSavedNetworkUpdateListener(new OnSavedNetworkUpdateListener());
-
-        mWifiConnectivityManagerEnabled = enable;
-
-        localLog("ConnectivityScanManager initialized and "
-                + (enable ? "enabled" : "disabled"));
     }
 
     /**
@@ -1152,13 +1148,30 @@ public class WifiConnectivityManager {
     }
 
     /**
-     * Handler when user toggles whether untrusted connection is allowed
+     * Handler when connectivity allows/disallows trusted connections (all of autojoin).
+     */
+    public void setTrustedConnectionAllowed(boolean allowed) {
+        localLog("setTrustedConnectionAllowed: allowed=" + allowed);
+
+        if (mTrustedConnectionAllowed != allowed) {
+            mTrustedConnectionAllowed = allowed;
+            // Enable auto-join if we have any pending network request (trusted or untrusted).
+            enable(mUntrustedConnectionAllowed || mTrustedConnectionAllowed);
+            startConnectivityScan(SCAN_IMMEDIATELY);
+        }
+    }
+
+
+    /**
+     * Handler when connectivity allows/disallows untrusted connections (ephemeral networks).
      */
     public void setUntrustedConnectionAllowed(boolean allowed) {
         localLog("setUntrustedConnectionAllowed: allowed=" + allowed);
 
         if (mUntrustedConnectionAllowed != allowed) {
             mUntrustedConnectionAllowed = allowed;
+            // Enable auto-join if we have any pending network request (trusted or untrusted).
+            enable(mUntrustedConnectionAllowed || mTrustedConnectionAllowed);
             startConnectivityScan(SCAN_IMMEDIATELY);
         }
     }
@@ -1374,16 +1387,20 @@ public class WifiConnectivityManager {
      * Start WifiConnectivityManager
      */
     private void start() {
+        if (mRunning) return;
         retrieveWifiScanner();
         mConnectivityHelper.getFirmwareRoamingInfo();
         clearBssidBlacklist();
         startConnectivityScan(SCAN_IMMEDIATELY);
+        mRunning = true;
     }
 
     /**
      * Stop and reset WifiConnectivityManager
      */
     private void stop() {
+        if (!mRunning) return;
+        mRunning = false;
         stopConnectivityScan();
         clearBssidBlacklist();
         resetLastPeriodicSingleScanTimeStamp();
@@ -1417,7 +1434,6 @@ public class WifiConnectivityManager {
 
         mWifiEnabled = enable;
         updateRunningState();
-
     }
 
     /**
diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java
index 24e125a29..8e367e92f 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -533,7 +533,7 @@ public class WifiInjector {
                 mWifiNetworkSelector, mWifiConnectivityHelper,
                 mWifiLastResortWatchdog, mOpenNetworkNotifier, mCarrierNetworkNotifier,
                 mCarrierNetworkConfig, mWifiMetrics, mWifiCoreHandlerThread.getLooper(),
-                mClock, mConnectivityLocalLog, true,
+                mClock, mConnectivityLocalLog,
                 mSavedNetworkEvaluator, mScoredNetworkEvaluator, mPasspointNetworkEvaluator);
     }
 
diff --git a/service/java/com/android/server/wifi/WifiNetworkFactory.java b/service/java/com/android/server/wifi/WifiNetworkFactory.java
new file mode 100644
index 000000000..8bc2dab2a
--- /dev/null
+++ b/service/java/com/android/server/wifi/WifiNetworkFactory.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.server.wifi;
+
+import android.content.Context;
+import android.net.NetworkCapabilities;
+import android.net.NetworkFactory;
+import android.net.NetworkRequest;
+import android.os.Looper;
+import android.util.Log;
+
+import java.io.FileDescriptor;
+import java.io.PrintWriter;
+
+/**
+ * Network factory to handle trusted wifi network requests.
+ */
+public class WifiNetworkFactory extends NetworkFactory {
+    private static final String TAG = "WifiNetworkFactory";
+    private static final int SCORE_FILTER = 60;
+
+    private final WifiConnectivityManager mWifiConnectivityManager;
+    private int mConnectionReqCount = 0;
+
+    public WifiNetworkFactory(Looper l, Context c, NetworkCapabilities f,
+                              WifiConnectivityManager connectivityManager) {
+        super(l, c, TAG, f);
+        mWifiConnectivityManager = connectivityManager;
+
+        setScoreFilter(SCORE_FILTER);
+    }
+
+    @Override
+    protected void needNetworkFor(NetworkRequest networkRequest, int score) {
+        if (++mConnectionReqCount == 1) {
+            mWifiConnectivityManager.setTrustedConnectionAllowed(true);
+        }
+    }
+
+    @Override
+    protected void releaseNetworkFor(NetworkRequest networkRequest) {
+        if (mConnectionReqCount == 0) {
+            Log.e(TAG, "No valid network request to release");
+            return;
+        }
+        if (--mConnectionReqCount == 0) {
+            mWifiConnectivityManager.setTrustedConnectionAllowed(false);
+        }
+    }
+
+    @Override
+    public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
+        super.dump(fd, pw, args);
+        pw.println(TAG + ": mConnectionReqCount " + mConnectionReqCount);
+    }
+
+    /**
+     * Check if there is at-least one connection request.
+     */
+    public boolean hasConnectionRequests() {
+        return mConnectionReqCount > 0;
+    }
+}
+