Merge changes I178ed839,Iff78d02a into udc-dev

* changes:
  Fix cache flush always causing response change
  Adjust the notifyNetworkUnrequested on handleRegisterListener


Former-commit-id: 177dade553

service-t/src/com/android/server/connectivity/mdns/MdnsDiscoveryManager.java

@@ -214,6 +214,9 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback {
 
     private void handleUnregisterListener(
             @NonNull String serviceType, @NonNull MdnsServiceBrowserListener listener) {
+        // Unrequested the network.
+        socketClient.notifyNetworkUnrequested(listener);
+
         final List<MdnsServiceTypeClient> serviceTypeClients =
                 perNetworkServiceTypeClients.getByServiceType(serviceType);
         if (serviceTypeClients.isEmpty()) {
@@ -231,8 +234,6 @@ public class MdnsDiscoveryManager implements MdnsSocketClientBase.Callback {
             // No discovery request. Stops the socket client.
             socketClient.stopDiscovery();
         }
-        // Unrequested the network.
-        socketClient.notifyNetworkUnrequested(listener);
     }
 
     @Override

service-t/src/com/android/server/connectivity/mdns/MdnsResponse.java

@@ -95,6 +95,14 @@ public class MdnsResponse {
         return !isSame;
     }
 
+    /**
+     * @return True if this response contains an identical (original TTL included) record.
+     */
+    public boolean hasIdenticalRecord(@NonNull MdnsRecord record) {
+        final int existing = records.indexOf(record);
+        return existing >= 0 && recordsAreSame(record, records.get(existing));
+    }
+
     /**
      * Adds a pointer record.
      *

service-t/src/com/android/server/connectivity/mdns/MdnsResponseDecoder.java

@@ -20,6 +20,7 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.net.Network;
 import android.os.SystemClock;
+import android.util.ArrayMap;
 import android.util.ArraySet;
 import android.util.Pair;
 
@@ -139,8 +140,11 @@ public class MdnsResponseDecoder {
 
         final ArraySet<MdnsResponse> modified = new ArraySet<>();
         final ArrayList<MdnsResponse> responses = new ArrayList<>(existingResponses.size());
+        final ArrayMap<MdnsResponse, MdnsResponse> augmentedToOriginal = new ArrayMap<>();
         for (MdnsResponse existing : existingResponses) {
-            responses.add(new MdnsResponse(existing));
+            final MdnsResponse copy = new MdnsResponse(existing);
+            responses.add(copy);
+            augmentedToOriginal.put(copy, existing);
         }
         // The response records are structured in a hierarchy, where some records reference
         // others, as follows:
@@ -166,7 +170,7 @@ public class MdnsResponseDecoder {
         // A: host name -> IP address
 
         // Loop 1: find PTR records, which identify distinct service instances.
-        long now = SystemClock.elapsedRealtime();
+        long now = clock.elapsedRealtime();
         for (MdnsRecord record : records) {
             if (record instanceof MdnsPointerRecord) {
                 String[] name = record.getName();
@@ -262,7 +266,11 @@ public class MdnsResponseDecoder {
                         findResponsesWithHostName(responses, inetRecord.getName());
                 for (MdnsResponse response : matchingResponses) {
                     if (assignInetRecord(response, inetRecord)) {
-                        modified.add(response);
+                        final MdnsResponse originalResponse = augmentedToOriginal.get(response);
+                        if (originalResponse == null
+                                || !originalResponse.hasIdenticalRecord(inetRecord)) {
+                            modified.add(response);
+                        }
                     }
                 }
             } else {
@@ -270,12 +278,26 @@ public class MdnsResponseDecoder {
                         findResponseWithHostName(responses, inetRecord.getName());
                 if (response != null) {
                     if (assignInetRecord(response, inetRecord)) {
-                        modified.add(response);
+                        final MdnsResponse originalResponse = augmentedToOriginal.get(response);
+                        if (originalResponse == null
+                                || !originalResponse.hasIdenticalRecord(inetRecord)) {
+                            modified.add(response);
+                        }
                     }
                 }
             }
         }
 
+        // Only responses that have new or modified address records were added to the modified set.
+        // Make sure responses that have lost address records are added to the set too.
+        for (int i = 0; i < augmentedToOriginal.size(); i++) {
+            final MdnsResponse augmented = augmentedToOriginal.keyAt(i);
+            final MdnsResponse original = augmentedToOriginal.valueAt(i);
+            if (augmented.getRecords().size() != original.getRecords().size()) {
+                modified.add(augmented);
+            }
+        }
+
         return Pair.create(modified, responses);
     }
 

tests/unit/java/com/android/server/connectivity/mdns/MdnsDiscoveryManagerTests.java

@@ -293,6 +293,47 @@ public class MdnsDiscoveryManagerTests {
                 .processResponse(response, ifIndex, NETWORK_1);
     }
 
+    @Test
+    public void testUnregisterListenerAfterSocketDestroyed() throws IOException {
+        // Create a ServiceTypeClient for SERVICE_TYPE_1
+        final MdnsSearchOptions network1Options =
+                MdnsSearchOptions.newBuilder().setNetwork(null /* network */).build();
+        final SocketCreationCallback callback = expectSocketCreationCallback(
+                SERVICE_TYPE_1, mockListenerOne, network1Options);
+        runOnHandler(() -> callback.onSocketCreated(null /* network */));
+        verify(mockServiceTypeClientType1NullNetwork).startSendAndReceive(
+                mockListenerOne, network1Options);
+
+        // Receive a response, it should be processed on the client.
+        final MdnsPacket response = createMdnsPacket(SERVICE_TYPE_1);
+        final int ifIndex = 1;
+        runOnHandler(() -> discoveryManager.onResponseReceived(
+                response, ifIndex, null /* network */));
+        verify(mockServiceTypeClientType1NullNetwork).processResponse(
+                response, ifIndex, null /* network */);
+
+        runOnHandler(() -> callback.onAllSocketsDestroyed(null /* network */));
+        verify(mockServiceTypeClientType1NullNetwork).notifySocketDestroyed();
+
+        // Receive a response again, it should not be processed.
+        runOnHandler(() -> discoveryManager.onResponseReceived(
+                response, ifIndex, null /* network */));
+        // Still times(1) as a response was received once previously
+        verify(mockServiceTypeClientType1NullNetwork, times(1))
+                .processResponse(response, ifIndex, null /* network */);
+
+        // Unregister the listener, notifyNetworkUnrequested should be called but other stop methods
+        // won't be call because the service type client was unregistered and destroyed. But those
+        // cleanups were done in notifySocketDestroyed when the socket was destroyed.
+        runOnHandler(() -> discoveryManager.unregisterListener(SERVICE_TYPE_1, mockListenerOne));
+        verify(socketClient).notifyNetworkUnrequested(mockListenerOne);
+        verify(mockServiceTypeClientType1NullNetwork, never()).stopSendAndReceive(any());
+        // The stopDiscovery() is only used by MdnsSocketClient, which doesn't send
+        // onAllSocketsDestroyed(). So the socket clients that send onAllSocketsDestroyed() do not
+        // need to call stopDiscovery().
+        verify(socketClient, never()).stopDiscovery();
+    }
+
     private MdnsPacket createMdnsPacket(String serviceType) {
         final String[] type = TextUtils.split(serviceType, "\\.");
         final ArrayList<String> name = new ArrayList<>(type.length + 1);

tests/unit/java/com/android/server/connectivity/mdns/MdnsResponseDecoderTests.java

@@ -28,6 +28,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 
 import android.net.Network;
@@ -406,6 +407,31 @@ public class MdnsResponseDecoderTests {
                 response.getInet4AddressRecord().getInet4Address());
     }
 
