From 3e64739a76ca6130088817c6da189d53308a8e40 Mon Sep 17 00:00:00 2001 From: Jonas Oreland Date: Thu, 3 Mar 2022 13:50:50 +0100 Subject: [PATCH] Add support for caching networks based on NetworkCallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change adds a cache for networks in the SimpleNetworkCallback that is already registered, allowing the cache to be used preferentially as opposed to the deprecated getAllNetworks call. This is a fork of https://webrtc-review.googlesource.com/c/src/+/251401 - adds field trials for new behavior - removes test that did not work - add (poor) test of field trials - remove the "network_monitor_java" build target (that I could not find any reference to...) Bug: webrtc:13741 Change-Id: I2829c2f1940d4b42455d8e1a2217cf15c133e22b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/252284 Reviewed-by: Xavier Lepaul‎ Commit-Queue: Jonas Oreland Cr-Commit-Position: refs/heads/main@{#36121} --- sdk/android/BUILD.gn | 18 --- .../org/webrtc/NetworkMonitorAutoDetect.java | 104 ++++++++++++++---- .../src/org/webrtc/NetworkMonitorTest.java | 88 ++++++++++++++- 3 files changed, 169 insertions(+), 41 deletions(-) diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn index 85ec5f7f99..e0b8b0c81c 100644 --- a/sdk/android/BUILD.gn +++ b/sdk/android/BUILD.gn @@ -537,24 +537,6 @@ if (is_android) { "//third_party/androidx:androidx_annotation_annotation_java", ] } - - rtc_android_library("network_monitor_java") { - visibility = [ "*" ] - sources = [ - "api/org/webrtc/NetworkChangeDetector.java", - "api/org/webrtc/NetworkChangeDetectorFactory.java", - "api/org/webrtc/NetworkMonitor.java", - "api/org/webrtc/NetworkMonitorAutoDetect.java", - ] - deps = [ - ":base_java", - ":logging_java", - "//rtc_base:base_java", - "//third_party/android_deps:com_android_support_support_annotations_java", - "//third_party/androidx:androidx_annotation_annotation_java", - ] - srcjar_deps = [ "//rtc_base:network_monitor_enums" ] - } } if (current_os == "linux" || is_android) { diff --git a/sdk/android/api/org/webrtc/NetworkMonitorAutoDetect.java b/sdk/android/api/org/webrtc/NetworkMonitorAutoDetect.java index 771059eb08..052e7c80a9 100644 --- a/sdk/android/api/org/webrtc/NetworkMonitorAutoDetect.java +++ b/sdk/android/api/org/webrtc/NetworkMonitorAutoDetect.java @@ -29,13 +29,18 @@ import android.net.wifi.p2p.WifiP2pGroup; import android.net.wifi.p2p.WifiP2pManager; import android.os.Build; import android.telephony.TelephonyManager; +import androidx.annotation.GuardedBy; +import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import java.net.InetAddress; import java.net.NetworkInterface; import java.net.SocketException; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; /** * Borrowed from Chromium's @@ -87,16 +92,23 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo return underlyingNetworkSubtypeForVpn; } } - /** - * The methods in this class get called when the network changes if the callback - * is registered with a proper network request. It is only available in Android Lollipop - * and above. - */ + @SuppressLint("NewApi") - private class SimpleNetworkCallback extends NetworkCallback { + @VisibleForTesting() + class SimpleNetworkCallback extends NetworkCallback { + @GuardedBy("availableNetworks") final Set availableNetworks; + + SimpleNetworkCallback(Set availableNetworks) { + this.availableNetworks = availableNetworks; + } + @Override public void onAvailable(Network network) { Logging.d(TAG, "Network becomes available: " + network.toString()); + + synchronized (availableNetworks) { + availableNetworks.add(network); + } onNetworkChanged(network); } @@ -130,6 +142,10 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo @Override public void onLost(Network network) { Logging.d(TAG, "Network " + network.toString() + " is disconnected"); + + synchronized (availableNetworks) { + availableNetworks.remove(network); + } observer.onNetworkDisconnect(networkToNetId(network)); } @@ -149,15 +165,40 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo */ @Nullable private final ConnectivityManager connectivityManager; - ConnectivityManagerDelegate(Context context) { - connectivityManager = - (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE); + /** + * Note: The availableNetworks set is instantiated in NetworkMonitorAutoDetect + * and the instance is mutated by SimpleNetworkCallback. + */ + @NonNull @GuardedBy("availableNetworks") private final Set availableNetworks; + + /** field trials */ + private final boolean getAllNetworksFromCache; + private final boolean requestVPN; + private final boolean includeOtherUidNetworks; + + ConnectivityManagerDelegate(Context context, Set availableNetworks) { + this((ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE), + availableNetworks, + PeerConnectionFactory.fieldTrialsFindFullName("WebRTC-NetworkMonitorAutoDetect")); } - // For testing. - ConnectivityManagerDelegate() { - // All the methods below should be overridden. - connectivityManager = null; + @VisibleForTesting + ConnectivityManagerDelegate(ConnectivityManager connectivityManager, + Set availableNetworks, String fieldTrials) { + this.connectivityManager = connectivityManager; + this.availableNetworks = availableNetworks; + this.getAllNetworksFromCache = checkFieldTrial(fieldTrials, "getAllNetworksFromCache", false); + this.requestVPN = checkFieldTrial(fieldTrials, "requestVPN", false); + this.includeOtherUidNetworks = checkFieldTrial(fieldTrials, "includeOtherUidNetworks", false); + } + + private static boolean checkFieldTrial(String fieldTrials, String key, boolean defaultValue) { + if (fieldTrials.contains(key + "/Enabled")) { + return true; + } else if (fieldTrials.contains(key + "/Disabled")) { + return false; + } + return defaultValue; } /** @@ -265,6 +306,13 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo if (connectivityManager == null) { return new Network[0]; } + + if (supportNetworkCallback() && getAllNetworksFromCache) { + synchronized (availableNetworks) { + return availableNetworks.toArray(new Network[0]); + } + } + return connectivityManager.getAllNetworks(); } @@ -385,14 +433,26 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo && capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); } + @SuppressLint("NewApi") + @VisibleForTesting() + NetworkRequest createNetworkRequest() { + // Requests the following capabilities by default: NOT_VPN, NOT_RESTRICTED, TRUSTED + NetworkRequest.Builder builder = + new NetworkRequest.Builder().addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); + + if (requestVPN) { + builder.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN); + } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && includeOtherUidNetworks) { + builder.setIncludeOtherUidNetworks(true); + } + return builder.build(); + } + /** Only callable on Lollipop and newer releases. */ @SuppressLint("NewApi") public void registerNetworkCallback(NetworkCallback networkCallback) { - connectivityManager.registerNetworkCallback( - new NetworkRequest.Builder() - .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) - .build(), - networkCallback); + connectivityManager.registerNetworkCallback(createNetworkRequest(), networkCallback); } /** Only callable on Lollipop and newer releases. */ @@ -424,7 +484,7 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo } public boolean supportNetworkCallback() { - return Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP && connectivityManager != null; + return connectivityManager != null; } } @@ -567,6 +627,8 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo private WifiDirectManagerDelegate wifiDirectManagerDelegate; private static boolean includeWifiDirect; + @GuardedBy("availableNetworks") final Set availableNetworks = new HashSet<>(); + private boolean isRegistered; private NetworkChangeDetector.ConnectionType connectionType; private String wifiSSID; @@ -576,7 +638,7 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo public NetworkMonitorAutoDetect(NetworkChangeDetector.Observer observer, Context context) { this.observer = observer; this.context = context; - connectivityManagerDelegate = new ConnectivityManagerDelegate(context); + connectivityManagerDelegate = new ConnectivityManagerDelegate(context, availableNetworks); wifiManagerDelegate = new WifiManagerDelegate(context); final NetworkState networkState = connectivityManagerDelegate.getNetworkState(); @@ -600,7 +662,7 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo tempNetworkCallback = null; } mobileNetworkCallback = tempNetworkCallback; - allNetworkCallback = new SimpleNetworkCallback(); + allNetworkCallback = new SimpleNetworkCallback(availableNetworks); connectivityManagerDelegate.registerNetworkCallback(allNetworkCallback); } else { mobileNetworkCallback = null; diff --git a/sdk/android/instrumentationtests/src/org/webrtc/NetworkMonitorTest.java b/sdk/android/instrumentationtests/src/org/webrtc/NetworkMonitorTest.java index a30292c5cb..a80cd952c8 100644 --- a/sdk/android/instrumentationtests/src/org/webrtc/NetworkMonitorTest.java +++ b/sdk/android/instrumentationtests/src/org/webrtc/NetworkMonitorTest.java @@ -10,17 +10,22 @@ package org.webrtc; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; import android.annotation.SuppressLint; import android.content.Context; import android.content.Intent; import android.net.ConnectivityManager; import android.net.Network; +import android.net.NetworkCapabilities; +import android.net.NetworkRequest; import android.os.Build; import android.os.Handler; import android.os.Looper; @@ -28,7 +33,11 @@ import android.support.test.InstrumentationRegistry; import androidx.annotation.Nullable; import androidx.test.filters.MediumTest; import androidx.test.filters.SmallTest; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.chromium.base.test.BaseJUnit4ClassRunner; import org.chromium.base.test.UiThreadTest; import org.junit.Before; @@ -39,6 +48,7 @@ import org.webrtc.NetworkChangeDetector.ConnectionType; import org.webrtc.NetworkChangeDetector.NetworkInformation; import org.webrtc.NetworkMonitorAutoDetect.ConnectivityManagerDelegate; import org.webrtc.NetworkMonitorAutoDetect.NetworkState; +import org.webrtc.NetworkMonitorAutoDetect.SimpleNetworkCallback; /** * Tests for org.webrtc.NetworkMonitor. @@ -83,6 +93,14 @@ public class NetworkMonitorTest { private int underlyingNetworkTypeForVpn; private int underlyingNetworkSubtypeForVpn; + MockConnectivityManagerDelegate() { + this(new HashSet<>(), ""); + } + + MockConnectivityManagerDelegate(Set availableNetworks, String fieldTrialsString) { + super(null, availableNetworks, fieldTrialsString); + } + @Override public NetworkState getNetworkState() { return new NetworkState(activeNetworkExists, networkType, networkSubtype, @@ -275,8 +293,8 @@ public class NetworkMonitorTest { @UiThreadTest @SmallTest public void testConnectivityManagerDelegateDoesNotCrash() { - ConnectivityManagerDelegate delegate = - new ConnectivityManagerDelegate(InstrumentationRegistry.getTargetContext()); + ConnectivityManagerDelegate delegate = new ConnectivityManagerDelegate( + InstrumentationRegistry.getTargetContext(), new HashSet<>()); delegate.getNetworkState(); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { Network[] networks = delegate.getAllNetworks(); @@ -288,6 +306,72 @@ public class NetworkMonitorTest { } } + /** Tests that ConnectivityManagerDelegate preferentially reads from the cache */ + @Test + @SmallTest + public void testConnectivityManagerDelegatePreferentiallyReadsFromCache() { + final Set availableNetworks = new HashSet<>(); + ConnectivityManagerDelegate delegate = new ConnectivityManagerDelegate( + (ConnectivityManager) InstrumentationRegistry.getTargetContext().getSystemService( + Context.CONNECTIVITY_SERVICE), + availableNetworks, "getAllNetworksFromCache/Enabled/"); + + Network[] networks = delegate.getAllNetworks(); + assertTrue(networks.length == 0); + + final Network mockNetwork = mock(Network.class); + availableNetworks.add(mockNetwork); + + assertArrayEquals(new Network[] {mockNetwork}, delegate.getAllNetworks()); + } + + /** Tests field trial parsing */ + + @Test + @SmallTest + public void testConnectivityManager_requestVPN_disabled() { + NetworkRequest request = getNetworkRequestForFieldTrials("requestVPN/Disabled"); + assertTrue(request.equals(new NetworkRequest.Builder() + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + .build())); + } + + @Test + @SmallTest + public void testConnectivityManager_requestVPN_enabled() { + NetworkRequest request = getNetworkRequestForFieldTrials("requestVPN/Enabled"); + assertTrue(request.equals(new NetworkRequest.Builder() + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + .removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN) + .build())); + } + + @Test + @SmallTest + public void testConnectivityManager_includeOtherUidNetworks_disabled() { + NetworkRequest request = getNetworkRequestForFieldTrials("includeOtherUidNetworks/Disabled"); + assertTrue(request.equals(new NetworkRequest.Builder() + .addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET) + .build())); + } + + @Test + @SmallTest + public void testConnectivityManager_includeOtherUidNetworks_enabled() { + NetworkRequest request = getNetworkRequestForFieldTrials("includeOtherUidNetworks/Enabled"); + NetworkRequest.Builder builder = + new NetworkRequest.Builder().addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + builder.setIncludeOtherUidNetworks(true); + } + assertTrue(request.equals(builder.build())); + } + + private NetworkRequest getNetworkRequestForFieldTrials(String fieldTrials) { + return new ConnectivityManagerDelegate(null, new HashSet<>(), fieldTrials) + .createNetworkRequest(); + } + /** * Tests that NetworkMonitorAutoDetect queryable APIs don't crash. This test cannot rely * on having any active network connections so it cannot usefully check results, but it can at