Snap for 7798373 from f90e9055 to mainline-tethering-release

Change-Id: I8f8ed301cb3c47747b65306c714340a0a4ef606f

common/moduleutils/src/android/net/shared/NetworkMonitorUtils.java

@@ -83,10 +83,6 @@ public class NetworkMonitorUtils {
             return true;
         }
 
-        // TODO: once TRANSPORT_TEST is @SystemApi in S and S SDK is stable (so constant shims can
-        // be replaced with the SDK constant that will be inlined), replace isTestNetwork with
-        // hasTransport(TRANSPORT_TEST)
-
         // Test networks that also have one of the major transport types are attempting to replicate
         // that transport on a test interface (for example, test ethernet networks with
         // EthernetManager#setIncludeTestInterfaces). Run validation on them for realistic tests.

common/networkstackclient/Android.bp

@@ -121,7 +121,7 @@ aidl_interface {
             enabled: false,
         },
     },
-    imports: ["ipmemorystore-aidl-interfaces"],
+    imports: ["ipmemorystore-aidl-interfaces-V10"],
     versions: [
         "1",
         "2",

src/android/net/ip/IpClient.java

@@ -906,6 +906,7 @@ public class IpClient extends StateMachine {
 
     private void stopStateMachineUpdaters() {
         mObserverRegistry.unregisterObserver(mLinkObserver);
+        mLinkObserver.clearInterfaceParams();
         mLinkObserver.shutdown();
     }
 
@@ -1969,7 +1970,6 @@ public class IpClient extends StateMachine {
             mHasDisabledIpv6OrAcceptRaOnProvLoss = false;
             mGratuitousNaTargetAddresses.clear();
 
-            mLinkObserver.clearInterfaceParams();
             resetLinkProperties();
             if (mStartTimeMillis > 0) {
                 // Completed a life-cycle; send a final empty LinkProperties

tests/integration/src/android/net/ip/IpClientIntegrationTestCommon.java

@@ -146,7 +146,6 @@ import android.system.Os;
 import androidx.annotation.NonNull;
 import androidx.test.InstrumentationRegistry;
 import androidx.test.filters.SmallTest;
-import androidx.test.runner.AndroidJUnit4;
 
 import com.android.internal.util.HexDump;
 import com.android.internal.util.StateMachine;
@@ -181,6 +180,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TestName;
 import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
 import org.mockito.ArgumentCaptor;
 import org.mockito.InOrder;
 import org.mockito.Mock;
@@ -224,7 +224,7 @@ import kotlin.LazyKt;
  *
  * Tests in this class can either be run with signature permissions, or with root access.
  */
-@RunWith(AndroidJUnit4.class)
+@RunWith(Parameterized.class)
 @SmallTest
 public abstract class IpClientIntegrationTestCommon {
     private static final int DATA_BUFFER_LEN = 4096;
@@ -247,6 +247,17 @@ public abstract class IpClientIntegrationTestCommon {
     @Rule
     public final TestName mTestNameRule = new TestName();
 
+    // Indicate whether the flag of parsing netlink event is enabled or not. If it's disabled,
+    // integration test still covers the old codepath(i.e. using NetworkObserver), otherwise,
+    // test goes through the new codepath(i.e. processRtNetlinkxxx).
+    @Parameterized.Parameter(0)
+    public boolean mIsNetlinkEventParseEnabled;
+
+    @Parameterized.Parameters
+    public static Iterable<? extends Object> data() {
+        return Arrays.asList(Boolean.valueOf("false"), Boolean.valueOf("true"));
+    }
+
     /**
      * Indicates that a test requires signature permissions to run.
      *
@@ -553,8 +564,14 @@ public abstract class IpClientIntegrationTestCommon {
 
     @Before
     public void setUp() throws Exception {
-        final Method testMethod = IpClientIntegrationTestCommon.class.getMethod(
-                mTestNameRule.getMethodName());
+        // Suffix "[0]" or "[1]" is added to the end of test method name after running with
+        // Parameterized.class, that's intended behavior, to iterate each test method with the
+        // parameterize value. However, Class#getMethod() throws NoSuchMethodException when
+        // searching the target test method name due to this change. Just keep the original test
+        // method name to fix NoSuchMethodException, and find the correct annotation associated
+        // to test method.
+        final String testMethodName = mTestNameRule.getMethodName().split("\\[")[0];
+        final Method testMethod = IpClientIntegrationTestCommon.class.getMethod(testMethodName);
         mIsSignatureRequiredTest = testMethod.getAnnotation(SignatureRequiredTest.class) != null;
         assumeFalse(testSkipped());
 
@@ -567,6 +584,12 @@ public abstract class IpClientIntegrationTestCommon {
         }
 
         mIIpClient = makeIIpClient(mIfaceName, mCb);
+
+        // Depend on the parameterized value to enable/disable netlink message refactor flag.
+        // Make sure both of the old codepath(rely on the INetdUnsolicitedEventListener aidl)
+        // and new codepath(parse netlink event from kernel) will be executed.
+        setFeatureEnabled(NetworkStackUtils.IPCLIENT_PARSE_NETLINK_EVENTS_VERSION,
+                mIsNetlinkEventParseEnabled /* default value */);
     }
 
     protected void setUpMocks() throws Exception {

tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java

@@ -16,6 +16,7 @@
 
 package com.android.server.connectivity;
 
+import static android.content.Intent.ACTION_CONFIGURATION_CHANGED;
 import static android.net.CaptivePortal.APP_RETURN_DISMISSED;
 import static android.net.CaptivePortal.APP_RETURN_WANTED_AS_IS;
 import static android.net.DnsResolver.TYPE_A;
@@ -637,6 +638,7 @@ public class NetworkMonitorTest {
 
         @Override
         protected void onQuitting() {
+            super.onQuitting();
             assertTrue(mCreatedNetworkMonitors.remove(this));
             mQuitCv.open();
         }
@@ -863,7 +865,6 @@ public class NetworkMonitorTest {
 
     @Test
     public void testGetHttpProbeUrl() {
-        final WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor();
         // If config_captive_portal_http_url is set and the global setting is set, the config is
         // used.
         doReturn(TEST_HTTP_URL).when(mResources).getString(R.string.config_captive_portal_http_url);
@@ -871,16 +872,21 @@ public class NetworkMonitorTest {
                 R.string.default_captive_portal_http_url);
         when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_HTTP_URL), any()))
                 .thenReturn(TEST_HTTP_OTHER_URL1);
-        assertEquals(TEST_HTTP_URL, wnm.getCaptivePortalServerHttpUrl());
+        final WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor();
+        assertEquals(TEST_HTTP_URL, wnm.getCaptivePortalServerHttpUrl(mContext));
         // If config_captive_portal_http_url is unset and the global setting is set, the global
         // setting is used.
         doReturn(null).when(mResources).getString(R.string.config_captive_portal_http_url);
-        assertEquals(TEST_HTTP_OTHER_URL1, wnm.getCaptivePortalServerHttpUrl());
+        assertEquals(TEST_HTTP_OTHER_URL1, wnm.getCaptivePortalServerHttpUrl(mContext));
         // If both config_captive_portal_http_url and global setting are unset,
-        // default_captive_portal_http_url is used.
+        // default_captive_portal_http_url is used. But the global setting will only be read in the
+        // constructor.
         when(mDependencies.getSetting(any(), eq(Settings.Global.CAPTIVE_PORTAL_HTTP_URL), any()))
                 .thenReturn(null);
-        assertEquals(TEST_HTTP_OTHER_URL2, wnm.getCaptivePortalServerHttpUrl());
+        assertEquals(TEST_HTTP_OTHER_URL1, wnm.getCaptivePortalServerHttpUrl(mContext));
+        // default_captive_portal_http_url is used when the configuration is applied in new NM.
+        final WrappedNetworkMonitor wnm2 = makeCellNotMeteredNetworkMonitor();
+        assertEquals(TEST_HTTP_OTHER_URL2, wnm2.getCaptivePortalServerHttpUrl(mContext));
     }
 
     @Test
@@ -937,6 +943,57 @@ public class NetworkMonitorTest {
         }
     }
 
+    @Test
+    public void testConfigurationChange_BeforeNMConnected() throws Exception {
+        final WrappedNetworkMonitor nm = new WrappedNetworkMonitor();
+        final ArgumentCaptor<BroadcastReceiver> receiverCaptor =
+                ArgumentCaptor.forClass(BroadcastReceiver.class);
+
+        // Verify configuration change receiver is registered after start().
+        verify(mContext, never()).registerReceiver(receiverCaptor.capture(),
+                argThat(receiver -> ACTION_CONFIGURATION_CHANGED.equals(receiver.getAction(0))));
+        nm.start();
+        mCreatedNetworkMonitors.add(nm);
+        HandlerUtils.waitForIdle(nm.getHandler(), HANDLER_TIMEOUT_MS);
+        verify(mContext, times(1)).registerReceiver(receiverCaptor.capture(),
+                argThat(receiver -> ACTION_CONFIGURATION_CHANGED.equals(receiver.getAction(0))));
+        // Update a new URL and send a configuration change
+        doReturn(TEST_HTTPS_OTHER_URL1).when(mResources).getString(
+                R.string.config_captive_portal_https_url);
+        receiverCaptor.getValue().onReceive(mContext, new Intent(ACTION_CONFIGURATION_CHANGED));
+        HandlerUtils.waitForIdle(nm.getHandler(), HANDLER_TIMEOUT_MS);
+        // Should stay in default state before receiving CMD_NETWORK_CONNECTED
+        verify(mOtherHttpsConnection1, never()).getResponseCode();
+    }
+
+    @Test
+    public void testIsCaptivePortal_ConfigurationChange_RenewUrls() throws Exception {
+        setStatus(mHttpsConnection, 204);
+        final NetworkMonitor nm = runValidatedNetworkTest();
+        final ArgumentCaptor<BroadcastReceiver> receiverCaptor =
+                ArgumentCaptor.forClass(BroadcastReceiver.class);
+        verify(mContext, times(1)).registerReceiver(receiverCaptor.capture(),
+                argThat(receiver -> ACTION_CONFIGURATION_CHANGED.equals(receiver.getAction(0))));
+
+        resetCallbacks();
+        // New URLs with partial connectivity
+        doReturn(TEST_HTTPS_OTHER_URL1).when(mResources).getString(
+                R.string.config_captive_portal_https_url);
+        doReturn(TEST_HTTP_OTHER_URL1).when(mResources).getString(
+                R.string.config_captive_portal_http_url);
+        setStatus(mOtherHttpsConnection1, 500);
+        setStatus(mOtherHttpConnection1, 204);
+
+        // Receive configuration. Expect a reevaluation triggered.
+        receiverCaptor.getValue().onReceive(mContext, new Intent(ACTION_CONFIGURATION_CHANGED));
+
+        HandlerUtils.waitForIdle(nm.getHandler(), HANDLER_TIMEOUT_MS);
+        verifyNetworkTested(NETWORK_VALIDATION_RESULT_PARTIAL,
+                NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP);
+        verify(mOtherHttpsConnection1, times(1)).getResponseCode();
+        verify(mOtherHttpConnection1, times(1)).getResponseCode();
+    }
+
     private CellInfoGsm makeTestCellInfoGsm(String mcc) throws Exception {
         final CellInfoGsm info = new CellInfoGsm();
         final CellIdentityGsm ci = makeCellIdentityGsm(0, 0, 0, 0, mcc, "01", "", "");
@@ -967,7 +1024,7 @@ public class NetworkMonitorTest {
     public void testMakeFallbackUrls() throws Exception {
         final WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor();
         // Value exist in setting provider.
-        URL[] urls = wnm.makeCaptivePortalFallbackUrls();
+        URL[] urls = wnm.makeCaptivePortalFallbackUrls(mContext);
         assertEquals(urls[0].toString(), TEST_FALLBACK_URL);
 
         // Clear setting provider value. Verify it to get configuration from resource instead.
@@ -975,13 +1032,13 @@ public class NetworkMonitorTest {
         // Verify that getting resource with exception.
         when(mResources.getStringArray(R.array.config_captive_portal_fallback_urls))
                 .thenThrow(Resources.NotFoundException.class);
-        urls = wnm.makeCaptivePortalFallbackUrls();
+        urls = wnm.makeCaptivePortalFallbackUrls(mContext);
         assertEquals(urls.length, 0);
 
         // Verify resource return 2 different URLs.
         doReturn(new String[] {"http://testUrl1.com", "http://testUrl2.com"}).when(mResources)
                 .getStringArray(R.array.config_captive_portal_fallback_urls);
-        urls = wnm.makeCaptivePortalFallbackUrls();
+        urls = wnm.makeCaptivePortalFallbackUrls(mContext);
         assertEquals(urls.length, 2);
         assertEquals("http://testUrl1.com", urls[0].toString());
         assertEquals("http://testUrl2.com", urls[1].toString());
@@ -992,7 +1049,7 @@ public class NetworkMonitorTest {
         setupNoSimCardNeighborMcc();
         doReturn(new String[] {"http://testUrl3.com"}).when(mMccResource)
                 .getStringArray(R.array.config_captive_portal_fallback_urls);
-        urls = wnm.makeCaptivePortalFallbackUrls();
+        urls = wnm.makeCaptivePortalFallbackUrls(mContext);
         assertEquals(urls.length, 2);
         assertEquals("http://testUrl1.com", urls[0].toString());
         assertEquals("http://testUrl2.com", urls[1].toString());
@@ -1005,7 +1062,7 @@ public class NetworkMonitorTest {
         doReturn(new String[] {"http://testUrl.com"}).when(mMccResource)
                 .getStringArray(R.array.config_captive_portal_fallback_urls);
         final WrappedNetworkMonitor wnm = makeCellNotMeteredNetworkMonitor();
-        final URL[] urls = wnm.makeCaptivePortalFallbackUrls();
+        final URL[] urls = wnm.makeCaptivePortalFallbackUrls(mMccContext);
         assertEquals(urls.length, 1);
         assertEquals("http://testUrl.com", urls[0].toString());
     }
@@ -1894,7 +1951,7 @@ public class NetworkMonitorTest {
         verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1))
                 .showProvisioningNotification(any(), any());
 
-        assertEquals(1, mRegisteredReceivers.size());
+        assertCaptivePortalAppReceiverRegistered(true /* isPortal */);
 
         // Check that startCaptivePortalApp sends the expected intent.
         nm.launchCaptivePortalApp();
@@ -1932,7 +1989,7 @@ public class NetworkMonitorTest {
                 .notifyNetworkTestedWithExtras(matchNetworkTestResultParcelable(
                         NETWORK_VALIDATION_RESULT_VALID,
                         NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP));
-        assertEquals(0, mRegisteredReceivers.size());
+        assertCaptivePortalAppReceiverRegistered(false /* isPortal */);
     }
 
     @Test
@@ -2518,7 +2575,7 @@ public class NetworkMonitorTest {
         verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1))
             .showProvisioningNotification(any(), any());
 
-        assertEquals(1, mRegisteredReceivers.size());
+        assertCaptivePortalAppReceiverRegistered(true /* isPortal */);
         // Check that startCaptivePortalApp sends the expected intent.
         nm.launchCaptivePortalApp();
 
@@ -2740,7 +2797,7 @@ public class NetworkMonitorTest {
         monitor.notifyNetworkConnected(linkProperties, networkCapabilities);
         verify(mCallbacks, timeout(HANDLER_TIMEOUT_MS).times(1))
                 .showProvisioningNotification(any(), any());
-        assertEquals(1, mRegisteredReceivers.size());
+        assertCaptivePortalAppReceiverRegistered(true /* isPortal */);
         verifyNetworkTested(VALIDATION_RESULT_PORTAL, 0 /* probesSucceeded */, TEST_LOGIN_URL);
 
         // Force reevaluation and confirm that the network is still captive
@@ -2903,21 +2960,21 @@ public class NetworkMonitorTest {
     private NetworkMonitor runPortalNetworkTest() throws RemoteException {
         final NetworkMonitor nm = runNetworkTest(VALIDATION_RESULT_PORTAL,
                 0 /* probesSucceeded */, TEST_LOGIN_URL);
-        assertEquals(1, mRegisteredReceivers.size());
+        assertCaptivePortalAppReceiverRegistered(true /* isPortal */);
         return nm;
     }
 
     private NetworkMonitor runNoValidationNetworkTest() throws RemoteException {
         final NetworkMonitor nm = runNetworkTest(NETWORK_VALIDATION_RESULT_VALID,
                 0 /* probesSucceeded */, null /* redirectUrl */);
-        assertEquals(0, mRegisteredReceivers.size());
+        assertCaptivePortalAppReceiverRegistered(false /* isPortal */);
         return nm;
     }
 
     private NetworkMonitor runFailedNetworkTest() throws RemoteException {
         final NetworkMonitor nm = runNetworkTest(
                 VALIDATION_RESULT_INVALID, 0 /* probesSucceeded */, null /* redirectUrl */);
-        assertEquals(0, mRegisteredReceivers.size());
+        assertCaptivePortalAppReceiverRegistered(false /* isPortal */);
         return nm;
     }
 
@@ -2925,7 +2982,7 @@ public class NetworkMonitorTest {
             throws RemoteException {
         final NetworkMonitor nm = runNetworkTest(NETWORK_VALIDATION_RESULT_PARTIAL,
                 probesSucceeded, null /* redirectUrl */);
-        assertEquals(0, mRegisteredReceivers.size());
+        assertCaptivePortalAppReceiverRegistered(false /* isPortal */);
         return nm;
     }
 
@@ -3057,5 +3114,13 @@ public class NetworkMonitorTest {
     private DataStallReportParcelable matchTcpDataStallParcelable() {
         return argThat(p -> (p.detectionMethod & ConstantsShim.DETECTION_METHOD_TCP_METRICS) != 0);
     }
+
+    private void assertCaptivePortalAppReceiverRegistered(boolean isPortal) {
+        // There will be configuration change receiver registered after NetworkMonitor being
+        // started. If captive portal app receiver is registered, then the size of the registered
+        // receivers will be 2. Otherwise, mRegisteredReceivers should only contain 1 configuration
+        // change receiver.
+        assertEquals(isPortal ? 2 : 1, mRegisteredReceivers.size());
+    }
 }