Merge "Migrate away from AsyncChannel in NetworkAgent"

5 files changed, 198 insertions, 74 deletions

diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java
index 09c3050f89b8..f756cda4b9bb 100644
--- a/core/java/android/net/ConnectivityManager.java
+++ b/core/java/android/net/ConnectivityManager.java
@@ -61,6 +61,7 @@ import android.util.ArrayMap;
 import android.util.Log;
 import android.util.SparseIntArray;
 
+import com.android.connectivity.aidl.INetworkAgent;
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.util.Preconditions;
 import com.android.internal.util.Protocol;
@@ -3287,9 +3288,9 @@ public class ConnectivityManager {
     @RequiresPermission(anyOf = {
             NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
             android.Manifest.permission.NETWORK_FACTORY})
-    public Network registerNetworkAgent(Messenger messenger, NetworkInfo ni, LinkProperties lp,
+    public Network registerNetworkAgent(INetworkAgent na, NetworkInfo ni, LinkProperties lp,
             NetworkCapabilities nc, int score, NetworkAgentConfig config) {
-        return registerNetworkAgent(messenger, ni, lp, nc, score, config, NetworkProvider.ID_NONE);
+        return registerNetworkAgent(na, ni, lp, nc, score, config, NetworkProvider.ID_NONE);
     }
 
     /**
@@ -3300,10 +3301,10 @@ public class ConnectivityManager {
     @RequiresPermission(anyOf = {
             NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
             android.Manifest.permission.NETWORK_FACTORY})
-    public Network registerNetworkAgent(Messenger messenger, NetworkInfo ni, LinkProperties lp,
+    public Network registerNetworkAgent(INetworkAgent na, NetworkInfo ni, LinkProperties lp,
             NetworkCapabilities nc, int score, NetworkAgentConfig config, int providerId) {
         try {
-            return mService.registerNetworkAgent(messenger, ni, lp, nc, score, config, providerId);
+            return mService.registerNetworkAgent(na, ni, lp, nc, score, config, providerId);
         } catch (RemoteException e) {
             throw e.rethrowFromSystemServer();
         }
diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl
index 95a2f2efeb7d..cbb11977ef23 100644
--- a/core/java/android/net/IConnectivityManager.aidl
+++ b/core/java/android/net/IConnectivityManager.aidl
@@ -37,6 +37,7 @@ import android.os.ParcelFileDescriptor;
 import android.os.PersistableBundle;
 import android.os.ResultReceiver;
 
+import com.android.connectivity.aidl.INetworkAgent;
 import com.android.internal.net.LegacyVpnInfo;
 import com.android.internal.net.VpnConfig;
 import com.android.internal.net.VpnInfo;
@@ -164,7 +165,7 @@ interface IConnectivityManager
 
     void declareNetworkRequestUnfulfillable(in NetworkRequest request);
 
-    Network registerNetworkAgent(in Messenger messenger, in NetworkInfo ni, in LinkProperties lp,
+    Network registerNetworkAgent(in INetworkAgent na, in NetworkInfo ni, in LinkProperties lp,
             in NetworkCapabilities nc, int score, in NetworkAgentConfig config,
             in int factorySerialNumber);
 
diff --git a/core/java/android/net/NattKeepalivePacketData.aidl b/core/java/android/net/NattKeepalivePacketData.aidl
new file mode 100644
index 000000000000..af644b54827c
--- /dev/null
+++ b/core/java/android/net/NattKeepalivePacketData.aidl
@@ -0,0 +1,18 @@
+/**
+ * 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 android.net;
+
+@JavaOnlyStableParcelable parcelable NattKeepalivePacketData;
diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java
index 6780167fa63e..4166c2c4f244 100644
--- a/core/java/android/net/NetworkAgent.java
+++ b/core/java/android/net/NetworkAgent.java
@@ -29,11 +29,12 @@ import android.os.ConditionVariable;
 import android.os.Handler;
 import android.os.Looper;
 import android.os.Message;
-import android.os.Messenger;
+import android.os.RemoteException;
 import android.util.Log;
 
+import com.android.connectivity.aidl.INetworkAgent;
+import com.android.connectivity.aidl.INetworkAgentRegistry;
 import com.android.internal.annotations.VisibleForTesting;
-import com.android.internal.util.AsyncChannel;
 import com.android.internal.util.Protocol;
 
 import java.lang.annotation.Retention;
@@ -94,12 +95,18 @@ public abstract class NetworkAgent {
     @Nullable
     private volatile Network mNetwork;
 
+    @Nullable
+    private volatile INetworkAgentRegistry mRegistry;
+
+    private interface RegistryAction {
+        void execute(@NonNull INetworkAgentRegistry registry) throws RemoteException;
+    }
+
     private final Handler mHandler;
-    private volatile AsyncChannel mAsyncChannel;
     private final String LOG_TAG;
     private static final boolean DBG = true;
     private static final boolean VDBG = false;
-    private final ArrayList<Message> mPreConnectedQueue = new ArrayList<Message>();
+    private final ArrayList<RegistryAction> mPreConnectedQueue = new ArrayList<>();
     private volatile long mLastBwRefreshTime = 0;
     private static final long BW_REFRESH_MIN_WIN_MS = 500;
     private boolean mBandwidthUpdateScheduled = false;
@@ -329,6 +336,17 @@ public abstract class NetworkAgent {
      */
     public static final int CMD_REMOVE_KEEPALIVE_PACKET_FILTER = BASE + 17;
 
+    /**
+     * Sent by ConnectivityService to the NetworkAgent to complete the bidirectional connection.
+     * obj = INetworkAgentRegistry
+     */
+    private static final int EVENT_AGENT_CONNECTED = BASE + 18;
+
+    /**
+     * Sent by ConnectivityService to the NetworkAgent to inform the agent that it was disconnected.
+     */
+    private static final int EVENT_AGENT_DISCONNECTED = BASE + 19;
+
     private static NetworkInfo getLegacyNetworkInfo(final NetworkAgentConfig config) {
         // The subtype can be changed with (TODO) setLegacySubtype, but it starts
         // with 0 (TelephonyManager.NETWORK_TYPE_UNKNOWN) and an empty description.
@@ -402,36 +420,33 @@ public abstract class NetworkAgent {
         @Override
         public void handleMessage(Message msg) {
             switch (msg.what) {
-                case AsyncChannel.CMD_CHANNEL_FULL_CONNECTION: {
-                    if (mAsyncChannel != null) {
+                case EVENT_AGENT_CONNECTED: {
+                    if (mRegistry != null) {
                         log("Received new connection while already connected!");
                     } else {
                         if (VDBG) log("NetworkAgent fully connected");
-                        AsyncChannel ac = new AsyncChannel();
-                        ac.connected(null, this, msg.replyTo);
-                        ac.replyToMessage(msg, AsyncChannel.CMD_CHANNEL_FULLY_CONNECTED,
-                                AsyncChannel.STATUS_SUCCESSFUL);
                         synchronized (mPreConnectedQueue) {
-                            mAsyncChannel = ac;
-                            for (Message m : mPreConnectedQueue) {
-                                ac.sendMessage(m);
+                            final INetworkAgentRegistry registry = (INetworkAgentRegistry) msg.obj;
+                            mRegistry = registry;
+                            for (RegistryAction a : mPreConnectedQueue) {
+                                try {
+                                    a.execute(registry);
+                                } catch (RemoteException e) {
+                                    Log.wtf(LOG_TAG, "Communication error with registry", e);
+                                    // Fall through
+                                }
                             }
                             mPreConnectedQueue.clear();
                         }
                     }
                     break;
                 }
-                case AsyncChannel.CMD_CHANNEL_DISCONNECT: {
-                    if (VDBG) log("CMD_CHANNEL_DISCONNECT");
-                    if (mAsyncChannel != null) mAsyncChannel.disconnect();
-                    break;
-                }
-                case AsyncChannel.CMD_CHANNEL_DISCONNECTED: {
+                case EVENT_AGENT_DISCONNECTED: {
                     if (DBG) log("NetworkAgent channel lost");
                     // let the client know CS is done with us.
                     onNetworkUnwanted();
                     synchronized (mPreConnectedQueue) {
-                        mAsyncChannel = null;
+                        mRegistry = null;
                     }
                     break;
                 }
@@ -494,15 +509,7 @@ public abstract class NetworkAgent {
                 }
 
                 case CMD_SET_SIGNAL_STRENGTH_THRESHOLDS: {
-                    ArrayList<Integer> thresholds =
-                            ((Bundle) msg.obj).getIntegerArrayList("thresholds");
-                    // TODO: Change signal strength thresholds API to use an ArrayList<Integer>
-                    // rather than convert to int[].
-                    int[] intThresholds = new int[(thresholds != null) ? thresholds.size() : 0];
-                    for (int i = 0; i < intThresholds.length; i++) {
-                        intThresholds[i] = thresholds.get(i);
-                    }
-                    onSignalStrengthThresholdsUpdated(intThresholds);
+                    onSignalStrengthThresholdsUpdated((int[]) msg.obj);
                     break;
                 }
                 case CMD_PREVENT_AUTOMATIC_RECONNECT: {
@@ -541,7 +548,7 @@ public abstract class NetworkAgent {
             }
             final ConnectivityManager cm = (ConnectivityManager) mInitialConfiguration.context
                     .getSystemService(Context.CONNECTIVITY_SERVICE);
-            mNetwork = cm.registerNetworkAgent(new Messenger(mHandler),
+            mNetwork = cm.registerNetworkAgent(new NetworkAgentBinder(mHandler),
                     new NetworkInfo(mInitialConfiguration.info),
                     mInitialConfiguration.properties, mInitialConfiguration.capabilities,
                     mInitialConfiguration.score, mInitialConfiguration.config, providerId);
@@ -550,6 +557,95 @@ public abstract class NetworkAgent {
         return mNetwork;
     }
 
+    private static class NetworkAgentBinder extends INetworkAgent.Stub {
+        private final Handler mHandler;
+
+        private NetworkAgentBinder(Handler handler) {
+            mHandler = handler;
+        }
+
+        @Override
+        public void onRegistered(@NonNull INetworkAgentRegistry registry) {
+            mHandler.sendMessage(mHandler.obtainMessage(EVENT_AGENT_CONNECTED, registry));
+        }
+
+        @Override
+        public void onDisconnected() {
+            mHandler.sendMessage(mHandler.obtainMessage(EVENT_AGENT_DISCONNECTED));
+        }
+
+        @Override
+        public void onBandwidthUpdateRequested() {
+            mHandler.sendMessage(mHandler.obtainMessage(CMD_REQUEST_BANDWIDTH_UPDATE));
+        }
+
+        @Override
+        public void onValidationStatusChanged(
+                int validationStatus, @Nullable String captivePortalUrl) {
+            // TODO: consider using a parcelable as argument when the interface is structured
+            Bundle redirectUrlBundle = new Bundle();
+            redirectUrlBundle.putString(NetworkAgent.REDIRECT_URL_KEY, captivePortalUrl);
+            mHandler.sendMessage(mHandler.obtainMessage(CMD_REPORT_NETWORK_STATUS,
+                    validationStatus, 0, redirectUrlBundle));
+        }
+
+        @Override
+        public void onSaveAcceptUnvalidated(boolean acceptUnvalidated) {
+            mHandler.sendMessage(mHandler.obtainMessage(CMD_SAVE_ACCEPT_UNVALIDATED,
+                    acceptUnvalidated ? 1 : 0, 0));
+        }
+
+        @Override
+        public void onStartNattSocketKeepalive(int slot, int intervalDurationMs,
+                @NonNull NattKeepalivePacketData packetData) {
+            mHandler.sendMessage(mHandler.obtainMessage(CMD_START_SOCKET_KEEPALIVE,
+                    slot, intervalDurationMs, packetData));
+        }
+
+        @Override
+        public void onStartTcpSocketKeepalive(int slot, int intervalDurationMs,
+                @NonNull TcpKeepalivePacketData packetData) {
+            mHandler.sendMessage(mHandler.obtainMessage(CMD_START_SOCKET_KEEPALIVE,
+                    slot, intervalDurationMs, packetData));
+        }
+
+        @Override
+        public void onStopSocketKeepalive(int slot) {
+            mHandler.sendMessage(mHandler.obtainMessage(CMD_STOP_SOCKET_KEEPALIVE, slot, 0));
+        }
+
+        @Override
+        public void onSignalStrengthThresholdsUpdated(@NonNull int[] thresholds) {
+            mHandler.sendMessage(mHandler.obtainMessage(
+                    CMD_SET_SIGNAL_STRENGTH_THRESHOLDS, thresholds));
+        }
+
+        @Override
+        public void onPreventAutomaticReconnect() {
+            mHandler.sendMessage(mHandler.obtainMessage(CMD_PREVENT_AUTOMATIC_RECONNECT));
+        }
+
+        @Override
+        public void onAddNattKeepalivePacketFilter(int slot,
+                @NonNull NattKeepalivePacketData packetData) {
+            mHandler.sendMessage(mHandler.obtainMessage(CMD_ADD_KEEPALIVE_PACKET_FILTER,
+                    slot, 0, packetData));
+        }
+
+        @Override
+        public void onAddTcpKeepalivePacketFilter(int slot,
+                @NonNull TcpKeepalivePacketData packetData) {
+            mHandler.sendMessage(mHandler.obtainMessage(CMD_ADD_KEEPALIVE_PACKET_FILTER,
+                    slot, 0, packetData));
+        }
+
+        @Override
+        public void onRemoveKeepalivePacketFilter(int slot) {
+            mHandler.sendMessage(mHandler.obtainMessage(CMD_REMOVE_KEEPALIVE_PACKET_FILTER,
+                    slot, 0));
+        }
+    }
+
     /**
      * Register this network agent with a testing harness.
      *
@@ -559,13 +655,13 @@ public abstract class NetworkAgent {
      *
      * @hide
      */
-    public Messenger registerForTest(final Network network) {
+    public INetworkAgent registerForTest(final Network network) {
         log("Registering NetworkAgent for test");
         synchronized (mRegisterLock) {
             mNetwork = network;
             mInitialConfiguration = null;
         }
-        return new Messenger(mHandler);
+        return new NetworkAgentBinder(mHandler);
     }
 
     /**
@@ -589,29 +685,17 @@ public abstract class NetworkAgent {
         return mNetwork;
     }
 
-    private void queueOrSendMessage(int what, Object obj) {
-        queueOrSendMessage(what, 0, 0, obj);
-    }
-
-    private void queueOrSendMessage(int what, int arg1, int arg2) {
-        queueOrSendMessage(what, arg1, arg2, null);
-    }
-
-    private void queueOrSendMessage(int what, int arg1, int arg2, Object obj) {
-        Message msg = Message.obtain();
-        msg.what = what;
-        msg.arg1 = arg1;
-        msg.arg2 = arg2;
-        msg.obj = obj;
-        queueOrSendMessage(msg);
-    }
-
-    private void queueOrSendMessage(Message msg) {
+    private void queueOrSendMessage(@NonNull RegistryAction action) {
         synchronized (mPreConnectedQueue) {
-            if (mAsyncChannel != null) {
-                mAsyncChannel.sendMessage(msg);
+            if (mRegistry != null) {
+                try {
+                    action.execute(mRegistry);
+                } catch (RemoteException e) {
+                    Log.wtf(LOG_TAG, "Error executing registry action", e);
+                    // Fall through: the channel is asynchronous and does not report errors back
+                }
             } else {
-                mPreConnectedQueue.add(msg);
+                mPreConnectedQueue.add(action);
             }
         }
     }
@@ -622,7 +706,8 @@ public abstract class NetworkAgent {
      */
     public final void sendLinkProperties(@NonNull LinkProperties linkProperties) {
         Objects.requireNonNull(linkProperties);
-        queueOrSendMessage(EVENT_NETWORK_PROPERTIES_CHANGED, new LinkProperties(linkProperties));
+        final LinkProperties lp = new LinkProperties(linkProperties);
+        queueOrSendMessage(reg -> reg.sendLinkProperties(lp));
     }
 
     /**
@@ -647,9 +732,7 @@ public abstract class NetworkAgent {
     public final void setUnderlyingNetworks(@Nullable List<Network> underlyingNetworks) {
         final ArrayList<Network> underlyingArray = (underlyingNetworks != null)
                 ? new ArrayList<>(underlyingNetworks) : null;
-        final Bundle bundle = new Bundle();
-        bundle.putParcelableArrayList(UNDERLYING_NETWORKS_KEY, underlyingArray);
-        queueOrSendMessage(EVENT_UNDERLYING_NETWORKS_CHANGED, bundle);
+        queueOrSendMessage(reg -> reg.sendUnderlyingNetworks(underlyingArray));
     }
 
     /**
@@ -659,7 +742,7 @@ public abstract class NetworkAgent {
     public void markConnected() {
         mNetworkInfo.setDetailedState(NetworkInfo.DetailedState.CONNECTED, null /* reason */,
                 mNetworkInfo.getExtraInfo());
-        queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, mNetworkInfo);
+        queueOrSendNetworkInfo(mNetworkInfo);
     }
 
     /**
@@ -672,7 +755,7 @@ public abstract class NetworkAgent {
         // When unregistering an agent nobody should use the extrainfo (or reason) any more.
         mNetworkInfo.setDetailedState(NetworkInfo.DetailedState.DISCONNECTED, null /* reason */,
                 null /* extraInfo */);
-        queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, mNetworkInfo);
+        queueOrSendNetworkInfo(mNetworkInfo);
     }
 
     /**
@@ -689,7 +772,7 @@ public abstract class NetworkAgent {
     @Deprecated
     public void setLegacySubtype(final int legacySubtype, @NonNull final String legacySubtypeName) {
         mNetworkInfo.setSubtype(legacySubtype, legacySubtypeName);
-        queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, mNetworkInfo);
+        queueOrSendNetworkInfo(mNetworkInfo);
     }
 
     /**
@@ -711,7 +794,7 @@ public abstract class NetworkAgent {
     @Deprecated
     public void setLegacyExtraInfo(@Nullable final String extraInfo) {
         mNetworkInfo.setExtraInfo(extraInfo);
-        queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, mNetworkInfo);
+        queueOrSendNetworkInfo(mNetworkInfo);
     }
 
     /**
@@ -720,7 +803,11 @@ public abstract class NetworkAgent {
      */
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
     public final void sendNetworkInfo(NetworkInfo networkInfo) {
-        queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, new NetworkInfo(networkInfo));
+        queueOrSendNetworkInfo(new NetworkInfo(networkInfo));
+    }
+
+    private void queueOrSendNetworkInfo(NetworkInfo networkInfo) {
+        queueOrSendMessage(reg -> reg.sendNetworkInfo(networkInfo));
     }
 
     /**
@@ -731,8 +818,8 @@ public abstract class NetworkAgent {
         Objects.requireNonNull(networkCapabilities);
         mBandwidthUpdatePending.set(false);
         mLastBwRefreshTime = System.currentTimeMillis();
-        queueOrSendMessage(EVENT_NETWORK_CAPABILITIES_CHANGED,
-                new NetworkCapabilities(networkCapabilities));
+        final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities);
+        queueOrSendMessage(reg -> reg.sendNetworkCapabilities(nc));
     }
 
     /**
@@ -744,7 +831,7 @@ public abstract class NetworkAgent {
         if (score < 0) {
             throw new IllegalArgumentException("Score must be >= 0");
         }
-        queueOrSendMessage(EVENT_NETWORK_SCORE_CHANGED, score, 0);
+        queueOrSendMessage(reg -> reg.sendScore(score));
     }
 
     /**
@@ -784,9 +871,8 @@ public abstract class NetworkAgent {
      * @hide should move to NetworkAgentConfig.
      */
     public void explicitlySelected(boolean explicitlySelected, boolean acceptUnvalidated) {
-        queueOrSendMessage(EVENT_SET_EXPLICITLY_SELECTED,
-                explicitlySelected ? 1 : 0,
-                acceptUnvalidated ? 1 : 0);
+        queueOrSendMessage(reg -> reg.sendExplicitlySelected(
+                explicitlySelected, acceptUnvalidated));
     }
 
     /**
@@ -909,7 +995,7 @@ public abstract class NetworkAgent {
      */
     public final void sendSocketKeepaliveEvent(int slot,
             @SocketKeepalive.KeepaliveEvent int event) {
-        queueOrSendMessage(EVENT_SOCKET_KEEPALIVE, slot, event);
+        queueOrSendMessage(reg -> reg.sendSocketKeepaliveEvent(slot, event));
     }
     /** @hide TODO delete once callers have moved to sendSocketKeepaliveEvent */
     public void onSocketKeepaliveEvent(int slot, int reason) {
diff --git a/core/java/android/net/TcpKeepalivePacketData.aidl b/core/java/android/net/TcpKeepalivePacketData.aidl
new file mode 100644
index 000000000000..fdc7af9fed5c
--- /dev/null
+++ b/core/java/android/net/TcpKeepalivePacketData.aidl
@@ -0,0 +1,18 @@
+/**
+ * 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 android.net;
+
+@JavaOnlyStableParcelable parcelable TcpKeepalivePacketData;