WifiDiagnostics: Fix alert handling

This is currently broken because |mWifiStateMachine| is always null in
WifiDiagnostics (was broken in a refactor of WifiDiagnostics in early O).

Changes in the CL:
a) Construct WifiDiagnostics directly in WifiInjector's constructor (No need
for makeWifiDiagnostics).
b) Directly handle the alert callback. This is already guaranteed to be
invoked from WifiStateMachine thread. There is no need for the extra hop
through WSM message handling.
c) Don't create 2 instances of WifiDiagnostics in WifiStateMachine &
WifiStateMachinePrime.

Some other related cleanups
1) Removed an unused binder object in WifiInjector.
2) Don't cache JavaRuntime instance in WifiInjector.

Bug: 79222338
Test: Unit tests
Test: Triggered an alert (each disconnect triggers an alert) and
verified that:
i) metrics are incremented, and
ii) dumpsys contains the alert data.

Change-Id: I59b157d57cc7ba234efa2dfe20fc59cc5834df2c

8 files changed, 53 insertions, 43 deletions

diff --git a/service/java/com/android/server/wifi/WifiDiagnostics.java b/service/java/com/android/server/wifi/WifiDiagnostics.java
index 23e6ebe93..412394388 100644
--- a/service/java/com/android/server/wifi/WifiDiagnostics.java
+++ b/service/java/com/android/server/wifi/WifiDiagnostics.java
@@ -16,6 +16,7 @@
 
 package com.android.server.wifi;
 
+import android.annotation.NonNull;
 import android.content.Context;
 import android.util.Base64;
 
@@ -101,30 +102,30 @@ class WifiDiagnostics extends BaseWifiDiagnostics {
     private boolean mIsLoggingEventHandlerRegistered;
     private WifiNative.RingBufferStatus[] mRingBuffers;
     private WifiNative.RingBufferStatus mPerPacketRingBuffer;
-    private WifiStateMachine mWifiStateMachine;
     private final BuildProperties mBuildProperties;
     private final WifiLog mLog;
     private final LastMileLogger mLastMileLogger;
     private final Runtime mJavaRuntime;
+    private final WifiMetrics mWifiMetrics;
     private int mMaxRingBufferSizeBytes;
     private WifiInjector mWifiInjector;
 
     public WifiDiagnostics(Context context, WifiInjector wifiInjector,
-                           WifiStateMachine wifiStateMachine, WifiNative wifiNative,
-                           BuildProperties buildProperties, LastMileLogger lastMileLogger) {
+                           WifiNative wifiNative, BuildProperties buildProperties,
+                           LastMileLogger lastMileLogger) {
         super(wifiNative);
         RING_BUFFER_BYTE_LIMIT_SMALL = context.getResources().getInteger(
                 R.integer.config_wifi_logger_ring_buffer_default_size_limit_kb) * 1024;
         RING_BUFFER_BYTE_LIMIT_LARGE = context.getResources().getInteger(
                 R.integer.config_wifi_logger_ring_buffer_verbose_size_limit_kb) * 1024;
 
-        mWifiStateMachine = wifiStateMachine;
         mBuildProperties = buildProperties;
         mIsLoggingEventHandlerRegistered = false;
         mMaxRingBufferSizeBytes = RING_BUFFER_BYTE_LIMIT_SMALL;
         mLog = wifiInjector.makeLog(TAG);
         mLastMileLogger = lastMileLogger;
         mJavaRuntime = wifiInjector.getJavaRuntime();
+        mWifiMetrics = wifiInjector.getWifiMetrics();
         mWifiInjector = wifiInjector;
     }
 
@@ -409,11 +410,9 @@ class WifiDiagnostics extends BaseWifiDiagnostics {
         }
     }
 
