Merge changes I99d494d3,I208ceceb

* changes:
  [DK4-0]Add CM#setTestLowTcpPollingTimerForKeepalive for testing
  [DK3] Send onPause/onResume keepalive callbacks

framework/src/android/net/ConnectivityManager.java

@@ -1501,6 +1501,22 @@ public class ConnectivityManager {
         }
     }
 
+    /**
+     * Temporarily set automaticOnOff keeplaive TCP polling alarm timer to 1 second.
+     *
+     * TODO: Remove this when the TCP polling design is replaced with callback.
+     * @params timeMs The time of expiry, with System.currentTimeMillis() base. The value should be
+     *                set no more than 5 minutes in the future.
+     * @hide
+     */
+    public void setTestLowTcpPollingTimerForKeepalive(long timeMs) {
+        try {
+            mService.setTestLowTcpPollingTimerForKeepalive(timeMs);
+        } catch (RemoteException e) {
+            throw e.rethrowFromSystemServer();
+        }
+    }
+
     /**
      * Informs ConnectivityService of whether the legacy lockdown VPN, as implemented by
      * LockdownVpnTracker, is in use. This is deprecated for new devices starting from Android 12
@@ -2213,9 +2229,13 @@ public class ConnectivityManager {
         /** The requested keepalive was successfully started. */
         @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
         public void onStarted() {}
+        /** The keepalive was resumed after being paused by the system. */
+        public void onResumed() {}
         /** The keepalive was successfully stopped. */
         @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
         public void onStopped() {}
+        /** The keepalive was paused automatically by the system. */
+        public void onPaused() {}
         /** An error occurred. */
         @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
         public void onError(int error) {}
@@ -2313,6 +2333,18 @@ public class ConnectivityManager {
                     }
                 }
 
+                @Override
+                public void onResumed() {
+                    final long token = Binder.clearCallingIdentity();
+                    try {
+                        mExecutor.execute(() -> {
+                            callback.onResumed();
+                        });
+                    } finally {
+                        Binder.restoreCallingIdentity(token);
+                    }
+                }
+
                 @Override
                 public void onStopped() {
                     final long token = Binder.clearCallingIdentity();
@@ -2326,6 +2358,19 @@ public class ConnectivityManager {
                     mExecutor.shutdown();
                 }
 
+                @Override
+                public void onPaused() {
+                    final long token = Binder.clearCallingIdentity();
+                    try {
+                        mExecutor.execute(() -> {
+                            callback.onPaused();
+                        });
+                    } finally {
+                        Binder.restoreCallingIdentity(token);
+                    }
+                    mExecutor.shutdown();
+                }
+
                 @Override
                 public void onError(int error) {
                     final long token = Binder.clearCallingIdentity();

framework/src/android/net/IConnectivityManager.aidl

@@ -251,4 +251,6 @@ interface IConnectivityManager
     IBinder getCompanionDeviceManagerProxyService();
 
     void setVpnNetworkPreference(String session, in UidRange[] ranges);
+
+    void setTestLowTcpPollingTimerForKeepalive(long timeMs);
 }

framework/src/android/net/ISocketKeepaliveCallback.aidl

@@ -31,4 +31,8 @@ oneway interface ISocketKeepaliveCallback
     void onError(int error);
     /** The keepalive on a TCP socket was stopped because the socket received data. */
     void onDataReceived();
+    /** The keepalive was paused by the system because it's not necessary right now. */
+    void onPaused();
+    /** The keepalive was resumed by the system after being suspended. */
+    void onResumed();
 }

framework/src/android/net/SocketKeepalive.java

