WifiConnectivityManager: Refactor instantiation in ClientModeImpl

Create instance of WifiConnectivityManager in the constructor of
ClientModeImpl to avoid the null checks throughout the class. This
CL does not fix the ugly cyclic dependencies between these classes. That
is more involved and will be taken up later. This CL makes the
refactoring of WifiNetworkFactory from ClientModeImpl easier (next CL up
the stack).

Bug: 116218227
Test: Unit tests
Test: Device boots up & auto-connects to saved wifi networks.
Change-Id: Ib7b321482bf3f2eff9707e82a64da02513071e8e

5 files changed, 65 insertions, 73 deletions

diff --git a/service/java/com/android/server/wifi/ClientModeImpl.java b/service/java/com/android/server/wifi/ClientModeImpl.java
index 7781805b2..077edde1d 100644
--- a/service/java/com/android/server/wifi/ClientModeImpl.java
+++ b/service/java/com/android/server/wifi/ClientModeImpl.java
@@ -172,13 +172,13 @@ public class ClientModeImpl extends StateMachine {
     protected void log(String s) {
         Log.d(getName(), s);
     }
-    private WifiMetrics mWifiMetrics;
-    private WifiInjector mWifiInjector;
-    private WifiMonitor mWifiMonitor;
-    private WifiNative mWifiNative;
-    private WifiPermissionsUtil mWifiPermissionsUtil;
-    private WifiConfigManager mWifiConfigManager;
-    private WifiConnectivityManager mWifiConnectivityManager;
+    private final WifiMetrics mWifiMetrics;
+    private final WifiInjector mWifiInjector;
+    private final WifiMonitor mWifiMonitor;
+    private final WifiNative mWifiNative;
+    private final WifiPermissionsUtil mWifiPermissionsUtil;
+    private final WifiConfigManager mWifiConfigManager;
+    private final WifiConnectivityManager mWifiConnectivityManager;
     private ConnectivityManager mCm;
     private BaseWifiDiagnostics mWifiDiagnostics;
     private final boolean mP2pSupported;
@@ -791,6 +791,7 @@ public class ClientModeImpl extends StateMachine {
         mWifiInfo = new ExtendedWifiInfo();
         mSupplicantStateTracker =
                 mFacade.makeSupplicantStateTracker(context, mWifiConfigManager, getHandler());
+        mWifiConnectivityManager = mWifiInjector.makeWifiConnectivityManager(this);
 
         mLinkProperties = new LinkProperties();
         mMcastLockManagerFilterController = new McastLockManagerFilterController();
@@ -1937,11 +1938,7 @@ public class ClientModeImpl extends StateMachine {
         mWifiDiagnostics.captureBugReportData(WifiDiagnostics.REPORT_REASON_USER_ACTION);
         mWifiDiagnostics.dump(fd, pw, args);
         dumpIpClient(fd, pw, args);
-        if (mWifiConnectivityManager != null) {
-            mWifiConnectivityManager.dump(fd, pw, args);
-        } else {
-            pw.println("mWifiConnectivityManager is not initialized");
-        }
+        mWifiConnectivityManager.dump(fd, pw, args);
         mWifiInjector.getWakeupController().dump(fd, pw, args);
     }
 
@@ -2430,9 +2427,7 @@ public class ClientModeImpl extends StateMachine {
 
         mWifiMetrics.setScreenState(screenOn);
 
-        if (mWifiConnectivityManager != null) {
-            mWifiConnectivityManager.handleScreenStateChanged(screenOn);
-        }
+        mWifiConnectivityManager.handleScreenStateChanged(screenOn);
 
         if (mVerboseLoggingEnabled) log("handleScreenStateChanged Exit: " + screenOn);
     }
@@ -3114,7 +3109,7 @@ public class ClientModeImpl extends StateMachine {
         protected void needNetworkFor(NetworkRequest networkRequest, int score) {
             synchronized (mWifiReqCountLock) {
                 if (++mConnectionReqCount == 1) {
-                    if (mWifiConnectivityManager != null && mUntrustedReqCount == 0) {
+                    if (mUntrustedReqCount == 0) {
                         mWifiConnectivityManager.enable(true);
                     }
                 }
@@ -3125,7 +3120,7 @@ public class ClientModeImpl extends StateMachine {
         protected void releaseNetworkFor(NetworkRequest networkRequest) {
             synchronized (mWifiReqCountLock) {
                 if (--mConnectionReqCount == 0) {
-                    if (mWifiConnectivityManager != null && mUntrustedReqCount == 0) {
+                    if (mUntrustedReqCount == 0) {
                         mWifiConnectivityManager.enable(false);
                     }
                 }
@@ -3150,12 +3145,10 @@ public class ClientModeImpl extends StateMachine {
                     NetworkCapabilities.NET_CAPABILITY_TRUSTED)) {
                 synchronized (mWifiReqCountLock) {
                     if (++mUntrustedReqCount == 1) {
-                        if (mWifiConnectivityManager != null) {
-                            if (mConnectionReqCount == 0) {
-                                mWifiConnectivityManager.enable(true);
-                            }
-                            mWifiConnectivityManager.setUntrustedConnectionAllowed(true);
+                        if (mConnectionReqCount == 0) {
+                            mWifiConnectivityManager.enable(true);
                         }
+                        mWifiConnectivityManager.setUntrustedConnectionAllowed(true);
                     }
                 }
             }
@@ -3167,11 +3160,9 @@ public class ClientModeImpl extends StateMachine {
                     NetworkCapabilities.NET_CAPABILITY_TRUSTED)) {
                 synchronized (mWifiReqCountLock) {
                     if (--mUntrustedReqCount == 0) {
-                        if (mWifiConnectivityManager != null) {
-                            mWifiConnectivityManager.setUntrustedConnectionAllowed(false);
-                            if (mConnectionReqCount == 0) {
-                                mWifiConnectivityManager.enable(false);
-                            }
+                        mWifiConnectivityManager.setUntrustedConnectionAllowed(false);
+                        if (mConnectionReqCount == 0) {
+                            mWifiConnectivityManager.enable(false);
                         }
                     }
                 }
@@ -3684,17 +3675,6 @@ public class ClientModeImpl extends StateMachine {
         setHighPerfModeEnabled(false);
 
         mWifiStateTracker.updateState(WifiStateTracker.INVALID);
-
-        if (mWifiConnectivityManager == null) {
-            synchronized (mWifiReqCountLock) {
-                mWifiConnectivityManager =
-                        mWifiInjector.makeWifiConnectivityManager(mWifiInfo,
-                                                                  hasConnectionRequests());
-                mWifiConnectivityManager.setUntrustedConnectionAllowed(mUntrustedReqCount > 0);
-                mWifiConnectivityManager.handleScreenStateChanged(mScreenOn);
-            }
-        }
-
         mIpClient = mFacade.makeIpClient(mContext, mInterfaceName, new IpClientCallback());
         mIpClient.setMulticastFilter(true);
         registerForWifiMonitorEvents();
diff --git a/service/java/com/android/server/wifi/WifiConnectivityManager.java b/service/java/com/android/server/wifi/WifiConnectivityManager.java
index 59dabdd63..07d9cd393 100644
--- a/service/java/com/android/server/wifi/WifiConnectivityManager.java
+++ b/service/java/com/android/server/wifi/WifiConnectivityManager.java
@@ -18,6 +18,7 @@ package com.android.server.wifi;
 
 import static android.net.wifi.WifiConfiguration.NetworkSelectionStatus.DISABLED_NO_INTERNET_TEMPORARY;
 
+import static com.android.internal.util.Preconditions.checkNotNull;
 import static com.android.server.wifi.ClientModeImpl.WIFI_WORK_SOURCE;
 
 import android.app.AlarmManager;
@@ -131,7 +132,7 @@ public class WifiConnectivityManager {
     private static final String TAG = "WifiConnectivityManager";
 
     private final ClientModeImpl mStateMachine;
-    private final WifiScanner mScanner;
+    private final WifiInjector mWifiInjector;
     private final WifiConfigManager mConfigManager;
     private final WifiInfo mWifiInfo;
     private final WifiConnectivityHelper mConnectivityHelper;
@@ -147,10 +148,11 @@ public class WifiConnectivityManager {
     private final ScoringParams mScoringParams;
     private final LocalLog mLocalLog;
     private final LinkedList<Long> mConnectionAttemptTimeStamps;
+    private WifiScanner mScanner;
 
     private boolean mDbg = false;
     private boolean mWifiEnabled = false;
-    private boolean mWifiConnectivityManagerEnabled = true;
+    private boolean mWifiConnectivityManagerEnabled = false;
     private boolean mScreenOn = false;
     private int mWifiState = WIFI_STATE_UNKNOWN;
     private boolean mUntrustedConnectionAllowed = false;
@@ -570,17 +572,17 @@ public class WifiConnectivityManager {
      */
     WifiConnectivityManager(Context context, ScoringParams scoringParams,
             ClientModeImpl stateMachine,
-            WifiScanner scanner, WifiConfigManager configManager, WifiInfo wifiInfo,
+            WifiInjector injector, WifiConfigManager configManager, WifiInfo wifiInfo,
             WifiNetworkSelector networkSelector, WifiConnectivityHelper connectivityHelper,
             WifiLastResortWatchdog wifiLastResortWatchdog, OpenNetworkNotifier openNetworkNotifier,
             CarrierNetworkNotifier carrierNetworkNotifier,
             CarrierNetworkConfig carrierNetworkConfig, WifiMetrics wifiMetrics, Looper looper,
-            Clock clock, LocalLog localLog, boolean enable, FrameworkFacade frameworkFacade,
+            Clock clock, LocalLog localLog, boolean enable,
             SavedNetworkEvaluator savedNetworkEvaluator,
             ScoredNetworkEvaluator scoredNetworkEvaluator,
             PasspointNetworkEvaluator passpointNetworkEvaluator) {
         mStateMachine = stateMachine;
-        mScanner = scanner;
+        mWifiInjector = injector;
         mConfigManager = configManager;
         mWifiInfo = wifiInfo;
         mNetworkSelector = networkSelector;
@@ -644,9 +646,6 @@ public class WifiConnectivityManager {
         mNetworkSelector.registerNetworkEvaluator(scoredNetworkEvaluator,
                 SCORED_NETWORK_EVALUATOR_PRIORITY);
 
-        // Register for all single scan results
-        mScanner.registerScanListener(mAllSingleScanListener);
-
         // Listen to WifiConfigManager network update events
         mConfigManager.setOnSavedNetworkUpdateListener(new OnSavedNetworkUpdateListener());
 
@@ -1351,6 +1350,19 @@ public class WifiConnectivityManager {
     }
 
     /**
+     * Helper method to populate WifiScanner handle. This is done lazily because
+     * WifiScanningService is started after WifiService.
+     */
+    private void retrieveWifiScanner() {
+        if (mScanner != null) return;
+        mScanner = mWifiInjector.getWifiScanner();
+        checkNotNull(mScanner);
+        // Register for all single scan results
+        mScanner.registerScanListener(mAllSingleScanListener);
+    }
+
+
+    /**
      * Clear the BSSID blacklist
      */
     private void clearBssidBlacklist() {
@@ -1362,6 +1374,7 @@ public class WifiConnectivityManager {
      * Start WifiConnectivityManager
      */
     private void start() {
+        retrieveWifiScanner();
         mConnectivityHelper.getFirmwareRoamingInfo();
         clearBssidBlacklist();
         startConnectivityScan(SCAN_IMMEDIATELY);
diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java
index ba288505e..24e125a29 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -25,7 +25,6 @@ import android.net.NetworkKey;
 import android.net.NetworkScoreManager;
 import android.net.wifi.IWifiScanner;
 import android.net.wifi.IWificond;
-import android.net.wifi.WifiInfo;
 import android.net.wifi.WifiManager;
 import android.net.wifi.WifiNetworkScoreCache;
 import android.net.wifi.WifiScanner;
@@ -95,15 +94,15 @@ public class WifiInjector {
     private final ClientModeImpl mClientModeImpl;
     private final ActiveModeWarden mActiveModeWarden;
     private final WifiSettingsStore mSettingsStore;
-    private final OpenNetworkNotifier mOpenNetworkNotifier;
-    private final CarrierNetworkNotifier mCarrierNetworkNotifier;
+    private OpenNetworkNotifier mOpenNetworkNotifier;
+    private CarrierNetworkNotifier mCarrierNetworkNotifier;
     private final CarrierNetworkConfig mCarrierNetworkConfig;
     private final WifiLockManager mLockManager;
     private final WifiController mWifiController;
     private final WificondControl mWificondControl;
     private final Clock mClock = new Clock();
     private final WifiMetrics mWifiMetrics;
-    private final WifiLastResortWatchdog mWifiLastResortWatchdog;
+    private WifiLastResortWatchdog mWifiLastResortWatchdog;
     private final PropertyService mPropertyService = new SystemPropertyService();
     private final BuildProperties mBuildProperties = new SystemBuildProperties();
     private final KeyStore mKeyStore = KeyStore.getInstance();
@@ -271,14 +270,6 @@ public class WifiInjector {
         mActiveModeWarden = new ActiveModeWarden(this, mContext, clientModeImplLooper,
                 mWifiNative, new DefaultModeManager(mContext, clientModeImplLooper),
                 mBatteryStats);
-        mOpenNetworkNotifier = new OpenNetworkNotifier(mContext,
-                mWifiCoreHandlerThread.getLooper(), mFrameworkFacade, mClock, mWifiMetrics,
-                mWifiConfigManager, mWifiConfigStore, mClientModeImpl,
-                new ConnectToNetworkNotificationBuilder(mContext, mFrameworkFacade));
-        mCarrierNetworkNotifier = new CarrierNetworkNotifier(mContext,
-                mWifiCoreHandlerThread.getLooper(), mFrameworkFacade, mClock, mWifiMetrics,
-                mWifiConfigManager, mWifiConfigStore, mClientModeImpl,
-                new ConnectToNetworkNotificationBuilder(mContext, mFrameworkFacade));
 
         WakeupNotificationFactory wakeupNotificationFactory =
                 new WakeupNotificationFactory(mContext, mFrameworkFacade);
@@ -295,8 +286,6 @@ public class WifiInjector {
                 mSettingsStore, mWifiServiceHandlerThread.getLooper(), mFrameworkFacade,
                 mActiveModeWarden);
         mSelfRecovery = new SelfRecovery(mWifiController, mClock);
-        mWifiLastResortWatchdog = new WifiLastResortWatchdog(mSelfRecovery, mClock,
-                mWifiMetrics, mClientModeImpl, clientModeImplLooper);
         mWifiMulticastLockManager = new WifiMulticastLockManager(
                 mClientModeImpl.getMcastLockManagerFilterController(),
                 BatteryStatsService.getService());
@@ -521,21 +510,30 @@ public class WifiInjector {
     }
 
     /**
-     * Obtain a new instance of WifiConnectivityManager.
+     * Construct a new instance of WifiConnectivityManager & its dependencies.
      *
      * Create and return a new WifiConnectivityManager.
-     * @param wifiInfo WifiInfo object for updating wifi state.
-     * @param hasConnectionRequests boolean indicating if WifiConnectivityManager to start
-     * immediately based on connection requests.
+     * @param clientModeImpl Instance of client mode impl.
+     * TODO(b/116233964): Remove cyclic dependency between WifiConnectivityManager & ClientModeImpl.
      */
-    public WifiConnectivityManager makeWifiConnectivityManager(WifiInfo wifiInfo,
-                                                               boolean hasConnectionRequests) {
+    public WifiConnectivityManager makeWifiConnectivityManager(ClientModeImpl clientModeImpl) {
+        mOpenNetworkNotifier = new OpenNetworkNotifier(mContext,
+                mWifiCoreHandlerThread.getLooper(), mFrameworkFacade, mClock, mWifiMetrics,
+                mWifiConfigManager, mWifiConfigStore, clientModeImpl,
+                new ConnectToNetworkNotificationBuilder(mContext, mFrameworkFacade));
+        mCarrierNetworkNotifier = new CarrierNetworkNotifier(mContext,
+                mWifiCoreHandlerThread.getLooper(), mFrameworkFacade, mClock, mWifiMetrics,
+                mWifiConfigManager, mWifiConfigStore, clientModeImpl,
+                new ConnectToNetworkNotificationBuilder(mContext, mFrameworkFacade));
+        mWifiLastResortWatchdog = new WifiLastResortWatchdog(mSelfRecovery, mClock,
+                mWifiMetrics, clientModeImpl, clientModeImpl.getHandler().getLooper());
         return new WifiConnectivityManager(mContext, getScoringParams(),
-                mClientModeImpl, getWifiScanner(),
-                mWifiConfigManager, wifiInfo, mWifiNetworkSelector, mWifiConnectivityHelper,
+                clientModeImpl, this,
+                mWifiConfigManager, clientModeImpl.getWifiInfo(),
+                mWifiNetworkSelector, mWifiConnectivityHelper,
                 mWifiLastResortWatchdog, mOpenNetworkNotifier, mCarrierNetworkNotifier,
                 mCarrierNetworkConfig, mWifiMetrics, mWifiCoreHandlerThread.getLooper(),
-                mClock, mConnectivityLocalLog, hasConnectionRequests, mFrameworkFacade,
+                mClock, mConnectivityLocalLog, true,
                 mSavedNetworkEvaluator, mScoredNetworkEvaluator, mPasspointNetworkEvaluator);
     }
 
diff --git a/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java b/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java
index 1dade80db..b317e2950 100644
--- a/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java
@@ -399,7 +399,7 @@ public class ClientModeImplTest {
         when(mWifiInjector.getWifiDiagnostics()).thenReturn(mWifiDiagnostics);
         when(mWifiInjector.getWifiConfigManager()).thenReturn(mWifiConfigManager);
         when(mWifiInjector.getWifiScanner()).thenReturn(mWifiScanner);
-        when(mWifiInjector.makeWifiConnectivityManager(any(WifiInfo.class), anyBoolean()))
+        when(mWifiInjector.makeWifiConnectivityManager(any()))
                 .thenReturn(mWifiConnectivityManager);
         when(mWifiInjector.getPasspointManager()).thenReturn(mPasspointManager);
         when(mWifiInjector.getWifiStateTracker()).thenReturn(mWifiStateTracker);
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java b/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java
index 625ed7180..7d0922959 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiConnectivityManagerTest.java
@@ -92,6 +92,7 @@ public class WifiConnectivityManagerTest {
         mWifiScanner = mockWifiScanner();
         mWifiConnectivityHelper = mockWifiConnectivityHelper();
         mWifiNS = mockWifiNetworkSelector();
+        when(mWifiInjector.getWifiScanner()).thenReturn(mWifiScanner);
         mWifiConnectivityManager = createConnectivityManager();
         verify(mWifiConfigManager).setOnSavedNetworkUpdateListener(anyObject());
         mWifiConnectivityManager.setWifiEnabled(true);
@@ -125,7 +126,7 @@ public class WifiConnectivityManagerTest {
     private WifiConfigManager mWifiConfigManager;
     private WifiInfo mWifiInfo;
     private LocalLog mLocalLog;
-    @Mock private FrameworkFacade mFrameworkFacade;
+    @Mock private WifiInjector mWifiInjector;
     @Mock private NetworkScoreManager mNetworkScoreManager;
     @Mock private Clock mClock;
     @Mock private WifiLastResortWatchdog mWifiLastResortWatchdog;
@@ -313,11 +314,11 @@ public class WifiConnectivityManagerTest {
     WifiConnectivityManager createConnectivityManager() {
         return new WifiConnectivityManager(mContext,
                 new ScoringParams(mContext),
-                mClientModeImpl, mWifiScanner,
+                mClientModeImpl, mWifiInjector,
                 mWifiConfigManager, mWifiInfo, mWifiNS, mWifiConnectivityHelper,
                 mWifiLastResortWatchdog, mOpenNetworkNotifier, mCarrierNetworkNotifier,
                 mCarrierNetworkConfig, mWifiMetrics, mLooper.getLooper(), mClock, mLocalLog, true,
-                mFrameworkFacade, null, null, null);
+                null, null, null);
     }
 
     /**