Merge changes Ib0c4a52e,Iff7d2981,Ie8af8b71,I7884faf8 into main

* changes:
  Rename mActiveTetheringRequests to mPendingTetheringRequests
  Give the two variants of ensureIpServerStarted different names.
  Move the TetheringRequest further up the stack.
  Un-nest legacyTether by moving lambda contents to new method.
diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java
index 11b14ed..3fcf356 100644
--- a/Tethering/src/com/android/networkstack/tethering/Tethering.java
+++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java
@@ -241,7 +241,7 @@
     // Currently active tethering requests per tethering type. Only one of each type can be
     // requested at a time. After a tethering type is requested, the map keeps tethering parameters
     // to be used after the interface comes up asynchronously.
-    private final SparseArray<TetheringRequest> mActiveTetheringRequests =
+    private final SparseArray<TetheringRequest> mPendingTetheringRequests =
             new SparseArray<>();
 
     private final Context mContext;
@@ -640,7 +640,7 @@
         }
 
         if (enabled) {
-            ensureIpServerStarted(iface);
+            ensureIpServerStartedForInterface(iface);
         } else {
             ensureIpServerStopped(iface);
         }
@@ -701,7 +701,7 @@
             final IIntResultListener listener) {
         mHandler.post(() -> {
             final int type = request.getTetheringType();
-            final TetheringRequest unfinishedRequest = mActiveTetheringRequests.get(type);
+            final TetheringRequest unfinishedRequest = mPendingTetheringRequests.get(type);
             // If tethering is already enabled with a different request,
             // disable before re-enabling.
             if (unfinishedRequest != null && !unfinishedRequest.equalsIgnoreUidPackage(request)) {
@@ -709,7 +709,7 @@
                         unfinishedRequest.getInterfaceName(), null);
                 mEntitlementMgr.stopProvisioningIfNeeded(type);
             }
-            mActiveTetheringRequests.put(type, request);
+            mPendingTetheringRequests.put(type, request);
 
             if (request.isExemptFromEntitlementCheck()) {
                 mEntitlementMgr.setExemptedDownstreamType(type);
@@ -728,7 +728,7 @@
         });
     }
     void stopTetheringInternal(int type) {
-        mActiveTetheringRequests.remove(type);
+        mPendingTetheringRequests.remove(type);
 
         enableTetheringInternal(type, false /* disabled */, null, null);
         mEntitlementMgr.stopProvisioningIfNeeded(type);
@@ -782,7 +782,7 @@
         // If changing tethering fail, remove corresponding request
         // no matter who trigger the start/stop.
         if (result != TETHER_ERROR_NO_ERROR) {
-            mActiveTetheringRequests.remove(type);
+            mPendingTetheringRequests.remove(type);
             mTetheringMetrics.updateErrorCode(type, result);
             mTetheringMetrics.sendReport(type);
         }
@@ -944,7 +944,9 @@
         public void onAvailable(String iface) {
             if (this != mBluetoothCallback) return;
 
-            enableIpServing(TETHERING_BLUETOOTH, iface, getRequestedState(TETHERING_BLUETOOTH));
+            final TetheringRequest request = getPendingTetheringRequest(TETHERING_BLUETOOTH);
+            enableIpServing(request, TETHERING_BLUETOOTH, iface,
+                    getRequestedState(TETHERING_BLUETOOTH));
             mConfiguredBluetoothIface = iface;
         }
 
@@ -999,7 +1001,10 @@
                 // Ethernet callback arrived after Ethernet tethering stopped. Ignore.
                 return;
             }
-            enableIpServing(TETHERING_ETHERNET, iface, getRequestedState(TETHERING_ETHERNET));
+
+            final TetheringRequest request = getPendingTetheringRequest(TETHERING_ETHERNET);
+            enableIpServing(request, TETHERING_ETHERNET, iface,
+                    getRequestedState(TETHERING_ETHERNET));
             mConfiguredEthernetIface = iface;
         }
 
@@ -1020,7 +1025,9 @@
             } else {
                 mConfiguredVirtualIface = iface;
             }
+            final TetheringRequest request = getPendingTetheringRequest(TETHERING_VIRTUAL);
             enableIpServing(
+                    request,
                     TETHERING_VIRTUAL,
                     mConfiguredVirtualIface,
                     getRequestedState(TETHERING_VIRTUAL));
