Merge "Migrate away from AsyncChannel in NetworkAgent" am: c9c51b665d am: 39364945d5

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/1484018

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: Ie74232c341548340fd893783c8b7186f3f3cb2aa

framework/Android.bp

@@ -0,0 +1,29 @@
+//
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+// TODO: use a java_library in the bootclasspath instead
+filegroup {
+    name: "framework-connectivity-sources",
+    srcs: [
+        "src/**/*.java",
+        "src/**/*.aidl",
+    ],
+    path: "src",
+    visibility: [
+        "//frameworks/base",
+        "//packages/modules/Connectivity:__subpackages__",
+    ],
+}

framework/src/com/android/connectivity/aidl/INetworkAgent.aidl

@@ -0,0 +1,46 @@
+/**
+ * Copyright (c) 2020, The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing perNmissions and
+ * limitations under the License.
+ */
+package com.android.connectivity.aidl;
+
+import android.net.NattKeepalivePacketData;
+import android.net.TcpKeepalivePacketData;
+
+import com.android.connectivity.aidl.INetworkAgentRegistry;
+
+/**
+ * Interface to notify NetworkAgent of connectivity events.
+ * @hide
+ */
+oneway interface INetworkAgent {
+    void onRegistered(in INetworkAgentRegistry registry);
+    void onDisconnected();
+    void onBandwidthUpdateRequested();
+    void onValidationStatusChanged(int validationStatus,
+            in @nullable String captivePortalUrl);
+    void onSaveAcceptUnvalidated(boolean acceptUnvalidated);
+    void onStartNattSocketKeepalive(int slot, int intervalDurationMs,
+        in NattKeepalivePacketData packetData);
+    void onStartTcpSocketKeepalive(int slot, int intervalDurationMs,
+        in TcpKeepalivePacketData packetData);
+    void onStopSocketKeepalive(int slot);
+    void onSignalStrengthThresholdsUpdated(in int[] thresholds);
+    void onPreventAutomaticReconnect();
+    void onAddNattKeepalivePacketFilter(int slot,
+        in NattKeepalivePacketData packetData);
+    void onAddTcpKeepalivePacketFilter(int slot,
+        in TcpKeepalivePacketData packetData);
+    void onRemoveKeepalivePacketFilter(int slot);
+}

framework/src/com/android/connectivity/aidl/INetworkAgentRegistry.aidl

@@ -0,0 +1,36 @@
+/**
+ * Copyright (c) 2020, The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing perNmissions and
+ * limitations under the License.
+ */
+package com.android.connectivity.aidl;
+
+import android.net.LinkProperties;
+import android.net.Network;
+import android.net.NetworkCapabilities;
+import android.net.NetworkInfo;
+
+/**
+ * Interface for NetworkAgents to send network network properties.
+ * @hide
+ */
+oneway interface INetworkAgentRegistry {
+    void sendNetworkCapabilities(in NetworkCapabilities nc);
+    void sendLinkProperties(in LinkProperties lp);
+    // TODO: consider replacing this by "markConnected()" and removing
+    void sendNetworkInfo(in NetworkInfo info);
+    void sendScore(int score);
+    void sendExplicitlySelected(boolean explicitlySelected, boolean acceptPartial);
+    void sendSocketKeepaliveEvent(int slot, int reason);
+    void sendUnderlyingNetworks(in @nullable List<Network> networks);
+}

tests/cts/net/src/android/net/cts/NetworkAgentTest.kt

@@ -23,15 +23,9 @@ import android.net.IpPrefix
 import android.net.KeepalivePacketData
 import android.net.LinkAddress
 import android.net.LinkProperties
+import android.net.NattKeepalivePacketData
 import android.net.Network
 import android.net.NetworkAgent
-import android.net.NetworkAgent.CMD_ADD_KEEPALIVE_PACKET_FILTER
-import android.net.NetworkAgent.CMD_PREVENT_AUTOMATIC_RECONNECT
-import android.net.NetworkAgent.CMD_REMOVE_KEEPALIVE_PACKET_FILTER
-import android.net.NetworkAgent.CMD_REPORT_NETWORK_STATUS
-import android.net.NetworkAgent.CMD_SAVE_ACCEPT_UNVALIDATED
-import android.net.NetworkAgent.CMD_START_SOCKET_KEEPALIVE
-import android.net.NetworkAgent.CMD_STOP_SOCKET_KEEPALIVE
 import android.net.NetworkAgent.INVALID_NETWORK
 import android.net.NetworkAgent.VALID_NETWORK
 import android.net.NetworkAgentConfig
