From 69955497d4bc265e919bd1eb196dc9a8480fbf75 Mon Sep 17 00:00:00 2001
From: markchien <markchien@google.com>
Date: Mon, 1 Oct 2018 21:18:15 +0800
Subject: [PATCH] Reduce excess local prefix computations

only recomputing on the LISTEN_ALL callback

Test: - build, flash, booted
      - runtest frameworks-net

bug: 110335330

Change-Id: I606574f1a8a2899ed4688d7d5ec2cbe0f2638a94
---
 .../tethering/UpstreamNetworkMonitor.java      | 18 ++++++++++++------
 .../tethering/UpstreamNetworkMonitorTest.java  | 12 ++++++++++++
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java b/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java
index f488be70af49c..3e5d5aa6ca541 100644
--- a/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java
+++ b/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java
@@ -399,9 +399,12 @@ public class UpstreamNetworkMonitor {
         @Override
         public void onLinkPropertiesChanged(Network network, LinkProperties newLp) {
             handleLinkProp(network, newLp);
-            // TODO(b/110335330): reduce the number of times this is called by
-            // only recomputing on the LISTEN_ALL callback.
-            recomputeLocalPrefixes();
+            // Any non-LISTEN_ALL callback will necessarily concern a network that will
+            // also match the LISTEN_ALL callback by construction of the LISTEN_ALL callback.
+            // So it's not useful to do this work for non-LISTEN_ALL callbacks.
+            if (mCallbackType == CALLBACK_LISTEN_ALL) {
+                recomputeLocalPrefixes();
+            }
         }
 
         @Override
@@ -417,9 +420,12 @@ public class UpstreamNetworkMonitor {
         @Override
         public void onLost(Network network) {
             handleLost(mCallbackType, network);
-            // TODO(b/110335330): reduce the number of times this is called by
-            // only recomputing on the LISTEN_ALL callback.
-            recomputeLocalPrefixes();
+            // Any non-LISTEN_ALL callback will necessarily concern a network that will
+            // also match the LISTEN_ALL callback by construction of the LISTEN_ALL callback.
+            // So it's not useful to do this work for non-LISTEN_ALL callbacks.
+            if (mCallbackType == CALLBACK_LISTEN_ALL) {
+                recomputeLocalPrefixes();
+            }
         }
     }
 
diff --git a/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java b/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java
index 3e21a2cfb7f9e..a22cbd4c95d81 100644
--- a/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java
+++ b/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java
@@ -475,6 +475,18 @@ public class UpstreamNetworkMonitorTest {
         assertPrefixSet(local, EXCLUDES, wifiLinkPrefixes);
         assertPrefixSet(local, INCLUDES, cellLinkPrefixes);
         assertPrefixSet(local, INCLUDES, dunLinkPrefixes);
+
+        // [5] Pretend mobile disconnected.
+        cellAgent.fakeDisconnect();
+        local = mUNM.getLocalPrefixes();
+        assertPrefixSet(local, EXCLUDES, wifiLinkPrefixes);
+        assertPrefixSet(local, EXCLUDES, cellLinkPrefixes);
+        assertPrefixSet(local, INCLUDES, dunLinkPrefixes);
+
+        // [6] Pretend DUN disconnected.
+        dunAgent.fakeDisconnect();
+        local = mUNM.getLocalPrefixes();
+        assertTrue(local.isEmpty());
     }
 
     private void assertSatisfiesLegacyType(int legacyType, NetworkState ns) {