@@ -1064,7 +1071,7 @@
      */
     @NonNull
     private TetheringRequest getOrCreatePendingTetheringRequest(int type) {
-        TetheringRequest pending = mActiveTetheringRequests.get(type);
+        TetheringRequest pending = mPendingTetheringRequests.get(type);
         if (pending != null) return pending;
 
         Log.w(TAG, "No pending TetheringRequest for type " + type + " found, creating a placeholder"
@@ -1074,6 +1081,59 @@
         return placeholder;
     }
 
+    private void handleLegacyTether(String iface, int requestedState,
+            final IIntResultListener listener) {
+        if (Build.VERSION.SDK_INT > Build.VERSION_CODES.VANILLA_ICE_CREAM) {
+            // After V, the TetheringManager and ConnectivityManager tether and untether methods
+            // throw UnsupportedOperationException, so this cannot happen in normal use. Ensure
+            // that this code cannot run even if callers use raw binder calls or other
+            // unsupported methods.
+            return;
+        }
+
+        final int type = ifaceNameToType(iface);
+        if (type == TETHERING_INVALID) {
+            Log.e(TAG, "Ignoring call to legacy tether for unknown iface " + iface);
+            try {
+                listener.onResult(TETHER_ERROR_UNKNOWN_IFACE);
+            } catch (RemoteException e) { }
+        }
+
+        int result = tetherInternal(null, iface, requestedState);
+        switch (type) {
+            case TETHERING_WIFI:
+                TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
+                        "Legacy tether API called on Wifi iface " + iface,
+                        CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
+                        CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI);
+                if (result == TETHER_ERROR_NO_ERROR) {
+                    TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
+                            "Legacy tether API succeeded on Wifi iface " + iface,
+                            CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
+                            CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_SUCCESS);
+                }
+                break;
+            case TETHERING_WIFI_P2P:
+                TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
+                        "Legacy tether API called on Wifi P2P iface " + iface,
+                        CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
+                        CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P);
+                if (result == TETHER_ERROR_NO_ERROR) {
+                    TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
+                            "Legacy tether API succeeded on Wifi P2P iface " + iface,
+                            CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
+                            CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P_SUCCESS);
+                }
+                break;
+            default:
+                // Do nothing
+                break;
+        }
+        try {
+            listener.onResult(result);
+        } catch (RemoteException e) { }
+    }
+
     /**
      * Legacy tether API that starts tethering with CONNECTIVITY_SCOPE_GLOBAL on the given iface.
      *
@@ -1088,51 +1148,14 @@
      * those broadcasts are disabled by OEM.
      */
     void legacyTether(String iface, int requestedState, final IIntResultListener listener) {
-        if (Build.VERSION.SDK_INT > Build.VERSION_CODES.VANILLA_ICE_CREAM) {
-            // After V, the TetheringManager and ConnectivityManager tether and untether methods
-            // throw UnsupportedOperationException, so this cannot happen in normal use. Ensure
-            // that this code cannot run even if callers use raw binder calls or other
-            // unsupported methods.
-            return;
-        }
-        mHandler.post(() -> {
-            int result = tetherInternal(iface, requestedState);
-            switch (ifaceNameToType(iface)) {
-                case TETHERING_WIFI:
-                    TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
-                            "Legacy tether API called on Wifi iface " + iface,
-                            CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
-                            CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI);
-                    if (result == TETHER_ERROR_NO_ERROR) {
-                        TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
-                                "Legacy tether API succeeded on Wifi iface " + iface,
-                                CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
-                                CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_SUCCESS);
-                    }
-                    break;
-                case TETHERING_WIFI_P2P:
-                    TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
-                            "Legacy tether API called on Wifi P2P iface " + iface,
-                            CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
-                            CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P);
-                    if (result == TETHER_ERROR_NO_ERROR) {
-                        TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
-                                "Legacy tether API succeeded on Wifi P2P iface " + iface,
-                                CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
-                                CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P_SUCCESS);
-                    }
-                    break;
-                default:
-                    // Do nothing
-                    break;
-            }
-            try {
-                listener.onResult(result);
-            } catch (RemoteException e) { }
-        });
+        mHandler.post(() -> handleLegacyTether(iface, requestedState, listener));
     }
 