@@ -64,16 +58,14 @@ import android.net.cts.NetworkAgentTest.TestableNetworkAgent.CallbackEntry.OnSta
 import android.net.cts.NetworkAgentTest.TestableNetworkAgent.CallbackEntry.OnStopSocketKeepalive
 import android.net.cts.NetworkAgentTest.TestableNetworkAgent.CallbackEntry.OnValidationStatus
 import android.os.Build
-import android.os.Bundle
-import android.os.Handler
 import android.os.HandlerThread
 import android.os.Looper
 import android.os.Message
-import android.os.Messenger
 import android.util.DebugUtils.valueToString
 import androidx.test.InstrumentationRegistry
 import androidx.test.runner.AndroidJUnit4
-import com.android.internal.util.AsyncChannel
+import com.android.connectivity.aidl.INetworkAgent
+import com.android.connectivity.aidl.INetworkAgentRegistry
 import com.android.net.module.util.ArrayTrackRecord
 import com.android.testutils.DevSdkIgnoreRule
 import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo
@@ -82,7 +74,6 @@ import com.android.testutils.RecorderCallback.CallbackEntry.Lost
 import com.android.testutils.TestableNetworkCallback
 import org.junit.After
 import org.junit.Assert.assertArrayEquals
-import org.junit.Assert.fail
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -93,6 +84,7 @@ import org.mockito.ArgumentMatchers.argThat
 import org.mockito.ArgumentMatchers.eq
 import org.mockito.Mockito.doReturn
 import org.mockito.Mockito.mock
+import org.mockito.Mockito.timeout
 import org.mockito.Mockito.verify
 import java.time.Duration
 import java.util.Arrays
@@ -103,6 +95,7 @@ import kotlin.test.assertFalse
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
+import kotlin.test.fail
 
 // This test doesn't really have a constraint on how fast the methods should return. If it's
 // going to fail, it will simply wait forever, so setting a high timeout lowers the flake ratio