+    @Test
+    public void testDecodeWithIpv4AddressRemove() throws IOException {
+        MdnsResponse response = makeMdnsResponse(0, DATAIN_SERVICE_NAME_1, List.of(
+                new PacketAndRecordClass(DATAIN_PTR_1,
+                        MdnsPointerRecord.class),
+                new PacketAndRecordClass(DATAIN_SERVICE_1,
+                        MdnsServiceRecord.class),
+                new PacketAndRecordClass(DATAIN_IPV4_1,
+                        MdnsInet4AddressRecord.class),
+                new PacketAndRecordClass(DATAIN_IPV4_2,
+                        MdnsInet4AddressRecord.class)));
+        // Now update the response removing the second v4 address
+        final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null);
+
+        final byte[] removedAddrResponse = makeResponsePacket(
+                List.of(DATAIN_PTR_1, DATAIN_SERVICE_1, DATAIN_IPV4_1));
+        final ArraySet<MdnsResponse> changes = decode(
+                decoder, removedAddrResponse, List.of(response));
+
+        assertEquals(1, changes.size());
+        assertEquals(1, changes.valueAt(0).getInet4AddressRecords().size());
+        assertEquals(parseNumericAddress("10.1.2.3"),
+                changes.valueAt(0).getInet4AddressRecords().get(0).getInet4Address());
+    }
+
     @Test
     public void testDecodeWithIpv6AddressChange() throws IOException {
         MdnsResponse response = makeMdnsResponse(0, DATAIN_SERVICE_NAME_1, List.of(
@@ -426,6 +452,29 @@ public class MdnsResponseDecoderTests {
                 response.getInet6AddressRecord().getInet6Address());
     }
 
+    @Test
+    public void testDecodeWithIpv6AddressRemoved() throws IOException {
+        MdnsResponse response = makeMdnsResponse(0, DATAIN_SERVICE_NAME_1, List.of(
+                new PacketAndRecordClass(DATAIN_PTR_1,
+                        MdnsPointerRecord.class),
+                new PacketAndRecordClass(DATAIN_SERVICE_1,
+                        MdnsServiceRecord.class),
+                new PacketAndRecordClass(DATAIN_IPV6_1,
+                        MdnsInet6AddressRecord.class),
+                new PacketAndRecordClass(DATAIN_IPV6_2,
+                        MdnsInet6AddressRecord.class)));
+        // Now update the response adding an address
+        final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null);
+        final byte[] removedAddrResponse = makeResponsePacket(
+                List.of(DATAIN_PTR_1, DATAIN_SERVICE_1, DATAIN_IPV6_1));
+        final ArraySet<MdnsResponse> updatedResponses = decode(
+                decoder, removedAddrResponse, List.of(response));
+        assertEquals(1, updatedResponses.size());
+        assertEquals(1, updatedResponses.valueAt(0).getInet6AddressRecords().size());
+        assertEquals(parseNumericAddress("aabb:ccdd:1122:3344:a0b0:c0d0:1020:3040"),
+                updatedResponses.valueAt(0).getInet6AddressRecords().get(0).getInet6Address());
+    }
+
     @Test
     public void testDecodeWithChangeOnText() throws IOException {
         MdnsResponse response = makeMdnsResponse(0, DATAIN_SERVICE_NAME_1, List.of(
@@ -493,11 +542,12 @@ public class MdnsResponseDecoderTests {
                         new PacketAndRecordClass(DATAIN_IPV4_1, MdnsInet4AddressRecord.class),
                         new PacketAndRecordClass(DATAIN_IPV6_1, MdnsInet6AddressRecord.class),
                         new PacketAndRecordClass(DATAIN_PTR_1, MdnsPointerRecord.class),
-                        new PacketAndRecordClass(DATAIN_SERVICE_2, MdnsServiceRecord.class),
+                        new PacketAndRecordClass(DATAIN_SERVICE_1, MdnsServiceRecord.class),
                         new PacketAndRecordClass(DATAIN_TEXT_1, MdnsTextRecord.class));
         // Create a two identical responses.
-        MdnsResponse response = makeMdnsResponse(0, DATAIN_SERVICE_NAME_1, recordList);
+        MdnsResponse response = makeMdnsResponse(12L /* time */, DATAIN_SERVICE_NAME_1, recordList);
 
+        doReturn(34L).when(mClock).elapsedRealtime();
         final MdnsResponseDecoder decoder = new MdnsResponseDecoder(mClock, null);
         final byte[] identicalResponse = makeResponsePacket(
                 recordList.stream().map(p -> p.packetData).collect(Collectors.toList()));

tests/unit/java/com/android/server/connectivity/mdns/MdnsServiceTypeClientTests.java

@@ -1078,21 +1078,17 @@ public class MdnsServiceTypeClientTests {
                 0 /* flags */,
                 Collections.emptyList() /* questions */,
                 List.of(
-                        // TODO: cacheFlush will cause addresses to be cleared and re-added every
-                        //  time, which is considered a change and triggers extra
-                        //  onServiceChanged callbacks. Sets cacheFlush bit to false until the
-                        //  issue is fixed.
                         new MdnsServiceRecord(serviceName, updatedReceiptTime,
-                                false /* cacheFlush */, TEST_TTL, 0 /* servicePriority */,
+                                true /* cacheFlush */, TEST_TTL, 0 /* servicePriority */,
                                 0 /* serviceWeight */, 1234 /* servicePort */, hostname),
                         new MdnsTextRecord(serviceName, updatedReceiptTime,
-                                false /* cacheFlush */, TEST_TTL,
+                                true /* cacheFlush */, TEST_TTL,
                                 Collections.emptyList() /* entries */),
                         new MdnsInetAddressRecord(hostname, updatedReceiptTime,
-                                false /* cacheFlush */, TEST_TTL,
+                                true /* cacheFlush */, TEST_TTL,
                                 InetAddresses.parseNumericAddress(ipV4Address)),
                         new MdnsInetAddressRecord(hostname, updatedReceiptTime,
-                                false /* cacheFlush */, TEST_TTL,
+                                true /* cacheFlush */, TEST_TTL,
                                 InetAddresses.parseNumericAddress(ipV6Address))),
                 Collections.emptyList() /* authorityRecords */,
                 Collections.emptyList() /* additionalRecords */);