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

5 files changed, 30 insertions, 39 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) {