Reset all maps in AndroidNetworkMonitor Start()/Stop()

This cl/ fixes another race condition with the recent additions
to NetworkMonitorAutoDetect (getAllNetworksFromCache).

The getAllNetworksFromCache-feature uses the by the Android team
preferred way of enumerating networks, i.e to register network listeners.

Th recent fix to add IsAdapterAvailable, https://webrtc-review.googlesource.com/c/src/+/257400
contained a bug in that the adapter_type_by_name_ map was not
reset either on disconnect or Start/Stop.

This cl/ addresses that including unit test.
It also de-obfuscates NetworkMonitor so that it always
calls NotifyOfActiveNetworkList on startMonitoring even
if list.size() == 0. This should not matter but makes
code easier to understand.

Bug: webrtc:13741
Change-Id: I438b877eebf769a8b2e7292b697ef1c0a349b24f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/258721
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36530}

sdk/android/api/org/webrtc/NetworkMonitor.java

@@ -138,10 +138,11 @@ public class NetworkMonitor {
     startMonitoring(
         applicationContext != null ? applicationContext : ContextUtils.getApplicationContext(),
         fieldTrialsString);
-    // The native observers expect a network list update after they call startMonitoring.
+
     synchronized (nativeNetworkObservers) {
       nativeNetworkObservers.add(nativeObserver);
     }
+    // The native observer expects a network list update after startMonitoring.
     updateObserverActiveNetworkList(nativeObserver);
     // currentConnectionType was updated in startMonitoring().
     // Need to notify the native observers here.
@@ -271,7 +272,7 @@ public class NetworkMonitor {
       networkInfoList =
           (networkChangeDetector == null) ? null : networkChangeDetector.getActiveNetworkList();
     }
-    if (networkInfoList == null || networkInfoList.size() == 0) {
+    if (networkInfoList == null) {
       return;
     }
 

sdk/android/native_unittests/android_network_monitor_unittest.cc

@@ -11,6 +11,7 @@
 #include "sdk/android/src/jni/android_network_monitor.h"
 
 #include "rtc_base/ip_address.h"
+#include "rtc_base/logging.h"
 #include "sdk/android/native_unittests/application_context_provider.h"
 #include "sdk/android/src/jni/jni_helpers.h"
 #include "test/gtest.h"
@@ -60,6 +61,10 @@ class AndroidNetworkMonitorTest : public ::testing::Test {
     network_monitor_->Stop();
   }
 
+  void Disconnect(jni::NetworkHandle handle) {
+    network_monitor_->OnNetworkDisconnected_n(handle);
+  }
+
  protected:
   test::ScopedKeyValueConfig field_trials_;
   std::unique_ptr<jni::AndroidNetworkMonitor> network_monitor_;
@@ -169,5 +174,63 @@ TEST_F(AndroidNetworkMonitorTest, TestUnderlyingVpnType) {
             network_monitor_->GetVpnUnderlyingAdapterType("v4-wlan0"));
 }
 
+// Verify that Disconnect makes interface unavailable.
+TEST_F(AndroidNetworkMonitorTest, Disconnect) {
+  network_monitor_->Start();
+
+  jni::NetworkHandle ipv4_handle = 100;
+  rtc::IPAddress ipv4_address(kTestIpv4Address);
+  jni::NetworkInformation net_info =
+      CreateNetworkInformation("wlan0", ipv4_handle, ipv4_address);
+  net_info.type = jni::NETWORK_WIFI;
+  network_monitor_->SetNetworkInfos({net_info});
+
+  EXPECT_TRUE(network_monitor_->IsAdapterAvailable("wlan0"));
+  EXPECT_TRUE(network_monitor_
+                  ->FindNetworkHandleFromAddressOrName(ipv4_address, "v4-wlan0")
+                  .has_value());
+  EXPECT_EQ(network_monitor_->GetAdapterType("v4-wlan0"),
+            rtc::ADAPTER_TYPE_WIFI);
+
+  // Check that values are reset on disconnect().
+  Disconnect(ipv4_handle);
+  EXPECT_FALSE(network_monitor_->IsAdapterAvailable("wlan0"));
+  EXPECT_FALSE(
+      network_monitor_
+          ->FindNetworkHandleFromAddressOrName(ipv4_address, "v4-wlan0")
+          .has_value());
+  EXPECT_EQ(network_monitor_->GetAdapterType("v4-wlan0"),
+            rtc::ADAPTER_TYPE_UNKNOWN);
+}
+
+// Verify that Stop() resets all caches.
+TEST_F(AndroidNetworkMonitorTest, Reset) {
+  network_monitor_->Start();
+
+  jni::NetworkHandle ipv4_handle = 100;
+  rtc::IPAddress ipv4_address(kTestIpv4Address);
+  jni::NetworkInformation net_info =
+      CreateNetworkInformation("wlan0", ipv4_handle, ipv4_address);
+  net_info.type = jni::NETWORK_WIFI;
+  network_monitor_->SetNetworkInfos({net_info});
+
+  EXPECT_TRUE(network_monitor_->IsAdapterAvailable("wlan0"));
+  EXPECT_TRUE(network_monitor_
+                  ->FindNetworkHandleFromAddressOrName(ipv4_address, "v4-wlan0")
+                  .has_value());
+  EXPECT_EQ(network_monitor_->GetAdapterType("v4-wlan0"),
+            rtc::ADAPTER_TYPE_WIFI);
+
+  // Check that values are reset on Stop().
+  network_monitor_->Stop();
+  EXPECT_FALSE(network_monitor_->IsAdapterAvailable("wlan0"));
+  EXPECT_FALSE(
+      network_monitor_
+          ->FindNetworkHandleFromAddressOrName(ipv4_address, "v4-wlan0")
+          .has_value());
+  EXPECT_EQ(network_monitor_->GetAdapterType("v4-wlan0"),
+            rtc::ADAPTER_TYPE_UNKNOWN);
+}
+
 }  // namespace test
 }  // namespace webrtc