@@ -140,7 +133,7 @@ class NetworkAgentTest {
 
     private val mCM = realContext.getSystemService(ConnectivityManager::class.java)
     private val mHandlerThread = HandlerThread("${javaClass.simpleName} handler thread")
-    private val mFakeConnectivityService by lazy { FakeConnectivityService(mHandlerThread.looper) }
+    private val mFakeConnectivityService = FakeConnectivityService()
 
     private class Provider(context: Context, looper: Looper) :
             NetworkProvider(context, looper, "NetworkAgentTest NetworkProvider")
@@ -167,39 +160,30 @@ class NetworkAgentTest {
      * This fake only supports speaking to one harnessed agent at a time because it
      * only keeps track of one async channel.
      */
-    private class FakeConnectivityService(looper: Looper) {
+    private class FakeConnectivityService {
-        private val CMD_EXPECT_DISCONNECT = 1
+        val mockRegistry = mock(INetworkAgentRegistry::class.java)
-        private var disconnectExpected = false
+        private var agentField: INetworkAgent? = null
-        private val msgHistory = ArrayTrackRecord<Message>().newReadHead()
+        private val registry = object : INetworkAgentRegistry.Stub(),
-        private val asyncChannel = AsyncChannel()
+                INetworkAgentRegistry by mockRegistry {
-        private val handler = object : Handler(looper) {
+            // asBinder has implementations in both INetworkAgentRegistry.Stub and mockRegistry, so
-            override fun handleMessage(msg: Message) {
+            // it needs to be disambiguated. Just fail the test as it should be unused here.
-                msgHistory.add(Message.obtain(msg)) // make a copy as the original will be recycled
+            // asBinder is used when sending the registry in binder transactions, so not in this
-                when (msg.what) {
+            // test (the test just uses in-process direct calls). If it were used across processes,
-                    CMD_EXPECT_DISCONNECT -> disconnectExpected = true
+            // using the Stub super.asBinder() implementation would allow sending the registry in
-                    AsyncChannel.CMD_CHANNEL_HALF_CONNECTED ->
+            // binder transactions, while recording incoming calls on the other mockito-generated
-                        asyncChannel.sendMessage(AsyncChannel.CMD_CHANNEL_FULL_CONNECTION)
+            // methods.
-                    AsyncChannel.CMD_CHANNEL_DISCONNECTED ->
+            override fun asBinder() = fail("asBinder should be unused in this test")
-                        if (!disconnectExpected) {
-                            fail("Agent unexpectedly disconnected")
-                        } else {
-                            disconnectExpected = false
-                        }
-                }
-            }
         }
 
-        fun connect(agentMsngr: Messenger) = asyncChannel.connect(realContext, handler, agentMsngr)
+        val agent: INetworkAgent
+            get() = agentField ?: fail("No INetworkAgent")
 
-        fun disconnect() = asyncChannel.disconnect()
+        fun connect(agent: INetworkAgent) {
+            this.agentField = agent
+            agent.onRegistered(registry)
+        }
 
-        fun sendMessage(what: Int, arg1: Int = 0, arg2: Int = 0, obj: Any? = null) =
+        fun disconnect() = agent.onDisconnected()
-            asyncChannel.sendMessage(Message(what, arg1, arg2, obj))
-
-        fun expectMessage(what: Int) =
-            assertNotNull(msgHistory.poll(DEFAULT_TIMEOUT_MS) { it.what == what })
-
-        fun willExpectDisconnectOnce() = handler.sendEmptyMessage(CMD_EXPECT_DISCONNECT)
     }
 
     private open class TestableNetworkAgent(
@@ -445,17 +429,15 @@ class NetworkAgentTest {
 
     @Test
     fun testSocketKeepalive(): Unit = createNetworkAgentWithFakeCS().let { agent ->
-        val packet = object : KeepalivePacketData(
+        val packet = NattKeepalivePacketData(
                 LOCAL_IPV4_ADDRESS /* srcAddress */, 1234 /* srcPort */,
                 REMOTE_IPV4_ADDRESS /* dstAddress */, 4567 /* dstPort */,
-                ByteArray(100 /* size */) { it.toByte() /* init */ }) {}
+                ByteArray(100 /* size */))
         val slot = 4
         val interval = 37
 
-        mFakeConnectivityService.sendMessage(CMD_ADD_KEEPALIVE_PACKET_FILTER,
+        mFakeConnectivityService.agent.onAddNattKeepalivePacketFilter(slot, packet)
-                arg1 = slot, obj = packet)
+        mFakeConnectivityService.agent.onStartNattSocketKeepalive(slot, interval, packet)
-        mFakeConnectivityService.sendMessage(CMD_START_SOCKET_KEEPALIVE,
-                arg1 = slot, arg2 = interval, obj = packet)
 
         agent.expectCallback<OnAddKeepalivePacketFilter>().let {
             assertEquals(it.slot, slot)
@@ -472,13 +454,11 @@ class NetworkAgentTest {
         // Check that when the agent sends a keepalive event, ConnectivityService receives the
         // expected message.
         agent.sendSocketKeepaliveEvent(slot, SocketKeepalive.ERROR_UNSUPPORTED)
-        mFakeConnectivityService.expectMessage(NetworkAgent.EVENT_SOCKET_KEEPALIVE).let() {
+        verify(mFakeConnectivityService.mockRegistry, timeout(DEFAULT_TIMEOUT_MS))
-            assertEquals(slot, it.arg1)
+                .sendSocketKeepaliveEvent(slot, SocketKeepalive.ERROR_UNSUPPORTED)
-            assertEquals(SocketKeepalive.ERROR_UNSUPPORTED, it.arg2)
-        }
 
-        mFakeConnectivityService.sendMessage(CMD_STOP_SOCKET_KEEPALIVE, arg1 = slot)
+        mFakeConnectivityService.agent.onStopSocketKeepalive(slot)
-        mFakeConnectivityService.sendMessage(CMD_REMOVE_KEEPALIVE_PACKET_FILTER, arg1 = slot)
+        mFakeConnectivityService.agent.onRemoveKeepalivePacketFilter(slot)
         agent.expectCallback<OnStopSocketKeepalive>().let {
             assertEquals(it.slot, slot)
         }
@@ -639,7 +619,7 @@ class NetworkAgentTest {
         val mockCm = mock(ConnectivityManager::class.java)
         doReturn(mockCm).`when`(mockContext).getSystemService(Context.CONNECTIVITY_SERVICE)
         createConnectedNetworkAgent(mockContext)
-        verify(mockCm).registerNetworkAgent(any(Messenger::class.java),
+        verify(mockCm).registerNetworkAgent(any(),
                 argThat<NetworkInfo> { it.detailedState == NetworkInfo.DetailedState.CONNECTING },
                 any(LinkProperties::class.java),
                 any(NetworkCapabilities::class.java),
@@ -651,7 +631,7 @@ class NetworkAgentTest {
     @Test
     fun testSetAcceptUnvalidated() {
         createNetworkAgentWithFakeCS().let { agent ->
-            mFakeConnectivityService.sendMessage(CMD_SAVE_ACCEPT_UNVALIDATED, 1)
+            mFakeConnectivityService.agent.onSaveAcceptUnvalidated(true)
             agent.expectCallback<OnSaveAcceptUnvalidated>().let {
                 assertTrue(it.accept)
             }
@@ -662,19 +642,18 @@ class NetworkAgentTest {
     @Test
     fun testSetAcceptUnvalidatedPreventAutomaticReconnect() {
         createNetworkAgentWithFakeCS().let { agent ->
-            mFakeConnectivityService.sendMessage(CMD_SAVE_ACCEPT_UNVALIDATED, 0)
+            mFakeConnectivityService.agent.onSaveAcceptUnvalidated(false)
-            mFakeConnectivityService.sendMessage(CMD_PREVENT_AUTOMATIC_RECONNECT)
+            mFakeConnectivityService.agent.onPreventAutomaticReconnect()
             agent.expectCallback<OnSaveAcceptUnvalidated>().let {
                 assertFalse(it.accept)
             }
             agent.expectCallback<OnAutomaticReconnectDisabled>()
             agent.assertNoCallback()
             // When automatic reconnect is turned off, the network is torn down and
-            // ConnectivityService sends a disconnect. This in turn causes the agent
+            // ConnectivityService disconnects. As part of the disconnect, ConnectivityService will
-            // to send a DISCONNECTED message to CS.
+            // also send itself a message to unregister the NetworkAgent from its internal
-            mFakeConnectivityService.willExpectDisconnectOnce()
+            // structure.
             mFakeConnectivityService.disconnect()
-            mFakeConnectivityService.expectMessage(AsyncChannel.CMD_CHANNEL_DISCONNECTED)
             agent.expectCallback<OnNetworkUnwanted>()
         }
     }
@@ -682,12 +661,10 @@ class NetworkAgentTest {
     @Test
     fun testPreventAutomaticReconnect() {
         createNetworkAgentWithFakeCS().let { agent ->
-            mFakeConnectivityService.sendMessage(CMD_PREVENT_AUTOMATIC_RECONNECT)
+            mFakeConnectivityService.agent.onPreventAutomaticReconnect()
             agent.expectCallback<OnAutomaticReconnectDisabled>()
             agent.assertNoCallback()
-            mFakeConnectivityService.willExpectDisconnectOnce()
             mFakeConnectivityService.disconnect()
-            mFakeConnectivityService.expectMessage(AsyncChannel.CMD_CHANNEL_DISCONNECTED)
             agent.expectCallback<OnNetworkUnwanted>()
         }
     }
@@ -695,18 +672,14 @@ class NetworkAgentTest {
     @Test
     fun testValidationStatus() = createNetworkAgentWithFakeCS().let { agent ->
         val uri = Uri.parse("http://www.google.com")
-        val bundle = Bundle().apply {
+        mFakeConnectivityService.agent.onValidationStatusChanged(VALID_NETWORK,
-            putString(NetworkAgent.REDIRECT_URL_KEY, uri.toString())
+                uri.toString())
-        }
-        mFakeConnectivityService.sendMessage(CMD_REPORT_NETWORK_STATUS,
-                arg1 = VALID_NETWORK, obj = bundle)
         agent.expectCallback<OnValidationStatus>().let {
             assertEquals(it.status, VALID_NETWORK)
             assertEquals(it.uri, uri)
         }
 
-        mFakeConnectivityService.sendMessage(CMD_REPORT_NETWORK_STATUS,
+        mFakeConnectivityService.agent.onValidationStatusChanged(INVALID_NETWORK, null)
-                arg1 = INVALID_NETWORK, obj = Bundle())
         agent.expectCallback<OnValidationStatus>().let {
             assertEquals(it.status, INVALID_NETWORK)
             assertNull(it.uri)