@@ -63,6 +63,12 @@ public abstract class SocketKeepalive implements AutoCloseable {
     @SystemApi
     public static final int SUCCESS = 0;
 
+    /**
+     * Success when trying to suspend.
+     * @hide
+     */
+    public static final int SUCCESS_PAUSED = 1;
+
     /**
      * No keepalive. This should only be internally as it indicates There is no keepalive.
      * It should not propagate to applications.
@@ -270,6 +276,18 @@ public abstract class SocketKeepalive implements AutoCloseable {
                 }
             }
 
+            @Override
+            public void onResumed() {
+                final long token = Binder.clearCallingIdentity();
+                try {
+                    mExecutor.execute(() -> {
+                        callback.onResumed();
+                    });
+                } finally {
+                    Binder.restoreCallingIdentity(token);
+                }
+            }
+
             @Override
             public void onStopped() {
                 final long token = Binder.clearCallingIdentity();
@@ -282,6 +300,18 @@ public abstract class SocketKeepalive implements AutoCloseable {
                 }
             }
 
+            @Override
+            public void onPaused() {
+                final long token = Binder.clearCallingIdentity();
+                try {
+                    executor.execute(() -> {
+                        callback.onPaused();
+                    });
+                } finally {
+                    Binder.restoreCallingIdentity(token);
+                }
+            }
+
             @Override
             public void onError(int error) {
                 final long token = Binder.clearCallingIdentity();
@@ -387,8 +417,18 @@ public abstract class SocketKeepalive implements AutoCloseable {
     public static class Callback {
         /** The requested keepalive was successfully started. */
         public void onStarted() {}
+        /**
+         * The keepalive was resumed by the system after being suspended.
+         * @hide
+         **/
+        public void onResumed() {}
         /** The keepalive was successfully stopped. */
         public void onStopped() {}
+        /**
+         * The keepalive was paused by the system because it's not necessary right now.
+         * @hide
+         **/
+        public void onPaused() {}
         /** An error occurred. */
         public void onError(@ErrorCode int error) {}
         /** The keepalive on a TCP socket was stopped because the socket received data. This is

service/src/com/android/server/ConnectivityService.java

@@ -765,6 +765,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
      */
     private static final int EVENT_SET_VPN_NETWORK_PREFERENCE = 59;
 
+    /**
+     * Event to use low TCP polling timer used in automatic on/off keepalive temporarily.
+     */
+    private static final int EVENT_SET_LOW_TCP_POLLING_UNTIL = 60;
+
     /**
      * Argument for {@link #EVENT_PROVISIONING_NOTIFICATION} to indicate that the notification
      * should be shown.
@@ -782,6 +787,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
      */
     private static final long MAX_TEST_ALLOW_BAD_WIFI_UNTIL_MS = 5 * 60 * 1000L;
 
+    /**
+     * The maximum alive time to decrease TCP polling timer in automatic on/off keepalive for
+     * testing.
+     */
+    private static final long MAX_TEST_LOW_TCP_POLLING_UNTIL_MS = 5 * 60 * 1000L;
+
     /**
      * The priority of the tc police rate limiter -- smaller value is higher priority.
      * This value needs to be coordinated with PRIO_CLAT, PRIO_TETHER4, and PRIO_TETHER6.
@@ -5001,6 +5012,22 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 mHandler.obtainMessage(EVENT_SET_TEST_ALLOW_BAD_WIFI_UNTIL, timeMs));
     }
 
+    @Override
+    public void setTestLowTcpPollingTimerForKeepalive(long timeMs) {
+        enforceSettingsPermission();
+        if (!Build.isDebuggable()) {
+            throw new IllegalStateException("Is not supported in non-debuggable build");
+        }
+
+        if (timeMs > System.currentTimeMillis() + MAX_TEST_LOW_TCP_POLLING_UNTIL_MS) {
+            throw new IllegalArgumentException("Argument should not exceed "
+                    + MAX_TEST_LOW_TCP_POLLING_UNTIL_MS + "ms from now");
+        }
+
+        mHandler.sendMessage(
+                mHandler.obtainMessage(EVENT_SET_LOW_TCP_POLLING_UNTIL, timeMs));
+    }
+
     private void handleSetAcceptUnvalidated(Network network, boolean accept, boolean always) {
         if (DBG) log("handleSetAcceptUnvalidated network=" + network +
                 " accept=" + accept + " always=" + always);
@@ -5642,6 +5669,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
                 case EVENT_SET_VPN_NETWORK_PREFERENCE:
                     handleSetVpnNetworkPreference((VpnNetworkPreferenceInfo) msg.obj);
                     break;
+                case EVENT_SET_LOW_TCP_POLLING_UNTIL: {
+                    final long time = ((Long) msg.obj).longValue();
+                    mKeepaliveTracker.handleSetTestLowTcpPollingTimer(time);
+                    break;
+                }
             }
         }
     }

service/src/com/android/server/connectivity/AutomaticOnOffKeepaliveTracker.java

@@ -18,7 +18,7 @@ package com.android.server.connectivity;
 
 import static android.net.NetworkAgent.CMD_START_SOCKET_KEEPALIVE;
 import static android.net.SocketKeepalive.ERROR_INVALID_SOCKET;
-import static android.net.SocketKeepalive.SUCCESS;
+import static android.net.SocketKeepalive.SUCCESS_PAUSED;
 import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;
 import static android.system.OsConstants.AF_INET;
 import static android.system.OsConstants.AF_INET6;
@@ -98,6 +98,7 @@ public class AutomaticOnOffKeepaliveTracker {
             "com.android.server.connectivity.KeepaliveTracker.TCP_POLLING_ALARM";
     private static final String EXTRA_BINDER_TOKEN = "token";
     private static final long DEFAULT_TCP_POLLING_INTERVAL_MS = 120_000L;
+    private static final long LOW_TCP_POLLING_INTERVAL_MS = 1_000L;
     private static final String AUTOMATIC_ON_OFF_KEEPALIVE_VERSION =
             "automatic_on_off_keepalive_version";
     /**
@@ -156,6 +157,8 @@ public class AutomaticOnOffKeepaliveTracker {
      * This should be only updated in ConnectivityService handler thread.
      */
     private final ArrayList<AutomaticOnOffKeepalive> mAutomaticOnOffKeepalives = new ArrayList<>();
+    // TODO: Remove this when TCP polling design is replaced with callback.
+    private long mTestLowTcpPollingTimerUntilMs = 0;
 
     private final BroadcastReceiver mReceiver = new BroadcastReceiver() {
         @Override
@@ -182,7 +185,7 @@ public class AutomaticOnOffKeepaliveTracker {
      *                     resumed if a TCP socket is open on the VPN.
      * See the documentation for the states for more detail.
      */
-    public class AutomaticOnOffKeepalive {
+    public class AutomaticOnOffKeepalive implements IBinder.DeathRecipient {
         @NonNull
         private final KeepaliveTracker.KeepaliveInfo mKi;
         @NonNull
@@ -239,6 +242,18 @@ public class AutomaticOnOffKeepaliveTracker {
             return BinderUtils.withCleanCallingIdentity(() -> PendingIntent.getBroadcast(
                     context, 0 /* requestCode */, intent, PendingIntent.FLAG_IMMUTABLE));
         }
+
+        @Override
+        public void binderDied() {
+            mConnectivityServiceHandler.post(() -> cleanupAutoOnOffKeepalive(this));
+        }
+
+        /** Close this automatic on/off keepalive */
+        public void close() {
+            // Close the duplicated fd that maintains the lifecycle of socket. If this fd was
+            // not duplicated this is a no-op.
+            FileUtils.closeQuietly(mFd);
+        }
     }
 
     public AutomaticOnOffKeepaliveTracker(@NonNull Context context, @NonNull Handler handler) {
@@ -264,7 +279,7 @@ public class AutomaticOnOffKeepaliveTracker {
 
     private void startTcpPollingAlarm(@NonNull PendingIntent alarm) {
         final long triggerAtMillis =
-                SystemClock.elapsedRealtime() + DEFAULT_TCP_POLLING_INTERVAL_MS;
+                SystemClock.elapsedRealtime() + getTcpPollingInterval();
         // Setup a non-wake up alarm.
         mAlarmManager.setExact(AlarmManager.ELAPSED_REALTIME, triggerAtMillis, alarm);
     }
@@ -297,7 +312,7 @@ public class AutomaticOnOffKeepaliveTracker {
             // SUSPENDED.
             if (ki.mAutomaticOnOffState == STATE_ENABLED) {
                 ki.mAutomaticOnOffState = STATE_SUSPENDED;
-                handleSuspendKeepalive(ki.mKi);
+                handlePauseKeepalive(ki.mKi);
             }
         } else {
             handleMaybeResumeKeepalive(ki);
@@ -374,6 +389,13 @@ public class AutomaticOnOffKeepaliveTracker {
         mKeepaliveTracker.handleStartKeepalive(autoKi.mKi);
 
         // Add automatic on/off request into list to track its life cycle.
+        try {
+            autoKi.mKi.mCallback.asBinder().linkToDeath(autoKi, 0);
+        } catch (RemoteException e) {
+            // The underlying keepalive performs its own cleanup
+            autoKi.binderDied();
+            return;
+        }
         mAutomaticOnOffKeepalives.add(autoKi);
         if (STATE_ALWAYS_ON != autoKi.mAutomaticOnOffState) {
             startTcpPollingAlarm(autoKi.mTcpPollingAlarm);
@@ -384,10 +406,9 @@ public class AutomaticOnOffKeepaliveTracker {
         mKeepaliveTracker.handleStartKeepalive(ki);
     }
 
-    private void handleSuspendKeepalive(@NonNull final KeepaliveTracker.KeepaliveInfo ki) {
+    private void handlePauseKeepalive(@NonNull final KeepaliveTracker.KeepaliveInfo ki) {
         // TODO : mKT.handleStopKeepalive should take a KeepaliveInfo instead
-        // TODO : create a separate success code for suspend
+        mKeepaliveTracker.handleStopKeepalive(ki.getNai(), ki.getSlot(), SUCCESS_PAUSED);
-        mKeepaliveTracker.handleStopKeepalive(ki.getNai(), ki.getSlot(), SUCCESS);
     }
 
     /**
@@ -399,6 +420,8 @@ public class AutomaticOnOffKeepaliveTracker {
         if (autoKi.mAutomaticOnOffState != STATE_SUSPENDED) {
             final KeepaliveTracker.KeepaliveInfo ki = autoKi.mKi;
             mKeepaliveTracker.handleStopKeepalive(ki.getNai(), ki.getSlot(), reason);
+        } else {
+            mKeepaliveTracker.finalizePausedKeepalive(autoKi.mKi);
         }
 
         cleanupAutoOnOffKeepalive(autoKi);
@@ -406,10 +429,15 @@ public class AutomaticOnOffKeepaliveTracker {
 
     private void cleanupAutoOnOffKeepalive(@NonNull final AutomaticOnOffKeepalive autoKi) {
         ensureRunningOnHandlerThread();
+        autoKi.close();
         if (null != autoKi.mTcpPollingAlarm) mAlarmManager.cancel(autoKi.mTcpPollingAlarm);
-        // Close the duplicated fd that maintains the lifecycle of socket.
+
-        FileUtils.closeQuietly(autoKi.mFd);
+        // If the KI is not in the array, it's because it was already removed, or it was never
-        mAutomaticOnOffKeepalives.remove(autoKi);
+        // added ; the only ways this can happen is if the keepalive is stopped by the app and the
+        // app dies immediately, or if the app died before the link to death could be registered.
+        if (!mAutomaticOnOffKeepalives.remove(autoKi)) return;
+
+        autoKi.mKi.mCallback.asBinder().unlinkToDeath(autoKi, 0);
     }
 
     /**
@@ -632,6 +660,20 @@ public class AutomaticOnOffKeepaliveTracker {
         }
     }
 
+    private long getTcpPollingInterval() {
+        final boolean useLowTimer = mTestLowTcpPollingTimerUntilMs > System.currentTimeMillis();
+        return useLowTimer ? LOW_TCP_POLLING_INTERVAL_MS : DEFAULT_TCP_POLLING_INTERVAL_MS;
+    }
+
+    /**
+     * Temporarily use low TCP polling timer for testing.
+     * The value works when the time set is more than {@link System.currentTimeMillis()}.
+     */
+    public void handleSetTestLowTcpPollingTimer(long timeMs) {
+        Log.d(TAG, "handleSetTestLowTcpPollingTimer: " + timeMs);
+        mTestLowTcpPollingTimerUntilMs = timeMs;
+    }
+
     /**
      * Dependencies class for testing.
      */

service/src/com/android/server/connectivity/KeepaliveTracker.java

@@ -32,6 +32,7 @@ import static android.net.SocketKeepalive.MAX_INTERVAL_SEC;
 import static android.net.SocketKeepalive.MIN_INTERVAL_SEC;
 import static android.net.SocketKeepalive.NO_KEEPALIVE;
 import static android.net.SocketKeepalive.SUCCESS;
+import static android.net.SocketKeepalive.SUCCESS_PAUSED;
 
 import android.annotation.NonNull;
 import android.annotation.Nullable;
@@ -134,6 +135,9 @@ public class KeepaliveTracker {
         public final NetworkAgentInfo mNai;
         private final int mType;
         public final FileDescriptor mFd;
+        // True if this was resumed from a previously turned off keepalive, otherwise false.
+        // This is necessary to send the correct callbacks.
+        public final boolean mResumed;
 
         public static final int TYPE_NATT = 1;
         public static final int TYPE_TCP = 2;
@@ -160,6 +164,16 @@ public class KeepaliveTracker {
                 int interval,
                 int type,
                 @Nullable FileDescriptor fd) throws InvalidSocketException {
+            this(callback, nai, packet, interval, type, fd, false /* resumed */);
+        }
+
+        KeepaliveInfo(@NonNull ISocketKeepaliveCallback callback,
+                @NonNull NetworkAgentInfo nai,
+                @NonNull KeepalivePacketData packet,
+                int interval,
+                int type,
+                @Nullable FileDescriptor fd,
+                boolean resumed) throws InvalidSocketException {
             mCallback = callback;
             mPid = Binder.getCallingPid();
             mUid = Binder.getCallingUid();
@@ -169,6 +183,7 @@ public class KeepaliveTracker {
             mPacket = packet;
             mInterval = interval;
             mType = type;
+            mResumed = resumed;
 
             // For SocketKeepalive, a dup of fd is kept in mFd so the source port from which the
             // keepalives are sent cannot be reused by another app even if the fd gets closed by
@@ -232,6 +247,8 @@ public class KeepaliveTracker {
 
         /** Called when the application process is killed. */
         public void binderDied() {
+            // TODO b/267106526 : this is not called on the handler thread but stop() happily
+            // assumes it is, which means this is a pretty dangerous race condition.
             stop(BINDER_DIED);
         }
 
@@ -327,6 +344,10 @@ public class KeepaliveTracker {
         }
 
         void start(int slot) {
+            // BINDER_DIED can happen if the binder died before the KeepaliveInfo was created and
+            // the constructor set the state to BINDER_DIED. If that's the case, the KI is already
+            // cleaned up.
+            if (BINDER_DIED == mStartedState) return;
             mSlot = slot;
             int error = isValid();
             if (error == SUCCESS) {
@@ -371,7 +392,10 @@ public class KeepaliveTracker {
             // To prevent races from re-entrance of stop(), return if the state is already stopping.
             // This might happen if multiple event sources stop keepalive in a short time. Such as
             // network disconnect after user calls stop(), or tear down socket after binder died.
-            if (mStartedState == STOPPING) return;
+            // Note that it's always possible this method is called by the auto keepalive timer
+            // or any other way after the binder died, hence the check for BINDER_DIED. If the
+            // binder has died, then the KI has already been cleaned up.
+            if (mStartedState == STOPPING || mStartedState == BINDER_DIED) return;
 
             // Store the reason of stopping, and report it after the keepalive is fully stopped.
             if (mStopReason != ERROR_STOP_REASON_UNINITIALIZED) {
@@ -382,9 +406,10 @@ public class KeepaliveTracker {
                     + ": " + reason);
             switch (mStartedState) {
                 case NOT_STARTED:
-                    // Remove the reference of the keepalive that meet error before starting,
+                    // Remove the reference to this keepalive that had an error before starting,
                     // e.g. invalid parameter.
                     cleanupStoppedKeepalive(mNai, mSlot);
+                    if (BINDER_DIED == reason) mStartedState = BINDER_DIED;
                     break;
                 default:
                     mStartedState = STOPPING;
@@ -422,7 +447,8 @@ public class KeepaliveTracker {
          * Construct a new KeepaliveInfo from existing KeepaliveInfo with a new fd.
          */
         public KeepaliveInfo withFd(@NonNull FileDescriptor fd) throws InvalidSocketException {
-            return new KeepaliveInfo(mCallback, mNai, mPacket, mInterval, mType, fd);
+            return new KeepaliveInfo(mCallback, mNai, mPacket, mInterval, mType, fd,
+                    true /* resumed */);
         }
     }
 
@@ -523,6 +549,12 @@ public class KeepaliveTracker {
             } catch (RemoteException e) {
                 Log.w(TAG, "Discarded onStop callback: " + reason);
             }
+        } else if (reason == SUCCESS_PAUSED) {
+            try {
+                ki.mCallback.onPaused();
+            } catch (RemoteException e) {
+                Log.w(TAG, "Discarded onPaused callback: " + reason);
+            }
         } else if (reason == DATA_RECEIVED) {
             try {
                 ki.mCallback.onDataReceived();
@@ -540,6 +572,25 @@ public class KeepaliveTracker {
         ki.unlinkDeathRecipient();
     }
 
+    /**
+     * Finalize a paused keepalive.
+     *
+     * This will simply send the onStopped() callback after checking that this keepalive is
+     * indeed paused.
+     *
+     * @param ki the keepalive to finalize
+     */
+    public void finalizePausedKeepalive(@NonNull final KeepaliveInfo ki) {
+        if (SUCCESS_PAUSED != ki.mStopReason) {
+            throw new IllegalStateException("Keepalive is not paused");
+        }
+        try {
+            ki.mCallback.onStopped();
+        } catch (RemoteException e) {
+            Log.w(TAG, "Discarded onStopped callback while finalizing paused keepalive");
+        }
+    }
+
     public void handleCheckKeepalivesStillValid(NetworkAgentInfo nai) {
         HashMap <Integer, KeepaliveInfo> networkKeepalives = mKeepalives.get(nai);
         if (networkKeepalives != null) {
@@ -589,9 +640,14 @@ public class KeepaliveTracker {
                 Log.d(TAG, "Started keepalive " + slot + " on " + nai.toShortString());
                 ki.mStartedState = KeepaliveInfo.STARTED;
                 try {
+                    if (ki.mResumed) {
+                        ki.mCallback.onResumed();
+                    } else {
                         ki.mCallback.onStarted();
+                    }
                 } catch (RemoteException e) {
-                    Log.w(TAG, "Discarded onStarted callback");
+                    Log.w(TAG, "Discarded " + (ki.mResumed ? "onResumed" : "onStarted")
+                            + " callback for slot " + slot);
                 }
             } else {
                 Log.d(TAG, "Failed to start keepalive " + slot + " on " + nai.toShortString()