sdk/android/src/jni/android_network_monitor.cc

@@ -243,6 +243,7 @@ void AndroidNetworkMonitor::Start() {
   if (started_) {
     return;
   }
+  reset();
   started_ = true;
   surface_cellular_types_ =
       field_trials_.IsEnabled("WebRTC-SurfaceCellularTypes");
@@ -265,6 +266,15 @@ void AndroidNetworkMonitor::Start() {
           env, field_trials_.Lookup("WebRTC-NetworkMonitorAutoDetect")));
 }
 
+void AndroidNetworkMonitor::reset() {
+  RTC_DCHECK_RUN_ON(network_thread_);
+  network_handle_by_address_.clear();
+  network_info_by_handle_.clear();
+  adapter_type_by_name_.clear();
+  vpn_underlying_adapter_type_by_name_.clear();
+  network_preference_by_adapter_type_.clear();
+}
+
 void AndroidNetworkMonitor::Stop() {
   RTC_DCHECK_RUN_ON(network_thread_);
   if (!started_) {
@@ -281,8 +291,7 @@ void AndroidNetworkMonitor::Stop() {
   Java_NetworkMonitor_stopMonitoring(env, j_network_monitor_,
                                      jlongFromPointer(this));
 
-  network_handle_by_address_.clear();
+  reset();
-  network_info_by_handle_.clear();
 }
 
 // The implementation is largely taken from UDPSocketPosix::BindToNetwork in
@@ -417,6 +426,7 @@ void AndroidNetworkMonitor::OnNetworkConnected_n(
   for (const rtc::IPAddress& address : network_info.ip_addresses) {
     network_handle_by_address_[address] = network_info.handle;
   }
+  RTC_CHECK(adapter_type_by_name_.size() == network_info_by_handle_.size());
   InvokeNetworksChangedCallback();
 }
 
@@ -472,8 +482,12 @@ void AndroidNetworkMonitor::OnNetworkDisconnected_n(NetworkHandle handle) {
     for (const rtc::IPAddress& address : iter->second.ip_addresses) {
       network_handle_by_address_.erase(address);
     }
+    adapter_type_by_name_.erase(iter->second.interface_name);
+    vpn_underlying_adapter_type_by_name_.erase(iter->second.interface_name);
     network_info_by_handle_.erase(iter);
   }
+
+  RTC_CHECK(adapter_type_by_name_.size() == network_info_by_handle_.size());
 }
 
 void AndroidNetworkMonitor::OnNetworkPreference_n(
@@ -491,8 +505,17 @@ void AndroidNetworkMonitor::OnNetworkPreference_n(
 void AndroidNetworkMonitor::SetNetworkInfos(
     const std::vector<NetworkInformation>& network_infos) {
   RTC_DCHECK_RUN_ON(network_thread_);
-  network_handle_by_address_.clear();
+
-  network_info_by_handle_.clear();
+  // We expect this method to be called once directly after startMonitoring.
+  // All the caches should be empty.
+  RTC_CHECK(network_handle_by_address_.empty());
+  RTC_CHECK(network_info_by_handle_.empty());
+  RTC_CHECK(adapter_type_by_name_.empty());
+  RTC_CHECK(vpn_underlying_adapter_type_by_name_.empty());
+  RTC_CHECK(network_preference_by_adapter_type_.empty());
+
+  // ...but reset just in case.
+  reset();
   RTC_LOG(LS_INFO) << "Android network monitor found " << network_infos.size()
                    << " networks";
   for (const NetworkInformation& network : network_infos) {

sdk/android/src/jni/android_network_monitor.h

@@ -29,6 +29,10 @@
 #include "sdk/android/src/jni/jni_helpers.h"
 
 namespace webrtc {
+namespace test {
+class AndroidNetworkMonitorTest;
+}  // namespace test
+
 namespace jni {
 
 typedef int64_t NetworkHandle;
@@ -120,6 +124,7 @@ class AndroidNetworkMonitor : public rtc::NetworkMonitorInterface {
       absl::string_view ifname) const;
 
  private:
+  void reset();
   void OnNetworkConnected_n(const NetworkInformation& network_info);
   void OnNetworkDisconnected_n(NetworkHandle network_handle);
   void OnNetworkPreference_n(NetworkType type,
@@ -163,6 +168,8 @@ class AndroidNetworkMonitor : public rtc::NetworkMonitorInterface {
       RTC_PT_GUARDED_BY(network_thread_) = nullptr;
 
   const FieldTrialsView& field_trials_;
+
+  friend class webrtc::test::AndroidNetworkMonitorTest;
 };
 
 class AndroidNetworkMonitorFactory : public rtc::NetworkMonitorFactory {