-    private int tetherInternal(String iface, int requestedState) {
+    // TODO: is it possible to make the request @NonNull here?
+    // Because the IpServer doesn't actually look at (almost anything in) the request, we can just
+    // make callers synthesize a reasonable-looking request and make this @NonNull.
+    private int tetherInternal(@Nullable TetheringRequest request, String iface,
+            int requestedState) {
         if (DBG) Log.d(TAG, "Tethering " + iface);
         TetherState tetherState = mTetherStates.get(iface);
         if (tetherState == null) {
@@ -1145,15 +1168,19 @@
             Log.e(TAG, "Tried to Tether an unavailable iface: " + iface + ", ignoring");
             return TETHER_ERROR_UNAVAIL_IFACE;
         }
-        // NOTE: If a CMD_TETHER_REQUESTED message is already in the TISM's queue but not yet
+        // NOTE: If a CMD_TETHER_REQUESTED message is already in the IpServer's queue but not yet
         // processed, this will be a no-op and it will not return an error.
         //
         // This code cannot race with untether() because they both run on the handler thread.
+        //
+        // TODO: once the request is @NonNull, it will be correct to fetch the type from it, because
+        // the type used to fetch (or synthesize) the request is the same as the type passed in at
+        // IpServer creation time.
+        // BUG: the connectivity scope in the request is ignored. This matches current behaviour.
+        // TODO: see if the connectivity scope can be made (or is already) correct, and fetch the
+        // requested state from there.
         final int type = tetherState.ipServer.interfaceType();
-        final TetheringRequest request = mActiveTetheringRequests.get(type, null);
-        if (request != null) {
-            mActiveTetheringRequests.delete(type);
-        }
+        mPendingTetheringRequests.remove(type);
         tetherState.ipServer.enable(requestedState, request);
         return TETHER_ERROR_NO_ERROR;
     }
@@ -1213,8 +1240,9 @@
         return mEntitlementMgr.isTetherProvisioningRequired(cfg);
     }
 
+    // TODO: make this take a TetheringRequest instead.
     private int getRequestedState(int type) {
-        final TetheringRequest request = mActiveTetheringRequests.get(type);
+        final TetheringRequest request = mPendingTetheringRequests.get(type);
 
         // The request could have been deleted before we had a chance to complete it.
         // If so, assume that the scope is the default scope for this tethering type.
@@ -1529,8 +1557,8 @@
     }
 
     @VisibleForTesting
-    SparseArray<TetheringRequest> getActiveTetheringRequests() {
-        return mActiveTetheringRequests;
+    SparseArray<TetheringRequest> getPendingTetheringRequests() {
+        return mPendingTetheringRequests;
     }
 
     @VisibleForTesting
@@ -1590,14 +1618,21 @@
         }
     }
 
