From 54714779b267a636bc66c9dd17d48db4f6d726ad Mon Sep 17 00:00:00 2001 From: Artem Titov Date: Fri, 8 Apr 2022 15:09:03 +0000 Subject: [PATCH] Revert "WebRTC-DeprecateGlobalFieldTrialString/Enabled/ - part 14/inf" This reverts commit f177081eeeab64658fb560b6e8cb235620ac4a1a. Reason for revert: breaks downstream project Original change's description: > WebRTC-DeprecateGlobalFieldTrialString/Enabled/ - part 14/inf > > This cl/ passes field trials all the way from c++ > to the android NetworkMonitorAutoDetect.java > > Bug: webrtc:10335 > Change-Id: Ic6842612eed36b684340f0f78f4087bee249cc50 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/257081 > Reviewed-by: Harald Alvestrand > Reviewed-by: Florent Castelli > Commit-Queue: Jonas Oreland > Cr-Commit-Position: refs/heads/main@{#36498} Bug: webrtc:10335 Change-Id: I8d881ea3f50cf4affde65800d759885b2581c6be No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/258482 Reviewed-by: Jonas Oreland Owners-Override: Artem Titov Commit-Queue: Artem Titov Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Cr-Commit-Position: refs/heads/main@{#36500} --- sdk/android/BUILD.gn | 4 +-- .../webrtc/NetworkChangeDetectorFactory.java | 3 +- .../api/org/webrtc/NetworkMonitor.java | 36 +++++++------------ .../org/webrtc/NetworkMonitorAutoDetect.java | 29 +++++++-------- .../src/org/webrtc/NetworkMonitorTest.java | 25 ++++++------- .../android_network_monitor_unittest.cc | 14 ++++---- .../src/jni/android_network_monitor.cc | 24 ++++++------- sdk/android/src/jni/android_network_monitor.h | 5 +-- 8 files changed, 56 insertions(+), 84 deletions(-) diff --git a/sdk/android/BUILD.gn b/sdk/android/BUILD.gn index dda61cea08..9863d6132e 100644 --- a/sdk/android/BUILD.gn +++ b/sdk/android/BUILD.gn @@ -1631,7 +1631,6 @@ if (is_android) { ":native_api_video", ":opensles_audio_device_module", ":video_jni", - "../../api:field_trials_view", "../../api:scoped_refptr", "../../api/rtc_event_log:rtc_event_log_factory", "../../api/task_queue:default_task_queue_factory", @@ -1658,8 +1657,9 @@ if (is_android) { "../../rtc_base/synchronization:mutex", "../../rtc_base/system:inline", "../../system_wrappers", + "../../system_wrappers:field_trial", + "../../test:field_trial", "../../test:fileutils", - "../../test:scoped_key_value_config", "../../test:test_support", "../../testing/gtest", ] diff --git a/sdk/android/api/org/webrtc/NetworkChangeDetectorFactory.java b/sdk/android/api/org/webrtc/NetworkChangeDetectorFactory.java index 260a8fe12b..14e98b2387 100644 --- a/sdk/android/api/org/webrtc/NetworkChangeDetectorFactory.java +++ b/sdk/android/api/org/webrtc/NetworkChangeDetectorFactory.java @@ -13,6 +13,5 @@ package org.webrtc; import android.content.Context; public interface NetworkChangeDetectorFactory { - public NetworkChangeDetector create( - NetworkChangeDetector.Observer observer, Context context, String fieldTrialsString); + public NetworkChangeDetector create(NetworkChangeDetector.Observer observer, Context context); } diff --git a/sdk/android/api/org/webrtc/NetworkMonitor.java b/sdk/android/api/org/webrtc/NetworkMonitor.java index c3649c340d..9e14a2e00b 100644 --- a/sdk/android/api/org/webrtc/NetworkMonitor.java +++ b/sdk/android/api/org/webrtc/NetworkMonitor.java @@ -46,8 +46,8 @@ public class NetworkMonitor { new NetworkChangeDetectorFactory() { @Override public NetworkChangeDetector create( - NetworkChangeDetector.Observer observer, Context context, String fieldTrialsString) { - return new NetworkMonitorAutoDetect(observer, context, fieldTrialsString); + NetworkChangeDetector.Observer observer, Context context) { + return new NetworkMonitorAutoDetect(observer, context); } }; @@ -101,26 +101,20 @@ public class NetworkMonitor { * multi-networking. This requires the embedding app have the platform ACCESS_NETWORK_STATE and * CHANGE_NETWORK_STATE permission. */ - public void startMonitoring(Context applicationContext, String fieldTrialsString) { + public void startMonitoring(Context applicationContext) { synchronized (networkChangeDetectorLock) { ++numObservers; if (networkChangeDetector == null) { - networkChangeDetector = createNetworkChangeDetector(applicationContext, fieldTrialsString); + networkChangeDetector = createNetworkChangeDetector(applicationContext); } currentConnectionType = networkChangeDetector.getCurrentConnectionType(); } } - /** Deprecated, use startMonitoring with fieldTrialsStringString argument. */ - @Deprecated - public void startMonitoring(Context applicationContext) { - startMonitoring(applicationContext, ""); - } - /** Deprecated, pass in application context in startMonitoring instead. */ @Deprecated public void startMonitoring() { - startMonitoring(ContextUtils.getApplicationContext(), ""); + startMonitoring(ContextUtils.getApplicationContext()); } /** @@ -129,15 +123,11 @@ public class NetworkMonitor { * CHANGE_NETWORK_STATE permission. */ @CalledByNative - private void startMonitoring( - @Nullable Context applicationContext, long nativeObserver, String fieldTrialsString) { - Logging.d(TAG, - "Start monitoring with native observer " + nativeObserver - + " fieldTrialsString: " + fieldTrialsString); + private void startMonitoring(@Nullable Context applicationContext, long nativeObserver) { + Logging.d(TAG, "Start monitoring with native observer " + nativeObserver); startMonitoring( - applicationContext != null ? applicationContext : ContextUtils.getApplicationContext(), - fieldTrialsString); + applicationContext != null ? applicationContext : ContextUtils.getApplicationContext()); // The native observers expect a network list update after they call startMonitoring. synchronized (nativeNetworkObservers) { nativeNetworkObservers.add(nativeObserver); @@ -187,8 +177,7 @@ public class NetworkMonitor { return currentConnectionType; } - private NetworkChangeDetector createNetworkChangeDetector( - Context appContext, String fieldTrialsString) { + private NetworkChangeDetector createNetworkChangeDetector(Context appContext) { return networkChangeDetectorFactory.create(new NetworkChangeDetector.Observer() { @Override public void onConnectionTypeChanged(NetworkChangeDetector.ConnectionType newConnectionType) { @@ -210,7 +199,7 @@ public class NetworkMonitor { List types, int preference) { notifyObserversOfNetworkPreference(types, preference); } - }, appContext, fieldTrialsString); + }, appContext); } private void updateCurrentConnectionType(NetworkChangeDetector.ConnectionType newConnectionType) { @@ -350,11 +339,10 @@ public class NetworkMonitor { } // For testing only. - static NetworkMonitorAutoDetect createAndSetAutoDetectForTest( - Context context, String fieldTrialsString) { + static NetworkMonitorAutoDetect createAndSetAutoDetectForTest(Context context) { NetworkMonitor networkMonitor = getInstance(); NetworkChangeDetector networkChangeDetector = - networkMonitor.createNetworkChangeDetector(context, fieldTrialsString); + networkMonitor.createNetworkChangeDetector(context); networkMonitor.networkChangeDetector = networkChangeDetector; return (NetworkMonitorAutoDetect) networkChangeDetector; } diff --git a/sdk/android/api/org/webrtc/NetworkMonitorAutoDetect.java b/sdk/android/api/org/webrtc/NetworkMonitorAutoDetect.java index 644ea95f54..750dcb303c 100644 --- a/sdk/android/api/org/webrtc/NetworkMonitorAutoDetect.java +++ b/sdk/android/api/org/webrtc/NetworkMonitorAutoDetect.java @@ -176,29 +176,26 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo private final boolean requestVPN; private final boolean includeOtherUidNetworks; - ConnectivityManagerDelegate( - Context context, Set availableNetworks, String fieldTrialsString) { + ConnectivityManagerDelegate(Context context, Set availableNetworks) { this((ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE), - availableNetworks, fieldTrialsString); + availableNetworks, + PeerConnectionFactory.fieldTrialsFindFullName("WebRTC-NetworkMonitorAutoDetect")); } @VisibleForTesting ConnectivityManagerDelegate(ConnectivityManager connectivityManager, - Set availableNetworks, String fieldTrialsString) { + Set availableNetworks, String fieldTrials) { this.connectivityManager = connectivityManager; this.availableNetworks = availableNetworks; - this.getAllNetworksFromCache = - checkFieldTrial(fieldTrialsString, "getAllNetworksFromCache", false); - this.requestVPN = checkFieldTrial(fieldTrialsString, "requestVPN", false); - this.includeOtherUidNetworks = - checkFieldTrial(fieldTrialsString, "includeOtherUidNetworks", false); + this.getAllNetworksFromCache = checkFieldTrial(fieldTrials, "getAllNetworksFromCache", false); + this.requestVPN = checkFieldTrial(fieldTrials, "requestVPN", false); + this.includeOtherUidNetworks = checkFieldTrial(fieldTrials, "includeOtherUidNetworks", false); } - private static boolean checkFieldTrial( - String fieldTrialsString, String key, boolean defaultValue) { - if (fieldTrialsString.contains(key + ":true")) { + private static boolean checkFieldTrial(String fieldTrials, String key, boolean defaultValue) { + if (fieldTrials.contains(key + ":true")) { return true; - } else if (fieldTrialsString.contains(key + ":false")) { + } else if (fieldTrials.contains(key + ":false")) { return false; } return defaultValue; @@ -638,12 +635,10 @@ public class NetworkMonitorAutoDetect extends BroadcastReceiver implements Netwo /** Constructs a NetworkMonitorAutoDetect. Should only be called on UI thread. */ @SuppressLint("NewApi") - public NetworkMonitorAutoDetect( - NetworkChangeDetector.Observer observer, Context context, String fieldTrialsString) { + public NetworkMonitorAutoDetect(NetworkChangeDetector.Observer observer, Context context) { this.observer = observer; this.context = context; - connectivityManagerDelegate = - new ConnectivityManagerDelegate(context, availableNetworks, fieldTrialsString); + connectivityManagerDelegate = new ConnectivityManagerDelegate(context, availableNetworks); wifiManagerDelegate = new WifiManagerDelegate(context); final NetworkState networkState = connectivityManagerDelegate.getNetworkState(); diff --git a/sdk/android/instrumentationtests/src/org/webrtc/NetworkMonitorTest.java b/sdk/android/instrumentationtests/src/org/webrtc/NetworkMonitorTest.java index 96f2fcbdae..fc87806dec 100644 --- a/sdk/android/instrumentationtests/src/org/webrtc/NetworkMonitorTest.java +++ b/sdk/android/instrumentationtests/src/org/webrtc/NetworkMonitorTest.java @@ -58,7 +58,6 @@ import org.webrtc.NetworkMonitorAutoDetect.SimpleNetworkCallback; public class NetworkMonitorTest { private static final long INVALID_NET_ID = -1; private NetworkChangeDetector detector; - private String fieldTrialsString = ""; /** * Listens for alerts fired by the NetworkMonitor when network status changes. @@ -95,7 +94,7 @@ public class NetworkMonitorTest { } MockConnectivityManagerDelegate(Set availableNetworks, String fieldTrialsString) { - super((ConnectivityManager) null, availableNetworks, fieldTrialsString); + super(null, availableNetworks, fieldTrialsString); } @Override @@ -190,13 +189,13 @@ public class NetworkMonitorTest { new NetworkChangeDetectorFactory() { @Override public NetworkChangeDetector create( - NetworkChangeDetector.Observer observer, Context context, String fieldTrialsString) { - detector = new NetworkMonitorAutoDetect(observer, context, fieldTrialsString); + NetworkChangeDetector.Observer observer, Context context) { + detector = new NetworkMonitorAutoDetect(observer, context); return detector; } }); - receiver = NetworkMonitor.createAndSetAutoDetectForTest(context, fieldTrialsString); + receiver = NetworkMonitor.createAndSetAutoDetectForTest(context); assertNotNull(receiver); connectivityDelegate = new MockConnectivityManagerDelegate(); @@ -229,8 +228,7 @@ public class NetworkMonitorTest { NetworkMonitorAutoDetect.Observer observer = new TestNetworkMonitorAutoDetectObserver(); - NetworkMonitorAutoDetect receiver = - new NetworkMonitorAutoDetect(observer, context, fieldTrialsString); + NetworkMonitorAutoDetect receiver = new NetworkMonitorAutoDetect(observer, context); assertTrue(receiver.isReceiverRegisteredForTesting()); } @@ -289,7 +287,7 @@ public class NetworkMonitorTest { @SmallTest public void testConnectivityManagerDelegateDoesNotCrash() { ConnectivityManagerDelegate delegate = new ConnectivityManagerDelegate( - InstrumentationRegistry.getTargetContext(), new HashSet<>(), fieldTrialsString); + InstrumentationRegistry.getTargetContext(), new HashSet<>()); delegate.getNetworkState(); Network[] networks = delegate.getAllNetworks(); if (networks.length >= 1) { @@ -361,9 +359,8 @@ public class NetworkMonitorTest { assertTrue(request.equals(builder.build())); } - private NetworkRequest getNetworkRequestForFieldTrials(String fieldTrialsString) { - return new ConnectivityManagerDelegate( - (ConnectivityManager) null, new HashSet<>(), fieldTrialsString) + private NetworkRequest getNetworkRequestForFieldTrials(String fieldTrials) { + return new ConnectivityManagerDelegate(null, new HashSet<>(), fieldTrials) .createNetworkRequest(); } @@ -376,8 +373,8 @@ public class NetworkMonitorTest { @SmallTest public void testQueryableAPIsDoNotCrash() { NetworkMonitorAutoDetect.Observer observer = new TestNetworkMonitorAutoDetectObserver(); - NetworkMonitorAutoDetect ncn = new NetworkMonitorAutoDetect( - observer, InstrumentationRegistry.getTargetContext(), fieldTrialsString); + NetworkMonitorAutoDetect ncn = + new NetworkMonitorAutoDetect(observer, InstrumentationRegistry.getTargetContext()); ncn.getDefaultNetId(); } @@ -389,7 +386,7 @@ public class NetworkMonitorTest { public void testStartStopMonitoring() { NetworkMonitor networkMonitor = NetworkMonitor.getInstance(); Context context = ContextUtils.getApplicationContext(); - networkMonitor.startMonitoring(context, fieldTrialsString); + networkMonitor.startMonitoring(context); assertEquals(1, networkMonitor.getNumObservers()); assertEquals(detector, networkMonitor.getNetworkChangeDetector()); networkMonitor.stopMonitoring(); diff --git a/sdk/android/native_unittests/android_network_monitor_unittest.cc b/sdk/android/native_unittests/android_network_monitor_unittest.cc index 129168cce2..c342ce692e 100644 --- a/sdk/android/native_unittests/android_network_monitor_unittest.cc +++ b/sdk/android/native_unittests/android_network_monitor_unittest.cc @@ -13,8 +13,9 @@ #include "rtc_base/ip_address.h" #include "sdk/android/native_unittests/application_context_provider.h" #include "sdk/android/src/jni/jni_helpers.h" +#include "system_wrappers/include/field_trial.h" +#include "test/field_trial.h" #include "test/gtest.h" -#include "test/scoped_key_value_config.h" namespace webrtc { namespace test { @@ -46,8 +47,8 @@ class AndroidNetworkMonitorTest : public ::testing::Test { AndroidNetworkMonitorTest() { JNIEnv* env = AttachCurrentThreadIfNeeded(); ScopedJavaLocalRef context = test::GetAppContextForTest(env); - network_monitor_ = std::make_unique( - env, context, field_trials_); + network_monitor_ = + std::make_unique(env, context); } void SetUp() override { @@ -61,7 +62,6 @@ class AndroidNetworkMonitorTest : public ::testing::Test { } protected: - test::ScopedKeyValueConfig field_trials_; std::unique_ptr network_monitor_; }; @@ -102,8 +102,7 @@ TEST_F(AndroidNetworkMonitorTest, TestFindNetworkHandleUsingFullIpv6Address) { TEST_F(AndroidNetworkMonitorTest, TestFindNetworkHandleIgnoringIpv6TemporaryPart) { - ScopedKeyValueConfig field_trials( - field_trials_, + ScopedFieldTrials field_trials( "WebRTC-FindNetworkHandleWithoutIpv6TemporaryPart/Enabled/"); // Start() updates the states introduced by the field trial. network_monitor_->Start(); @@ -155,8 +154,7 @@ TEST_F(AndroidNetworkMonitorTest, TestFindNetworkHandleUsingIfName) { } TEST_F(AndroidNetworkMonitorTest, TestUnderlyingVpnType) { - ScopedKeyValueConfig field_trials(field_trials_, - "WebRTC-BindUsingInterfaceName/Enabled/"); + ScopedFieldTrials field_trials("WebRTC-BindUsingInterfaceName/Enabled/"); jni::NetworkHandle ipv4_handle = 100; rtc::IPAddress ipv4_address(kTestIpv4Address); jni::NetworkInformation net_info = diff --git a/sdk/android/src/jni/android_network_monitor.cc b/sdk/android/src/jni/android_network_monitor.cc index 59a9204fe2..67206c8505 100644 --- a/sdk/android/src/jni/android_network_monitor.cc +++ b/sdk/android/src/jni/android_network_monitor.cc @@ -28,6 +28,7 @@ #include "sdk/android/generated_base_jni/NetworkMonitor_jni.h" #include "sdk/android/native_api/jni/java_types.h" #include "sdk/android/src/jni/jni_helpers.h" +#include "system_wrappers/include/field_trial.h" namespace webrtc { namespace jni { @@ -226,13 +227,11 @@ std::string NetworkInformation::ToString() const { AndroidNetworkMonitor::AndroidNetworkMonitor( JNIEnv* env, - const JavaRef& j_application_context, - const FieldTrialsView& field_trials) + const JavaRef& j_application_context) : android_sdk_int_(Java_NetworkMonitor_androidSdkInt(env)), j_application_context_(env, j_application_context), j_network_monitor_(env, Java_NetworkMonitor_getInstance(env)), - network_thread_(rtc::Thread::Current()), - field_trials_(field_trials) {} + network_thread_(rtc::Thread::Current()) {} AndroidNetworkMonitor::~AndroidNetworkMonitor() { RTC_DCHECK(!started_); @@ -245,12 +244,13 @@ void AndroidNetworkMonitor::Start() { } started_ = true; surface_cellular_types_ = - field_trials_.IsEnabled("WebRTC-SurfaceCellularTypes"); - find_network_handle_without_ipv6_temporary_part_ = field_trials_.IsEnabled( - "WebRTC-FindNetworkHandleWithoutIpv6TemporaryPart"); + webrtc::field_trial::IsEnabled("WebRTC-SurfaceCellularTypes"); + find_network_handle_without_ipv6_temporary_part_ = + webrtc::field_trial::IsEnabled( + "WebRTC-FindNetworkHandleWithoutIpv6TemporaryPart"); bind_using_ifname_ = - !field_trials_.IsDisabled("WebRTC-BindUsingInterfaceName"); - disable_is_adapter_available_ = field_trials_.IsDisabled( + !webrtc::field_trial::IsDisabled("WebRTC-BindUsingInterfaceName"); + disable_is_adapter_available_ = webrtc::field_trial::IsDisabled( "WebRTC-AndroidNetworkMonitor-IsAdapterAvailable"); // This pointer is also accessed by the methods called from java threads. @@ -260,9 +260,7 @@ void AndroidNetworkMonitor::Start() { JNIEnv* env = AttachCurrentThreadIfNeeded(); Java_NetworkMonitor_startMonitoring( - env, j_network_monitor_, j_application_context_, jlongFromPointer(this), - NativeToJavaString( - env, field_trials_.Lookup("WebRTC-NetworkMonitorAutoDetect"))); + env, j_network_monitor_, j_application_context_, jlongFromPointer(this)); } void AndroidNetworkMonitor::Stop() { @@ -610,7 +608,7 @@ rtc::NetworkMonitorInterface* AndroidNetworkMonitorFactory::CreateNetworkMonitor( const FieldTrialsView& field_trials) { return new AndroidNetworkMonitor(AttachCurrentThreadIfNeeded(), - j_application_context_, field_trials); + j_application_context_); } void AndroidNetworkMonitor::NotifyConnectionTypeChanged( diff --git a/sdk/android/src/jni/android_network_monitor.h b/sdk/android/src/jni/android_network_monitor.h index dd661acdcf..a7cb766c0c 100644 --- a/sdk/android/src/jni/android_network_monitor.h +++ b/sdk/android/src/jni/android_network_monitor.h @@ -70,8 +70,7 @@ struct NetworkInformation { class AndroidNetworkMonitor : public rtc::NetworkMonitorInterface { public: AndroidNetworkMonitor(JNIEnv* env, - const JavaRef& j_application_context, - const FieldTrialsView& field_trials); + const JavaRef& j_application_context); ~AndroidNetworkMonitor() override; // TODO(sakal): Remove once down stream dependencies have been updated. @@ -161,8 +160,6 @@ class AndroidNetworkMonitor : public rtc::NetworkMonitorInterface { rtc::scoped_refptr safety_flag_ RTC_PT_GUARDED_BY(network_thread_) = nullptr; - - const FieldTrialsView& field_trials_; }; class AndroidNetworkMonitorFactory : public rtc::NetworkMonitorFactory {