Always trigger safe mode callbacks

This change ensures that safe mode callbacks are always triggered from
the VcnGatewayConnection up, ensuring that if a gateway connection
is restarted and successfully validates, it will notify the Vcn class,
and kick it out of safe mode.

Previously, if a VCN had entered safe mode, and a config was updated,
the newly updated VcnGatewayConnection would be brought up as "not" in
safe mode, and thus the onSafeModeStatusChanged() callback would never
be fired if the new VcnGatewayConnection successfully connected.

Calling onSafeModeStatusChanged() multiple times is safe, and serves as
a "connection status changed" callback.

Bug: 196841140
Test: atest FrameworksVcnTests
Test: manual testing
Change-Id: I43c82ada9e9429e8283be65ab73d13075dfd6038

4 files changed, 24 insertions, 11 deletions

diff --git a/services/core/java/com/android/server/vcn/Vcn.java b/services/core/java/com/android/server/vcn/Vcn.java
index 382398a210bb..e0cc8e182079 100644
--- a/services/core/java/com/android/server/vcn/Vcn.java
+++ b/services/core/java/com/android/server/vcn/Vcn.java
@@ -352,7 +352,7 @@ public class Vcn extends Handler {
     }
 
     private void handleSafeModeStatusChanged() {
-        logDbg("VcnGatewayConnection safe mode status changed");
+        logVdbg("VcnGatewayConnection safe mode status changed");
         boolean hasSafeModeGatewayConnection = false;
 
         // If any VcnGatewayConnection is in safe mode, mark the entire VCN as being in safe mode
@@ -368,7 +368,7 @@ public class Vcn extends Handler {
                 hasSafeModeGatewayConnection ? VCN_STATUS_CODE_SAFE_MODE : VCN_STATUS_CODE_ACTIVE;
         if (oldStatus != mCurrentStatus) {
             mVcnCallback.onSafeModeStatusChanged(hasSafeModeGatewayConnection);
-            logDbg(
+            logInfo(
                     "Safe mode "
                             + (mCurrentStatus == VCN_STATUS_CODE_SAFE_MODE ? "entered" : "exited"));
         }
@@ -539,6 +539,16 @@ public class Vcn extends Handler {
         Slog.d(TAG, getLogPrefix() + msg, tr);
     }
 
+    private void logInfo(String msg) {
+        Slog.i(TAG, getLogPrefix() + msg);
+        LOCAL_LOG.log(getLogPrefix() + "INFO: " + msg);
+    }
+
+    private void logInfo(String msg, Throwable tr) {
+        Slog.i(TAG, getLogPrefix() + msg, tr);
+        LOCAL_LOG.log(getLogPrefix() + "INFO: " + msg + tr);
+    }
+
     private void logErr(String msg) {
         Slog.e(TAG, getLogPrefix() + msg);
         LOCAL_LOG.log(getLogPrefix() + "ERR: " + msg);
diff --git a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java
index 450257fcdecb..7dec4e785f5c 100644
--- a/services/core/java/com/android/server/vcn/VcnGatewayConnection.java
+++ b/services/core/java/com/android/server/vcn/VcnGatewayConnection.java
@@ -1677,10 +1677,8 @@ public class VcnGatewayConnection extends StateMachine {
             mFailedAttempts = 0;
             cancelSafeModeAlarm();
 
-            if (mIsInSafeMode) {
-                mIsInSafeMode = false;
-                mGatewayStatusCallback.onSafeModeStatusChanged();
-            }
+            mIsInSafeMode = false;
+            mGatewayStatusCallback.onSafeModeStatusChanged();
         }
 
         protected void applyTransform(
diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java
index 0f84f6ebe522..c9a8947ab5ef 100644
--- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java
+++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionConnectedStateTest.java
@@ -322,6 +322,7 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection
         triggerValidation(NetworkAgent.VALIDATION_STATUS_VALID);
         verify(mSafeModeTimeoutAlarm).cancel();
         assertFalse(mGatewayConnection.isInSafeMode());
+        verifySafeModeStateAndCallbackFired(1 /* invocationCount */, false /* isInSafeMode */);
     }
 
     @Test
@@ -391,6 +392,7 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection
 
         triggerValidation(NetworkAgent.VALIDATION_STATUS_VALID);
 
+        verifySafeModeStateAndCallbackFired(2 /* invocationCount */, false /* isInSafeMode */);
         assertFalse(mGatewayConnection.isInSafeMode());
     }
 
@@ -400,7 +402,7 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection
         mTestLooper.dispatchAll();
 
         triggerValidation(NetworkAgent.VALIDATION_STATUS_VALID);
-        assertFalse(mGatewayConnection.isInSafeMode());
+        verifySafeModeStateAndCallbackFired(1 /* invocationCount */, false /* isInSafeMode */);
 
         // Trigger a failed validation, and the subsequent safemode timeout.
         triggerValidation(NetworkAgent.VALIDATION_STATUS_NOT_VALID);
@@ -416,7 +418,7 @@ public class VcnGatewayConnectionConnectedStateTest extends VcnGatewayConnection
         runnableCaptor.getValue().run();
         mTestLooper.dispatchAll();
 
-        assertTrue(mGatewayConnection.isInSafeMode());
+        verifySafeModeStateAndCallbackFired(2 /* invocationCount */, true /* isInSafeMode */);
     }
 
     private Consumer<VcnNetworkAgent> setupNetworkAndGetUnwantedCallback() {
diff --git a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java
index a696b3ae28f7..64d0bca15ce9 100644
--- a/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java
+++ b/tests/vcn/java/com/android/server/vcn/VcnGatewayConnectionTestBase.java
@@ -23,7 +23,6 @@ import static com.android.server.vcn.VcnTestUtils.setupIpSecManager;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.CALLS_REAL_METHODS;
@@ -301,6 +300,11 @@ public class VcnGatewayConnectionTestBase {
                 expectCanceled);
     }
 
+    protected void verifySafeModeStateAndCallbackFired(int invocationCount, boolean isInSafeMode) {
+        verify(mGatewayStatusCallback, times(invocationCount)).onSafeModeStatusChanged();
+        assertEquals(isInSafeMode, mGatewayConnection.isInSafeMode());
+    }
+
     protected void verifySafeModeTimeoutNotifiesCallbackAndUnregistersNetworkAgent(
             @NonNull State expectedState) {
         // Set a VcnNetworkAgent, and expect it to be unregistered and cleared
@@ -314,9 +318,8 @@ public class VcnGatewayConnectionTestBase {
         delayedEvent.run();
         mTestLooper.dispatchAll();
 
-        verify(mGatewayStatusCallback).onSafeModeStatusChanged();
         assertEquals(expectedState, mGatewayConnection.getCurrentState());
-        assertTrue(mGatewayConnection.isInSafeMode());
+        verifySafeModeStateAndCallbackFired(1, true);
 
         verify(mockNetworkAgent).unregister();
         assertNull(mGatewayConnection.getNetworkAgent());