-    private void enableIpServing(int tetheringType, String ifname, int ipServingMode) {
-        enableIpServing(tetheringType, ifname, ipServingMode, false /* isNcm */);
+    final TetheringRequest getPendingTetheringRequest(int type) {
+        return mPendingTetheringRequests.get(type, null);
     }
 
-    private void enableIpServing(int tetheringType, String ifname, int ipServingMode,
-            boolean isNcm) {
-        ensureIpServerStarted(ifname, tetheringType, isNcm);
-        if (tetherInternal(ifname, ipServingMode) != TETHER_ERROR_NO_ERROR) {
+    // TODO: make the request @NonNull and move the tetheringType and ipServingMode into it.
+    private void enableIpServing(@Nullable TetheringRequest request, int tetheringType,
+            String ifname, int ipServingMode) {
+        enableIpServing(request, tetheringType, ifname, ipServingMode, false /* isNcm */);
+    }
+
+    // TODO: make the request @NonNull and move the tetheringType and ipServingMode into it.
+    private void enableIpServing(@Nullable TetheringRequest request, int tetheringType,
+            String ifname, int ipServingMode, boolean isNcm) {
+        ensureIpServerStartedForType(ifname, tetheringType, isNcm);
+        if (tetherInternal(request, ifname, ipServingMode) != TETHER_ERROR_NO_ERROR) {
             Log.e(TAG, "unable start tethering on iface " + ifname);
         }
     }
@@ -1649,7 +1684,9 @@
             mLog.e(ifname + " is not a tetherable iface, ignoring");
             return;
         }
-        enableIpServing(type, ifname, IpServer.STATE_LOCAL_ONLY);
+        // No need to look for active requests. There can never be explicit requests for
+        // TETHERING_WIFI_P2P because enableTetheringInternal ignores that type.
+        enableIpServing(null, type, ifname, IpServer.STATE_LOCAL_ONLY);
     }
 
     private void disableWifiP2pIpServingIfNeeded(String ifname) {
@@ -1659,17 +1696,35 @@
         disableWifiIpServingCommon(TETHERING_WIFI_P2P, ifname);
     }
 
+    // TODO: fold this in to enableWifiIpServing.  We cannot do this at the moment because there
+    // are tests that send wifi AP broadcasts with a null interface. But if this can't happen on
+    // real devices, we should fix those tests to always pass in an interface.
+    private int maybeInferWifiTetheringType(String ifname) {
+        return SdkLevel.isAtLeastT() ? TETHERING_WIFI : ifaceNameToType(ifname);
+    }
+
     private void enableWifiIpServing(String ifname, int wifiIpMode) {
         mLog.log("request WiFi tethering - interface=" + ifname + " state=" + wifiIpMode);
 
         // Map wifiIpMode values to IpServer.Callback serving states.
+        TetheringRequest request;
         final int ipServingMode;
+        final int type;
         switch (wifiIpMode) {
             case IFACE_IP_MODE_TETHERED:
                 ipServingMode = IpServer.STATE_TETHERED;
+                type = maybeInferWifiTetheringType(ifname);
+                request = getPendingTetheringRequest(type);
                 break;
             case IFACE_IP_MODE_LOCAL_ONLY:
                 ipServingMode = IpServer.STATE_LOCAL_ONLY;
+                type = maybeInferWifiTetheringType(ifname);
+                // BUG: this request is incorrect - instead of LOHS, it will reflect whatever
+                // request (if any) is being processed for TETHERING_WIFI. However, this is the
+                // historical behaviour. It mostly works because a) most of the time there is no
+                // such request b) tetherinternal doesn't look at the connectivity scope of the
+                // request, it takes the scope from requestedState.
+                request = getPendingTetheringRequest(type);
                 break;
             default:
                 mLog.e("Cannot enable IP serving in unknown WiFi mode: " + wifiIpMode);
@@ -1678,14 +1733,13 @@
 
         // After T, tethering always trust the iface pass by state change intent. This allow
         // tethering to deprecate tetherable wifi regexs after T.
-        final int type = SdkLevel.isAtLeastT() ? TETHERING_WIFI : ifaceNameToType(ifname);
         if (!checkTetherableType(type)) {
             mLog.e(ifname + " is not a tetherable iface, ignoring");
             return;
         }
 
         if (!TextUtils.isEmpty(ifname)) {
-            enableIpServing(type, ifname, ipServingMode);
+            enableIpServing(request, type, ifname, ipServingMode);
         } else {
             mLog.e("Cannot enable IP serving on missing interface name");
         }
@@ -1715,10 +1769,11 @@
             return;
         }
 
+        final TetheringRequest request = getPendingTetheringRequest(tetheringType);
         if (ifaces != null) {
             for (String iface : ifaces) {
                 if (ifaceNameToType(iface) == tetheringType) {
-                    enableIpServing(tetheringType, iface, requestedState, forNcmFunction);
+                    enableIpServing(request, tetheringType, iface, requestedState, forNcmFunction);
                     return;
                 }
             }
@@ -2950,7 +3005,7 @@
         return type != TETHERING_INVALID;
     }
 
-    private void ensureIpServerStarted(final String iface) {
+    private void ensureIpServerStartedForInterface(final String iface) {
         // If we don't care about this type of interface, ignore.
         final int interfaceType = ifaceNameToType(iface);
         if (!checkTetherableType(interfaceType)) {
@@ -2959,10 +3014,11 @@
             return;
         }
 
-        ensureIpServerStarted(iface, interfaceType, false /* isNcm */);
+        ensureIpServerStartedForType(iface, interfaceType, false /* isNcm */);
     }
 
-    private void ensureIpServerStarted(final String iface, int interfaceType, boolean isNcm) {
+    private void ensureIpServerStartedForType(final String iface, int interfaceType,
+            boolean isNcm) {
         // If we have already started a TISM for this interface, skip.
         if (mTetherStates.containsKey(iface)) {
             mLog.log("active iface (" + iface + ") reported as added, ignoring");
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
index e50a7fd..866b219 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
@@ -957,8 +957,8 @@
         mTethering.startTethering(request, TEST_CALLER_PKG, null);
         mLooper.dispatchAll();
 
-        assertEquals(1, mTethering.getActiveTetheringRequests().size());
-        assertEquals(request, mTethering.getActiveTetheringRequests().get(TETHERING_USB));
+        assertEquals(1, mTethering.getPendingTetheringRequests().size());
+        assertEquals(request, mTethering.getPendingTetheringRequests().get(TETHERING_USB));
 
         if (mTethering.getTetheringConfiguration().isUsingNcm()) {
             verify(mUsbManager).setCurrentFunctions(UsbManager.FUNCTION_NCM);
@@ -2170,7 +2170,7 @@
         runUsbTethering(upstreamState);
         assertContains(Arrays.asList(mTethering.getTetheredIfaces()), TEST_RNDIS_IFNAME);
         assertTrue(mTethering.isTetheringActive());
-        assertEquals(0, mTethering.getActiveTetheringRequests().size());
+        assertEquals(0, mTethering.getPendingTetheringRequests().size());
 
         final Tethering.UserRestrictionActionListener ural = makeUserRestrictionActionListener(
                 mTethering, false /* currentDisallow */, true /* nextDisallow */);