-    synchronized void onWifiAlert(int errorCode, byte[] buffer) {
-        if (mWifiStateMachine != null) {
-            mWifiStateMachine.sendMessage(
-                    WifiStateMachine.CMD_FIRMWARE_ALERT, errorCode, 0, buffer);
-        }
+    synchronized void onWifiAlert(int errorCode, @NonNull byte[] buffer) {
+        captureAlertData(errorCode, buffer);
+        mWifiMetrics.incrementAlertReasonCount(errorCode);
     }
 
     private boolean isVerboseLoggingEnabled() {
@@ -550,6 +549,11 @@ class WifiDiagnostics extends BaseWifiDiagnostics {
         return mLastBugReports;
     }
 
+    @VisibleForTesting
+    LimitedCircularArray<BugReport> getAlertReports() {
+        return mLastAlerts;
+    }
+
     private String compressToBase64(byte[] input) {
         String result;
         //compress
diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java
index 7fc49ece2..358abd546 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -132,12 +132,12 @@ public class WifiInjector {
     private HalDeviceManager mHalDeviceManager;
     private final IBatteryStats mBatteryStats;
     private final WifiStateTracker mWifiStateTracker;
-    private final Runtime mJavaRuntime;
     private final SelfRecovery mSelfRecovery;
     private final WakeupController mWakeupController;
     private final INetworkManagementService mNwManagementService;
     private final ScanRequestProxy mScanRequestProxy;
     private final SarManager mSarManager;
+    private final BaseWifiDiagnostics mWifiDiagnostics;
 
     private final boolean mUseRealLogger;
 
@@ -251,16 +251,20 @@ public class WifiInjector {
                 (ActivityManager) mContext.getSystemService(Context.ACTIVITY_SERVICE),
                 this, mWifiConfigManager,
                 mWifiPermissionsUtil, mWifiMetrics, mClock);
-        // mWifiStateMachine has an implicit dependency on mJavaRuntime due to WifiDiagnostics.
-        mJavaRuntime = Runtime.getRuntime();
         mSarManager = new SarManager(mContext, makeTelephonyManager(), wifiStateMachineLooper,
                 mWifiNative);
+        if (mUseRealLogger) {
+            mWifiDiagnostics = new WifiDiagnostics(
+                    mContext, this, mWifiNative, mBuildProperties,
+                    new LastMileLogger(this));
+        } else {
+            mWifiDiagnostics = new BaseWifiDiagnostics(mWifiNative);
+        }
         mWifiStateMachine = new WifiStateMachine(mContext, mFrameworkFacade,
                 wifiStateMachineLooper, UserManager.get(mContext),
                 this, mBackupManagerProxy, mCountryCode, mWifiNative,
                 new WrongPasswordNotifier(mContext, mFrameworkFacade),
                 mSarManager);
-        IBinder b = mFrameworkFacade.getService(Context.NETWORKMANAGEMENT_SERVICE);
         mWifiStateMachinePrime = new WifiStateMachinePrime(this, mContext, wifiStateMachineLooper,
                 mWifiNative, new DefaultModeManager(mContext, wifiStateMachineLooper),
                 mBatteryStats);
@@ -493,14 +497,8 @@ public class WifiInjector {
         return new LogcatLog(tag);
     }
 
-    public BaseWifiDiagnostics makeWifiDiagnostics(WifiNative wifiNative) {
-        if (mUseRealLogger) {
-            return new WifiDiagnostics(
-                    mContext, this, mWifiStateMachine, wifiNative, mBuildProperties,
-                    new LastMileLogger(this));
-        } else {
-            return new BaseWifiDiagnostics(wifiNative);
-        }
+    public BaseWifiDiagnostics getWifiDiagnostics() {
+        return mWifiDiagnostics;
     }
 
     /**
@@ -578,10 +576,6 @@ public class WifiInjector {
         return mHalDeviceManager;
     }
 
-    public Runtime getJavaRuntime() {
-        return mJavaRuntime;
-    }
-
     public WifiNative getWifiNative() {
         return mWifiNative;
     }
@@ -610,6 +604,10 @@ public class WifiInjector {
         return mScanRequestProxy;
     }
 
+    public Runtime getJavaRuntime() {
+        return Runtime.getRuntime();
+    }
+
     public ActivityManagerService getActivityManagerService() {
         return (ActivityManagerService) ActivityManager.getService();
     }
diff --git a/service/java/com/android/server/wifi/WifiStateMachine.java b/service/java/com/android/server/wifi/WifiStateMachine.java
index 5161a3117..5e49a4c6f 100644
--- a/service/java/com/android/server/wifi/WifiStateMachine.java
+++ b/service/java/com/android/server/wifi/WifiStateMachine.java
@@ -495,9 +495,6 @@ public class WifiStateMachine extends StateMachine {
     /* Get matching network */
     static final int CMD_GET_MATCHING_CONFIG                            = BASE + 99;
 
-    /* alert from firmware */
-    static final int CMD_FIRMWARE_ALERT                                 = BASE + 100;
-
     /* SIM is removed; reset any cached data for it */
     static final int CMD_RESET_SIM_NETWORKS                             = BASE + 101;
 
@@ -801,7 +798,7 @@ public class WifiStateMachine extends StateMachine {
         mPasspointManager = mWifiInjector.getPasspointManager();
 
         mWifiMonitor = mWifiInjector.getWifiMonitor();
-        mWifiDiagnostics = mWifiInjector.makeWifiDiagnostics(mWifiNative);
+        mWifiDiagnostics = mWifiInjector.getWifiDiagnostics();
         mScanRequestProxy = mWifiInjector.getScanRequestProxy();
         mWifiPermissionsWrapper = mWifiInjector.getWifiPermissionsWrapper();
 
@@ -3422,14 +3419,6 @@ public class WifiStateMachine extends StateMachine {
                     int featureSet = mWifiNative.getSupportedFeatureSet(mInterfaceName);
                     replyToMessage(message, message.what, featureSet);
                     break;
-                case CMD_FIRMWARE_ALERT:
-                    if (mWifiDiagnostics != null) {
-                        byte[] buffer = (byte[])message.obj;
-                        int alertReason = message.arg1;
-                        mWifiDiagnostics.captureAlertData(alertReason, buffer);
-                        mWifiMetrics.incrementAlertReasonCount(alertReason);
-                    }
-                    break;
                 case CMD_GET_LINK_LAYER_STATS:
                     // Not supported hence reply with error message
                     replyToMessage(message, message.what, null);
diff --git a/service/java/com/android/server/wifi/WifiStateMachinePrime.java b/service/java/com/android/server/wifi/WifiStateMachinePrime.java
index fdacd1460..3c0a4825b 100644
--- a/service/java/com/android/server/wifi/WifiStateMachinePrime.java
+++ b/service/java/com/android/server/wifi/WifiStateMachinePrime.java
@@ -137,7 +137,7 @@ public class WifiStateMachinePrime {
         mDefaultModeManager = defaultModeManager;
         mBatteryStats = batteryStats;
         mSelfRecovery = mWifiInjector.getSelfRecovery();
-        mWifiDiagnostics = mWifiInjector.makeWifiDiagnostics(mWifiNative);
+        mWifiDiagnostics = mWifiInjector.getWifiDiagnostics();
         mScanRequestProxy = mWifiInjector.getScanRequestProxy();
         mModeStateMachine = new ModeStateMachine();
         mWifiNativeStatusListener = new WifiNativeStatusListener();
diff --git a/service/java/com/android/server/wifi/WifiVendorHal.java b/service/java/com/android/server/wifi/WifiVendorHal.java
index 08d03d7de..1e622ca1b 100644
--- a/service/java/com/android/server/wifi/WifiVendorHal.java
+++ b/service/java/com/android/server/wifi/WifiVendorHal.java
@@ -2886,7 +2886,7 @@ public class WifiVendorHal {
 
         @Override
         public void onDebugErrorAlert(int errorCode, java.util.ArrayList<Byte> debugData) {
-            mVerboseLog.d("onDebugErrorAlert " + errorCode);
+            mLog.w("onDebugErrorAlert " + errorCode);
             mHalEventHandler.post(() -> {
                 WifiNative.WifiLoggerEventHandler eventHandler;
                 synchronized (sLock) {
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java b/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java
index 613d30019..d380cc7fd 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java
@@ -59,7 +59,6 @@ import java.util.regex.Pattern;
  */
 @SmallTest
 public class WifiDiagnosticsTest {
-    @Mock WifiStateMachine mWsm;
     @Mock WifiNative mWifiNative;
     @Mock BuildProperties mBuildProperties;
     @Mock Context mContext;
@@ -69,6 +68,7 @@ public class WifiDiagnosticsTest {
     @Mock Runtime mJavaRuntime;
     @Mock Process mExternalProcess;
     @Mock ActivityManagerService mActivityManagerService;
+    @Mock WifiMetrics mWifiMetrics;
     WifiDiagnostics mWifiDiagnostics;
 
     private static final String FAKE_RING_BUFFER_NAME = "fake-ring-buffer";
@@ -76,6 +76,8 @@ public class WifiDiagnosticsTest {
     private static final int LARGE_RING_BUFFER_SIZE_KB = 1024;
     private static final int BYTES_PER_KBYTE = 1024;
     private static final long FAKE_CONNECTION_ID = 1;
+    private static final int ALERT_REASON_CODE = 1;
+    private static final byte[] ALERT_DATA = {0 , 4, 5};
 
     private WifiNative.RingBufferStatus mFakeRbs;
     /**
@@ -118,9 +120,10 @@ public class WifiDiagnosticsTest {
         when(mWifiInjector.makeLog(anyString())).thenReturn(mLog);
         when(mWifiInjector.getJavaRuntime()).thenReturn(mJavaRuntime);
         when(mWifiInjector.getActivityManagerService()).thenReturn(mActivityManagerService);
+        when(mWifiInjector.getWifiMetrics()).thenReturn(mWifiMetrics);
 
         mWifiDiagnostics = new WifiDiagnostics(
-                mContext, mWifiInjector, mWsm, mWifiNative, mBuildProperties, mLastMileLogger);
+                mContext, mWifiInjector, mWifiNative, mBuildProperties, mLastMileLogger);
         mWifiNative.enableVerboseLogging(0);
     }
 
@@ -699,6 +702,22 @@ public class WifiDiagnosticsTest {
         assertEquals(0, getLoggerRingBufferData().length);
     }
 
+    /**
+     * Verifies that we capture a bugreport & store alert data when WifiNative invokes
+     * the alert callback.
+     */
+    @Test
+    public void onWifiAlertCapturesBugreportAndIncrementsMetrics() throws Exception {
+        mWifiDiagnostics.onWifiAlert(ALERT_REASON_CODE, ALERT_DATA);
+
+        assertEquals(1, mWifiDiagnostics.getAlertReports().size());
+        WifiDiagnostics.BugReport alertReport = mWifiDiagnostics.getAlertReports().get(0);
+        assertEquals(ALERT_REASON_CODE, alertReport.errorCode);
+        assertArrayEquals(ALERT_DATA, alertReport.alertData);
+
+        verify(mWifiMetrics).incrementAlertReasonCount(ALERT_REASON_CODE);
+    }
+
     /** Verifies that we skip the firmware and driver dumps if verbose is not enabled. */
     @Test
     public void captureBugReportSkipsFirmwareAndDriverDumpsByDefault() {
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiStateMachinePrimeTest.java b/tests/wifitests/src/com/android/server/wifi/WifiStateMachinePrimeTest.java
index ec1c0e8fe..0413dbc44 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiStateMachinePrimeTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiStateMachinePrimeTest.java
@@ -86,7 +86,7 @@ public class WifiStateMachinePrimeTest {
         mLooper = new TestLooper();
 
         when(mWifiInjector.getSelfRecovery()).thenReturn(mSelfRecovery);
-        when(mWifiInjector.makeWifiDiagnostics(eq(mWifiNative))).thenReturn(mWifiDiagnostics);
+        when(mWifiInjector.getWifiDiagnostics()).thenReturn(mWifiDiagnostics);
         when(mWifiInjector.getScanRequestProxy()).thenReturn(mScanRequestProxy);
 
         mWifiStateMachinePrime = createWifiStateMachinePrime();
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java
index b33b0b59f..7c0739b83 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiStateMachineTest.java
@@ -393,7 +393,7 @@ public class WifiStateMachineTest {
         when(mWifiInjector.getBuildProperties()).thenReturn(mBuildProperties);
         when(mWifiInjector.getKeyStore()).thenReturn(mock(KeyStore.class));
         when(mWifiInjector.getWifiBackupRestore()).thenReturn(mock(WifiBackupRestore.class));
-        when(mWifiInjector.makeWifiDiagnostics(anyObject())).thenReturn(mWifiDiagnostics);
+        when(mWifiInjector.getWifiDiagnostics()).thenReturn(mWifiDiagnostics);
         when(mWifiInjector.getWifiConfigManager()).thenReturn(mWifiConfigManager);
         when(mWifiInjector.getWifiScanner()).thenReturn(mWifiScanner);
         when(mWifiInjector.makeWifiConnectivityManager(any(WifiInfo.class), anyBoolean()))