[DSRM] Fix for failing to perform data stall recovery soon after

       skipping data recovery action under poor network signal.

Symptom: If a data stall recovery is skipped under poor
         network signal strength for the very first time.
	 Then in the next iteration, data stall recovery is
	 skipped due to mIsValidNetwork flag remains false always.

Solution: Refactor the existing conditions for data stall recovery.

Bug: 249687945
Test: Manual test passed. See the correct time in log.
      Telephony sanity test passed. Result: b/270503972.

Signed-off-by: Diaesh Antony <diaeshantony@google.com>
Change-Id: I16abe882dfc965b7f48b37d79e5e678efbc490b0
Merged-In: I16abe882dfc965b7f48b37d79e5e678efbc490b0

2 files changed, 65 insertions, 11 deletions

diff --git a/src/java/com/android/internal/telephony/data/DataStallRecoveryManager.java b/src/java/com/android/internal/telephony/data/DataStallRecoveryManager.java
index 0feb1ef3d8..12c0ef5575 100644
--- a/src/java/com/android/internal/telephony/data/DataStallRecoveryManager.java
+++ b/src/java/com/android/internal/telephony/data/DataStallRecoveryManager.java
@@ -151,6 +151,8 @@ public class DataStallRecoveryManager extends Handler {
     private @ElapsedRealtimeLong long mTimeLastRecoveryStartMs;
     /** Whether current network is good or not */
     private boolean mIsValidNetwork;
+    /** Whether data stall recovery is triggered or not */
+    private boolean mRecoveryTriggered = false;
     /** Whether data stall happened or not. */
     private boolean mDataStalled;
     /** Whether the result of last action(RADIO_RESTART) reported. */
@@ -353,6 +355,7 @@ public class DataStallRecoveryManager extends Handler {
      */
     private void reset() {
         mIsValidNetwork = true;
+        mRecoveryTriggered = false;
         mIsAttemptedAllSteps = false;
         mRadioStateChangedDuringDataStall = false;
         mIsAirPlaneModeEnableDuringDataStall = false;
@@ -374,15 +377,12 @@ public class DataStallRecoveryManager extends Handler {
         setNetworkValidationState(isValid);
         if (isValid) {
             reset();
-        } else {
-            if (mIsValidNetwork || isRecoveryAlreadyStarted()) {
-                mIsValidNetwork = false;
-                if (isRecoveryNeeded(true)) {
-                    log("trigger data stall recovery");
-                    mTimeLastRecoveryStartMs = SystemClock.elapsedRealtime();
-                    sendMessage(obtainMessage(EVENT_DO_RECOVERY));
-                }
-            }
+        } else if (isRecoveryNeeded(true)) {
+            // Set the network as invalid, because recovery is needed
+            mIsValidNetwork = false;
+            log("trigger data stall recovery");
+            mTimeLastRecoveryStartMs = SystemClock.elapsedRealtime();
+            sendMessage(obtainMessage(EVENT_DO_RECOVERY));
         }
     }
 
@@ -456,7 +456,7 @@ public class DataStallRecoveryManager extends Handler {
      * @return {@code true} if recovery already started, {@code false} recovery not started.
      */
     private boolean isRecoveryAlreadyStarted() {
-        return getRecoveryAction() != RECOVERY_ACTION_GET_DATA_CALL_LIST;
+        return getRecoveryAction() != RECOVERY_ACTION_GET_DATA_CALL_LIST || mRecoveryTriggered;
     }
 
     /**
@@ -543,6 +543,12 @@ public class DataStallRecoveryManager extends Handler {
     private boolean isRecoveryNeeded(boolean isNeedToCheckTimer) {
         logv("enter: isRecoveryNeeded()");
 
+        // Skip if network is invalid and recovery was not started yet
+        if (!mIsValidNetwork && !isRecoveryAlreadyStarted()) {
+            logl("skip when network still remains invalid and recovery was not started yet");
+            return false;
+        }
+
         // Skip recovery if we have already attempted all steps.
         if (mIsAttemptedAllSteps) {
             logl("skip retrying continue recovery action");
@@ -578,7 +584,6 @@ public class DataStallRecoveryManager extends Handler {
             logl("skip data stall recovery as data not connected");
             return false;
         }
-
         return true;
     }
 
@@ -671,6 +676,7 @@ public class DataStallRecoveryManager extends Handler {
     private void doRecovery() {
         @RecoveryAction final int recoveryAction = getRecoveryAction();
         final int signalStrength = mPhone.getSignalStrength().getLevel();
+        mRecoveryTriggered = true;
 
         // DSRM used sendMessageDelayed to process the next event EVENT_DO_RECOVERY, so it need
         // to check the condition if DSRM need to process the recovery action.
diff --git a/tests/telephonytests/src/com/android/internal/telephony/data/DataStallRecoveryManagerTest.java b/tests/telephonytests/src/com/android/internal/telephony/data/DataStallRecoveryManagerTest.java
index 7b76379d45..313240fd30 100644
--- a/tests/telephonytests/src/com/android/internal/telephony/data/DataStallRecoveryManagerTest.java
+++ b/tests/telephonytests/src/com/android/internal/telephony/data/DataStallRecoveryManagerTest.java
@@ -352,6 +352,54 @@ public class DataStallRecoveryManagerTest extends TelephonyTest {
     }
 
     @Test
+    public void testNextRecoveryAfterSkippingUnderPoorSignal() throws Exception {
+        // Test to validate if the next recovery action is performed in good signal
+        // soon after skipping the recovery action under poor signal condition
+        sendOnInternetDataNetworkCallback(true);
+        mDataStallRecoveryManager.setRecoveryAction(1);
+        doReturn(1).when(mSignalStrength).getLevel();
+        doReturn(mSignalStrength).when(mPhone).getSignalStrength();
+        doReturn(PhoneConstants.State.IDLE).when(mPhone).getState();
+
+        logd("Sending validation failed callback");
+        sendValidationStatusCallback(NetworkAgent.VALIDATION_STATUS_NOT_VALID);
+        processAllMessages();
+        moveTimeForward(101);
+
+        // verify skipping recovery action under poor signal condition
+        assertThat(mDataStallRecoveryManager.getRecoveryAction()).isEqualTo(1);
+
+        // Set the signal condition to good
+        doReturn(3).when(mSignalStrength).getLevel();
+
+        logd("Sending validation failed callback");
+        sendValidationStatusCallback(NetworkAgent.VALIDATION_STATUS_NOT_VALID);
+        processAllMessages();
+        moveTimeForward(101);
+
+        // verify next recovery action is performed under good signal condition
+        assertThat(mDataStallRecoveryManager.getRecoveryAction()).isEqualTo(3);
+    }
+
+    @Test
+    public void testDoNotRecoveryForAlwaysInvalidNetwork() throws Exception {
+        // Test to verify that recovery action is not performed for always invalid network
+        // In some lab testing scenarios, n/w validation always remain invalid.
+        sendOnInternetDataNetworkCallback(false);
+        doReturn(mSignalStrength).when(mPhone).getSignalStrength();
+        doReturn(PhoneConstants.State.IDLE).when(mPhone).getState();
+        mDataStallRecoveryManager
+                .setRecoveryAction(DataStallRecoveryManager.RECOVERY_ACTION_GET_DATA_CALL_LIST);
+
+        logd("Sending validation failed callback");
+        sendValidationStatusCallback(NetworkAgent.VALIDATION_STATUS_NOT_VALID);
+        processAllFutureMessages();
+        moveTimeForward(101);
+
+        assertThat(mDataStallRecoveryManager.getRecoveryAction()).isEqualTo(0);
+    }
+
+    @Test
     public void testStartTimeNotZero() throws Exception {
         sendOnInternetDataNetworkCallback(false);
         doReturn(mSignalStrength).when(mPhone).getSignalStrength();