From c7ea04af9146c8115f49cc8a5b5f1016c9dec8dd Mon Sep 17 00:00:00 2001
From: Jonas Oreland <jonaso@webrtc.org>
Date: Fri, 3 Apr 2020 10:12:28 +0200
Subject: [PATCH] Don't trigger OnNetworkChange when changing from 3G to 4G

This patch is a follow up to https://webrtc-review.googlesource.com/c/src/+/172582
and change so that a switch from CELLULAR_X to CELLULAR_Y does not
trigger OnNetworkChange.

This is needed as the OnNetworkChange signals triggers
BasicPortAllocator to rescan all networks and generate new candidates.

The actual adapter type change is still possible to react on using
SignalTypeChanged.

BUG: webrtc:11473
Change-Id: Icc1a945b8a4df1714c6ec4b02ec759ecada92d7f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/172802
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30992}
---
 rtc_base/network.cc          | 17 +++++++++-
 rtc_base/network.h           |  6 ++--
 rtc_base/network_unittest.cc | 61 ++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 3 deletions(-)

diff --git a/rtc_base/network.cc b/rtc_base/network.cc
index ffa8f94d66..f30063d991 100644
--- a/rtc_base/network.cc
+++ b/rtc_base/network.cc
@@ -147,6 +147,18 @@ bool IsIgnoredIPv6(const InterfaceAddress& ip) {
 }
 #endif  // !defined(__native_client__)
 
+// Note: consider changing to const Network* as arguments
+// if/when considering other changes that should not trigger
+// OnNetworksChanged.
+bool ShouldAdapterChangeTriggerNetworkChange(rtc::AdapterType old_type,
+                                             rtc::AdapterType new_type) {
+  // skip triggering OnNetworksChanged if
+  // changing from one cellular to another.
+  if (Network::IsCellular(old_type) && Network::IsCellular(new_type))
+    return false;
+  return true;
+}
+
 }  // namespace
 
 // These addresses are used as the targets to find out the default local address
@@ -350,8 +362,11 @@ void NetworkManagerBase::MergeNetworkList(const NetworkList& new_networks,
       merged_list.push_back(existing_net);
       if (net->type() != ADAPTER_TYPE_UNKNOWN &&
           net->type() != existing_net->type()) {
+        if (ShouldAdapterChangeTriggerNetworkChange(existing_net->type(),
+                                                    net->type())) {
+          *changed = true;
+        }
         existing_net->set_type(net->type());
-        *changed = true;
       }
       // If the existing network was not active, networks have changed.
       if (!existing_net->active()) {
diff --git a/rtc_base/network.h b/rtc_base/network.h
index ed601a518e..bd05b6ae16 100644
--- a/rtc_base/network.h
+++ b/rtc_base/network.h
@@ -409,8 +409,10 @@ class RTC_EXPORT Network {
 
   bool IsVpn() const { return type_ == ADAPTER_TYPE_VPN; }
 
-  bool IsCellular() const {
-    switch (type_) {
+  bool IsCellular() const { return IsCellular(type_); }
+
+  static bool IsCellular(AdapterType type) {
+    switch (type) {
       case ADAPTER_TYPE_CELLULAR:
       case ADAPTER_TYPE_CELLULAR_2G:
       case ADAPTER_TYPE_CELLULAR_3G:
diff --git a/rtc_base/network_unittest.cc b/rtc_base/network_unittest.cc
index d5aa8ac317..cd693563e7 100644
--- a/rtc_base/network_unittest.cc
+++ b/rtc_base/network_unittest.cc
@@ -1118,4 +1118,65 @@ TEST_F(NetworkTest, MAYBE_DefaultLocalAddress) {
   manager.StopUpdating();
 }
 
+// Test that MergeNetworkList does not set change = true
+// when changing from cellular_X to cellular_Y.
+TEST_F(NetworkTest, TestWhenNetworkListChangeReturnsChangedFlag) {
+  BasicNetworkManager manager;
+
+  IPAddress ip1;
+  EXPECT_TRUE(IPFromString("2400:4030:1:2c00:be30:0:0:1", &ip1));
+  Network* net1 = new Network("em1", "em1", TruncateIP(ip1, 64), 64);
+  net1->set_type(ADAPTER_TYPE_CELLULAR_3G);
+  net1->AddIP(ip1);
+  NetworkManager::NetworkList list;
+  list.push_back(net1);
+
+  {
+    bool changed;
+    MergeNetworkList(manager, list, &changed);
+    EXPECT_TRUE(changed);
+    NetworkManager::NetworkList list2;
+    manager.GetNetworks(&list2);
+    EXPECT_EQ(list2.size(), 1uL);
+    EXPECT_EQ(ADAPTER_TYPE_CELLULAR_3G, list2[0]->type());
+  }
+
+  // Modify net1 from 3G to 4G
+  {
+    Network* net2 = new Network("em1", "em1", TruncateIP(ip1, 64), 64);
+    net2->set_type(ADAPTER_TYPE_CELLULAR_4G);
+    net2->AddIP(ip1);
+    list.clear();
+    list.push_back(net2);
+    bool changed;
+    MergeNetworkList(manager, list, &changed);
+
+    // Change from 3G to 4G shall not trigger OnNetworksChanged,
+    // i.e changed = false.
+    EXPECT_FALSE(changed);
+    NetworkManager::NetworkList list2;
+    manager.GetNetworks(&list2);
+    ASSERT_EQ(list2.size(), 1uL);
+    EXPECT_EQ(ADAPTER_TYPE_CELLULAR_4G, list2[0]->type());
+  }
+
+  // Don't modify.
+  {
+    Network* net2 = new Network("em1", "em1", TruncateIP(ip1, 64), 64);
+    net2->set_type(ADAPTER_TYPE_CELLULAR_4G);
+    net2->AddIP(ip1);
+    list.clear();
+    list.push_back(net2);
+    bool changed;
+    MergeNetworkList(manager, list, &changed);
+
+    // No change.
+    EXPECT_FALSE(changed);
+    NetworkManager::NetworkList list2;
+    manager.GetNetworks(&list2);
+    ASSERT_EQ(list2.size(), 1uL);
+    EXPECT_EQ(ADAPTER_TYPE_CELLULAR_4G, list2[0]->type());
+  }
+}
+
 }  // namespace rtc