Revert "Revert "Add DhcpLeaseCallbacks""

This reverts commit e8fff42022f8c22e84d51bc093189d469bdd9af1.

Bug: 135411507
Test: atest NetworkStackTests
Change-Id: I0609301d7b37309a35c764a7551b0ca93b3faeee

10 files changed, 380 insertions, 30 deletions

diff --git a/common/networkstackclient/Android.bp b/common/networkstackclient/Android.bp
index dbe8ff0..a34a637 100644
--- a/common/networkstackclient/Android.bp
+++ b/common/networkstackclient/Android.bp
@@ -61,7 +61,9 @@ aidl_interface {
         "src/android/net/ProvisioningConfigurationParcelable.aidl",
         "src/android/net/ScanResultInfoParcelable.aidl",
         "src/android/net/TcpKeepalivePacketDataParcelable.aidl",
+        "src/android/net/dhcp/DhcpLeaseParcelable.aidl",
         "src/android/net/dhcp/DhcpServingParamsParcel.aidl",
+        "src/android/net/dhcp/IDhcpLeaseCallbacks.aidl",
         "src/android/net/dhcp/IDhcpServer.aidl",
         "src/android/net/dhcp/IDhcpServerCallbacks.aidl",
         "src/android/net/ip/IIpClient.aidl",
diff --git a/common/networkstackclient/src/android/net/dhcp/DhcpLeaseParcelable.aidl b/common/networkstackclient/src/android/net/dhcp/DhcpLeaseParcelable.aidl
new file mode 100644
index 0000000..ba3390d
--- /dev/null
+++ b/common/networkstackclient/src/android/net/dhcp/DhcpLeaseParcelable.aidl
@@ -0,0 +1,32 @@
+/**
+ * Copyright (c) 2020, The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing perNmissions and
+ * limitations under the License.
+ */
+
+package android.net.dhcp;
+
+parcelable DhcpLeaseParcelable {
+    // Client ID of the lease; may be null.
+    byte[] clientId;
+    // MAC address provided by the client.
+    byte[] hwAddr;
+    // IPv4 address of the lease, in network byte order.
+    int netAddr;
+    // Prefix length of the lease (0-32)
+    int prefixLength;
+    // Expiration time of the lease, to compare with SystemClock.elapsedRealtime().
+    long expTime;
+    // Hostname provided by the client, if any, or null.
+    String hostname;
+}
\ No newline at end of file
diff --git a/common/networkstackclient/src/android/net/dhcp/IDhcpLeaseCallbacks.aidl b/common/networkstackclient/src/android/net/dhcp/IDhcpLeaseCallbacks.aidl
new file mode 100644
index 0000000..cf2dfa8
--- /dev/null
+++ b/common/networkstackclient/src/android/net/dhcp/IDhcpLeaseCallbacks.aidl
@@ -0,0 +1,30 @@
+/**
+ * Copyright (c) 2020, The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing perNmissions and
+ * limitations under the License.
+ */
+
+package android.net.dhcp;
+
+import android.net.dhcp.DhcpLeaseParcelable;
+
+oneway interface IDhcpLeaseCallbacks {
+    /**
+     * Called when a lease is committed or released on the DHCP server.
+     *
+     * <p>This only reports lease changes after assigning a lease, or after releasing a lease
+     * following a DHCPRELEASE: this callback will not be fired when a lease just expires.
+     * @param newLeases The new list of leases tracked by the server.
+     */
+    void onLeasesChanged(in List<DhcpLeaseParcelable> newLeases);
+}
\ No newline at end of file
diff --git a/common/networkstackclient/src/android/net/dhcp/IDhcpServer.aidl b/common/networkstackclient/src/android/net/dhcp/IDhcpServer.aidl
index 559433b..dd93174 100644
--- a/common/networkstackclient/src/android/net/dhcp/IDhcpServer.aidl
+++ b/common/networkstackclient/src/android/net/dhcp/IDhcpServer.aidl
@@ -18,6 +18,7 @@ package android.net.dhcp;
 
 import android.net.INetworkStackStatusCallback;
 import android.net.dhcp.DhcpServingParamsParcel;
+import android.net.dhcp.IDhcpLeaseCallbacks;
 
 /** @hide */
 oneway interface IDhcpServer {
@@ -26,7 +27,11 @@ oneway interface IDhcpServer {
     const int STATUS_INVALID_ARGUMENT = 2;
     const int STATUS_UNKNOWN_ERROR = 3;
 
-    void start(in INetworkStackStatusCallback cb);
-    void updateParams(in DhcpServingParamsParcel params, in INetworkStackStatusCallback cb);
-    void stop(in INetworkStackStatusCallback cb);
+    void start(in INetworkStackStatusCallback cb) = 0;
+    void startWithCallbacks(in INetworkStackStatusCallback statusCb,
+        in IDhcpLeaseCallbacks leaseCb) = 3;
+    void updateParams(in DhcpServingParamsParcel params, in INetworkStackStatusCallback cb) = 1;
+    void stop(in INetworkStackStatusCallback cb) = 2;
+
+    // Next available ID: 4
 }
diff --git a/src/android/net/dhcp/DhcpLease.java b/src/android/net/dhcp/DhcpLease.java
index 37d9cc0..3226f28 100644
--- a/src/android/net/dhcp/DhcpLease.java
+++ b/src/android/net/dhcp/DhcpLease.java
@@ -16,6 +16,8 @@
 
 package android.net.dhcp;
 
+import static android.net.shared.Inet4AddressUtils.inet4AddressToIntHTH;
+
 import android.net.MacAddress;
 import android.os.SystemClock;
 import android.text.TextUtils;
@@ -43,6 +45,7 @@ public class DhcpLease {
     private final MacAddress mHwAddr;
     @NonNull
     private final Inet4Address mNetAddr;
+    private final int mPrefixLength;
     /**
      * Expiration time for the lease, to compare with {@link SystemClock#elapsedRealtime()}.
      */
@@ -51,10 +54,12 @@ public class DhcpLease {
     private final String mHostname;
 
     public DhcpLease(@Nullable byte[] clientId, @NonNull MacAddress hwAddr,
-            @NonNull Inet4Address netAddr, long expTime, @Nullable String hostname) {
+            @NonNull Inet4Address netAddr, int prefixLength, long expTime,
+            @Nullable String hostname) {
         mClientId = (clientId == null ? null : Arrays.copyOf(clientId, clientId.length));
         mHwAddr = hwAddr;
         mNetAddr = netAddr;
+        mPrefixLength = prefixLength;
         mExpTime = expTime;
         mHostname = hostname;
     }
@@ -87,6 +92,10 @@ public class DhcpLease {
         return mNetAddr;
     }
 
+    public int getPrefixLength() {
+        return mPrefixLength;
+    }
+
     public long getExpTime() {
         return mExpTime;
     }
@@ -99,7 +108,8 @@ public class DhcpLease {
      * @return A {@link DhcpLease} with expiration time set to max(expTime, currentExpTime)
      */
     public DhcpLease renewedLease(long expTime, @Nullable String hostname) {
-        return new DhcpLease(mClientId, mHwAddr, mNetAddr, Math.max(expTime, mExpTime),
+        return new DhcpLease(mClientId, mHwAddr, mNetAddr, mPrefixLength,
+                Math.max(expTime, mExpTime),
                 (hostname == null ? mHostname : hostname));
     }
 
@@ -125,13 +135,14 @@ public class DhcpLease {
         return Arrays.equals(mClientId, other.mClientId)
                 && mHwAddr.equals(other.mHwAddr)
                 && mNetAddr.equals(other.mNetAddr)
+                && mPrefixLength == other.mPrefixLength
                 && mExpTime == other.mExpTime
                 && TextUtils.equals(mHostname, other.mHostname);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(mClientId, mHwAddr, mNetAddr, mHostname, mExpTime);
+        return Objects.hash(mClientId, mHwAddr, mNetAddr, mPrefixLength, mHostname, mExpTime);
     }
 
     static String clientIdToString(byte[] bytes) {
@@ -147,8 +158,24 @@ public class DhcpLease {
 
     @Override
     public String toString() {
-        return String.format("clientId: %s, hwAddr: %s, netAddr: %s, expTime: %d, hostname: %s",
+        return String.format("clientId: %s, hwAddr: %s, netAddr: %s/%d, expTime: %d,"
+                        + "hostname: %s",
                 clientIdToString(mClientId), mHwAddr.toString(), inet4AddrToString(mNetAddr),
-                mExpTime, mHostname);
+                mPrefixLength, mExpTime, mHostname);
+    }
+
+    /**
+     * Create a {@link DhcpLeaseParcelable} containing the information held in this lease.
+     */
+    public DhcpLeaseParcelable toParcelable() {
+        final DhcpLeaseParcelable p = new DhcpLeaseParcelable();
+        p.clientId = mClientId == null ? null : Arrays.copyOf(mClientId, mClientId.length);
+        p.hwAddr = mHwAddr.toByteArray();
+        p.netAddr = inet4AddressToIntHTH(mNetAddr);
+        p.prefixLength = mPrefixLength;
+        p.expTime = mExpTime;
+        p.hostname = mHostname;
+
+        return p;
     }
 }
diff --git a/src/android/net/dhcp/DhcpLeaseRepository.java b/src/android/net/dhcp/DhcpLeaseRepository.java
index 4e74dc8..1dc2f7f 100644
--- a/src/android/net/dhcp/DhcpLeaseRepository.java
+++ b/src/android/net/dhcp/DhcpLeaseRepository.java
@@ -31,6 +31,8 @@ import android.net.IpPrefix;
 import android.net.MacAddress;
 import android.net.dhcp.DhcpServer.Clock;
 import android.net.util.SharedLog;
+import android.os.RemoteCallbackList;
+import android.os.RemoteException;
 import android.util.ArrayMap;
 
 import androidx.annotation.NonNull;
@@ -45,6 +47,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Function;
 
@@ -73,6 +76,7 @@ class DhcpLeaseRepository {
     @NonNull
     private Set<Inet4Address> mReservedAddrs;
     private int mSubnetAddr;
+    private int mPrefixLength;
     private int mSubnetMask;
     private int mNumAddresses;
     private long mLeaseTimeMs;
@@ -84,6 +88,9 @@ class DhcpLeaseRepository {
      */
     private long mNextExpirationCheck = EXPIRATION_NEVER;
 
+    @NonNull
+    private RemoteCallbackList<IDhcpLeaseCallbacks> mLeaseCallbacks = new RemoteCallbackList<>();
+
     static class DhcpLeaseException extends Exception {
         DhcpLeaseException(String message) {
             super(message);
@@ -131,27 +138,34 @@ class DhcpLeaseRepository {
             long leaseTimeMs) {
         mPrefix = prefix;
         mReservedAddrs = Collections.unmodifiableSet(new HashSet<>(reservedAddrs));
-        mSubnetMask = prefixLengthToV4NetmaskIntHTH(prefix.getPrefixLength());
+        mPrefixLength = prefix.getPrefixLength();
+        mSubnetMask = prefixLengthToV4NetmaskIntHTH(mPrefixLength);
         mSubnetAddr = inet4AddressToIntHTH((Inet4Address) prefix.getAddress()) & mSubnetMask;
         mNumAddresses = 1 << (IPV4_ADDR_BITS - prefix.getPrefixLength());
         mLeaseTimeMs = leaseTimeMs;
 
-        cleanMap(mCommittedLeases);
         cleanMap(mDeclinedAddrs);
+        if (cleanMap(mCommittedLeases)) {
+            notifyLeasesChanged();
+        }
     }
 
     /**
      * From a map keyed by {@link Inet4Address}, remove entries where the key is invalid (as
      * specified by {@link #isValidAddress(Inet4Address)}), or is a reserved address.
+     * @return true iff at least one entry was removed.
      */
-    private <T> void cleanMap(Map<Inet4Address, T> map) {
+    private <T> boolean cleanMap(Map<Inet4Address, T> map) {
         final Iterator<Entry<Inet4Address, T>> it = map.entrySet().iterator();
+        boolean removed = false;
         while (it.hasNext()) {
             final Inet4Address addr = it.next().getKey();
             if (!isValidAddress(addr) || mReservedAddrs.contains(addr)) {
                 it.remove();
+                removed = true;
             }
         }
+        return removed;
     }
 
     /**
@@ -181,7 +195,7 @@ class DhcpLeaseRepository {
             mLog.log("Offering extended lease " + newLease);
             // Do not update lease time in the map: the offer is not committed yet.
         } else if (reqAddr != null && isValidAddress(reqAddr) && isAvailable(reqAddr)) {
-            newLease = new DhcpLease(clientId, hwAddr, reqAddr, expTime, hostname);
+            newLease = new DhcpLease(clientId, hwAddr, reqAddr, mPrefixLength, expTime, hostname);
             mLog.log("Offering requested lease " + newLease);
         } else {
             newLease = makeNewOffer(clientId, hwAddr, expTime, hostname);
@@ -267,7 +281,8 @@ class DhcpLeaseRepository {
         if (assignedLease != null) {
             if (sidSet && reqAddr != null) {
                 // Client in SELECTING state; remove any current lease before creating a new one.
-                mCommittedLeases.remove(assignedLease.getNetAddr());
+                // Do not notify of change as it will be done when the new lease is committed.
+                removeLease(assignedLease.getNetAddr(), false /* notifyChange */);
             } else if (!assignedLease.getNetAddr().equals(leaseAddr)) {
                 // reqAddr null (RENEWING/REBINDING): client renewing its own lease for clientAddr.
                 // reqAddr set with sid not set (INIT-REBOOT): client verifying configuration.
@@ -314,7 +329,7 @@ class DhcpLeaseRepository {
         final DhcpLease lease;
         if (currentLease == null) {
             if (isValidAddress(addr) && !mReservedAddrs.contains(addr)) {
-                lease = new DhcpLease(clientId, hwAddr, addr, expTime, hostname);
+                lease = new DhcpLease(clientId, hwAddr, addr, mPrefixLength, expTime, hostname);
             } else {
                 throw new InvalidAddressException("Lease not found and address unavailable");
             }
@@ -328,6 +343,13 @@ class DhcpLeaseRepository {
     private void commitLease(@NonNull DhcpLease lease) {
         mCommittedLeases.put(lease.getNetAddr(), lease);
         maybeUpdateEarliestExpiration(lease.getExpTime());
+        notifyLeasesChanged();
+    }
+
+    private void removeLease(@NonNull Inet4Address address, boolean notifyChange) {
+        // Earliest expiration remains <= the first expiry time on remove, so no need to update it.
+        mCommittedLeases.remove(address);
+        if (notifyChange) notifyLeasesChanged();
     }
 
     /**
@@ -343,8 +365,8 @@ class DhcpLeaseRepository {
             return false;
         }
         if (currentLease.matchesClient(clientId, hwAddr)) {
-            mCommittedLeases.remove(addr);
             mLog.log("Released lease " + currentLease);
+            removeLease(addr, true /* notifyChange */);
             return true;
         }
         mLog.w(String.format("Not releasing lease %s: does not match client (cid %s, hwAddr %s)",
@@ -352,6 +374,24 @@ class DhcpLeaseRepository {
         return false;
     }
 
+    private void notifyLeasesChanged() {
+        final List<DhcpLeaseParcelable> leaseParcelables =
+                new ArrayList<>(mCommittedLeases.size());
+        for (DhcpLease committedLease : mCommittedLeases.values()) {
+            leaseParcelables.add(committedLease.toParcelable());
+        }
+
+        final int cbCount = mLeaseCallbacks.beginBroadcast();
+        for (int i = 0; i < cbCount; i++) {
+            try {
+                mLeaseCallbacks.getBroadcastItem(i).onLeasesChanged(leaseParcelables);
+            } catch (RemoteException e) {
+                mLog.e("Could not send lease callback", e);
+            }
+        }
+        mLeaseCallbacks.finishBroadcast();
+    }
+
     public void markLeaseDeclined(@NonNull Inet4Address addr) {
         if (mDeclinedAddrs.containsKey(addr) || !isValidAddress(addr)) {
             mLog.logf("Not marking %s as declined: already declined or not assignable",
@@ -383,6 +423,14 @@ class DhcpLeaseRepository {
     }
 
     /**
+     * Add callbacks that will be called on leases update.
+     */
+    public void addLeaseCallbacks(@NonNull IDhcpLeaseCallbacks cb) {
+        Objects.requireNonNull(cb, "Callbacks must be non-null");
+        mLeaseCallbacks.register(cb);
+    }
+
+    /**
      * Given the expiration time of a new committed lease or declined address, update
      * {@link #mNextExpirationCheck} so it stays lower than or equal to the time for the first lease
      * to expire.
@@ -541,7 +589,7 @@ class DhcpLeaseRepository {
         for (int i = 0; i < mNumAddresses; i++) {
             final Inet4Address addr = intToInet4AddressHTH(intAddr);
             if (isAvailable(addr) && !mDeclinedAddrs.containsKey(addr)) {
-                return new DhcpLease(clientId, hwAddr, addr, expTime, hostname);
+                return new DhcpLease(clientId, hwAddr, addr, mPrefixLength, expTime, hostname);
             }
             intAddr = getNextAddress(intAddr);
         }
@@ -557,7 +605,7 @@ class DhcpLeaseRepository {
             // However declined addresses may have been requested (typically by the machine that was
             // already using the address) after being declined.
             if (isAvailable(addr)) {
-                return new DhcpLease(clientId, hwAddr, addr, expTime, hostname);
+                return new DhcpLease(clientId, hwAddr, addr, mPrefixLength, expTime, hostname);
             }
         }
 
diff --git a/src/android/net/dhcp/DhcpServer.java b/src/android/net/dhcp/DhcpServer.java
index 6aadc04..bcca47a 100644
--- a/src/android/net/dhcp/DhcpServer.java
+++ b/src/android/net/dhcp/DhcpServer.java
@@ -274,10 +274,22 @@ public class DhcpServer extends IDhcpServer.Stub {
      */
     @Override
     public void start(@Nullable INetworkStackStatusCallback cb) {
+        startWithCallbacks(cb, null);
+    }
+
+    /**
+     * Start listening for and responding to packets, with optional callbacks for lease events.
+     *
+     * <p>It is not legal to call this method more than once; in particular the server cannot be
+     * restarted after being stopped.
+     */
+    @Override
+    public void startWithCallbacks(@Nullable INetworkStackStatusCallback statusCb,
+            @Nullable IDhcpLeaseCallbacks leaseCb) {
         mDeps.checkCaller();
         mHandlerThread.start();
         mHandler = new ServerHandler(mHandlerThread.getLooper());
-        sendMessage(CMD_START_DHCP_SERVER, cb);
+        sendMessage(CMD_START_DHCP_SERVER, new Pair<>(statusCb, leaseCb));
     }
 
     /**
@@ -344,9 +356,12 @@ public class DhcpServer extends IDhcpServer.Stub {
                     cb = pair.second;
                     break;
                 case CMD_START_DHCP_SERVER:
+                    final Pair<INetworkStackStatusCallback, IDhcpLeaseCallbacks> obj =
+                            (Pair<INetworkStackStatusCallback, IDhcpLeaseCallbacks>) msg.obj;
+                    cb = obj.first;
+                    mLeaseRepo.addLeaseCallbacks(obj.second);
                     mPacketListener = mDeps.makePacketListener();
                     mPacketListener.start();
-                    cb = (INetworkStackStatusCallback) msg.obj;
                     break;
                 case CMD_STOP_DHCP_SERVER:
                     if (mPacketListener != null) {
diff --git a/tests/unit/src/android/net/dhcp/DhcpLeaseRepositoryTest.java b/tests/unit/src/android/net/dhcp/DhcpLeaseRepositoryTest.java
index 27d7255..82f9b50 100644
--- a/tests/unit/src/android/net/dhcp/DhcpLeaseRepositoryTest.java
+++ b/tests/unit/src/android/net/dhcp/DhcpLeaseRepositoryTest.java
@@ -20,6 +20,7 @@ import static android.net.InetAddresses.parseNumericAddress;
 import static android.net.dhcp.DhcpLease.HOSTNAME_NONE;
 import static android.net.dhcp.DhcpLeaseRepository.CLIENTID_UNSPEC;
 import static android.net.dhcp.DhcpLeaseRepository.INETADDR_UNSPEC;
+import static android.net.shared.Inet4AddressUtils.intToInet4AddressHTH;
 
 import static com.android.server.util.NetworkStackConstants.IPV4_ADDR_ANY;
 
@@ -29,14 +30,26 @@ import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.Mockito.atLeastOnce;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
+import static java.lang.String.format;
+import static java.util.stream.Collectors.toSet;
+
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.net.IpPrefix;
 import android.net.MacAddress;
 import android.net.dhcp.DhcpServer.Clock;
 import android.net.util.SharedLog;
+import android.os.Binder;
+import android.os.RemoteException;
 
 import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
@@ -47,8 +60,6 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
-import static java.lang.String.format;
-
 import java.net.Inet4Address;
 import java.util.Arrays;
 import java.util.Collections;
@@ -80,10 +91,15 @@ public class DhcpLeaseRepositoryTest {
 
     @NonNull
     private SharedLog mLog;
-    @NonNull @Mock
+    @NonNull
+    @Mock
     private Clock mClock;
     @NonNull
     private DhcpLeaseRepository mRepo;
+    @NonNull
+    @Mock
+    private IDhcpLeaseCallbacks mCallbacks;
+    private final Binder mCallbacksBinder = new Binder();
 
     private static Inet4Address parseAddr4(String inet4Addr) {
         return (Inet4Address) parseNumericAddress(inet4Addr);
@@ -94,8 +110,12 @@ public class DhcpLeaseRepositoryTest {
         MockitoAnnotations.initMocks(this);
         mLog = new SharedLog("DhcpLeaseRepositoryTest");
         when(mClock.elapsedRealtime()).thenReturn(TEST_TIME);
+        // Use a non-null Binder for linkToDeath
+        when(mCallbacks.asBinder()).thenReturn(mCallbacksBinder);
         mRepo = new DhcpLeaseRepository(
                 TEST_IP_PREFIX, TEST_EXCL_SET, TEST_LEASE_TIME_MS, mLog, mClock);
+        mRepo.addLeaseCallbacks(mCallbacks);
+        verify(mCallbacks, atLeastOnce()).asBinder();
     }
 
     /**
@@ -129,6 +149,7 @@ public class DhcpLeaseRepositoryTest {
 
         // /28 should have 16 addresses, 14 w/o the first/last, 11 w/o excluded addresses
         requestAddresses((byte) 11);
+        verify(mCallbacks, times(11)).onLeasesChanged(any());
 
         try {
             mRepo.getOffer(null, TEST_MAC_2,
@@ -137,6 +158,7 @@ public class DhcpLeaseRepositoryTest {
         } catch (DhcpLeaseRepository.OutOfAddressesException e) {
             // Expected
         }
+        verifyNoMoreInteractions(mCallbacks);
     }
 
     @Test
@@ -149,6 +171,7 @@ public class DhcpLeaseRepositoryTest {
         final Inet4Address declinedFirstAddrIn28 = parseAddr4("192.168.42.240");
 
         final DhcpLease reqAddrIn28Lease = requestLeaseSelecting(TEST_MAC_1, reqAddrIn28);
+        verifyLeasesChangedCallback(reqAddrIn28Lease);
         mRepo.markLeaseDeclined(declinedAddrIn28);
         mRepo.markLeaseDeclined(declinedFirstAddrIn28);
 
@@ -157,16 +180,21 @@ public class DhcpLeaseRepositoryTest {
         final Inet4Address declinedAddrIn22 = parseAddr4("192.168.42.4");
 
         final DhcpLease reqAddrIn22Lease = requestLeaseSelecting(TEST_MAC_3, reqAddrIn22);
+        verifyLeasesChangedCallback(reqAddrIn28Lease, reqAddrIn22Lease);
         mRepo.markLeaseDeclined(declinedAddrIn22);
 
         // Address that will be reserved in the updateParams call below
         final Inet4Address reservedAddr = parseAddr4("192.168.42.244");
         final DhcpLease reservedAddrLease = requestLeaseSelecting(TEST_MAC_2, reservedAddr);
+        verifyLeasesChangedCallback(reqAddrIn28Lease, reqAddrIn22Lease, reservedAddrLease);
 
         // Update from /22 to /28 and add another reserved address
         Set<Inet4Address> newReserved = new HashSet<>(TEST_EXCL_SET);
         newReserved.add(reservedAddr);
         mRepo.updateParams(new IpPrefix(TEST_SERVER_ADDR, 28), newReserved, TEST_LEASE_TIME_MS);
+        // Callback is called for the second time with just this lease
+        verifyLeasesChangedCallback(2 /* times */, reqAddrIn28Lease);
+        verifyNoMoreInteractions(mCallbacks);
 
         assertHasLease(reqAddrIn28Lease);
         assertDeclined(declinedAddrIn28);
@@ -249,6 +277,9 @@ public class DhcpLeaseRepositoryTest {
                 TEST_INETADDR_1 /* reqAddr */, TEST_HOSTNAME_1);
         assertEquals(TEST_INETADDR_1, offer.getNetAddr());
         assertEquals(TEST_HOSTNAME_1, offer.getHostname());
+
+        // Leases are not committed on offer
+        verify(mCallbacks, never()).onLeasesChanged(any());
     }
 
     @Test
@@ -293,10 +324,14 @@ public class DhcpLeaseRepositoryTest {
         final DhcpLease lease1 = requestLeaseSelecting(TEST_MAC_1, TEST_INETADDR_1,
                 TEST_HOSTNAME_1);
 
+        verifyLeasesChangedCallback(lease1);
+
         // Second request from same client for a different address
         final DhcpLease lease2 = requestLeaseSelecting(TEST_MAC_1, TEST_INETADDR_2,
                 TEST_HOSTNAME_2);
 
+        verifyLeasesChangedCallback(lease2);
+
         assertEquals(TEST_INETADDR_1, lease1.getNetAddr());
         assertEquals(TEST_HOSTNAME_1, lease1.getHostname());
 
@@ -306,6 +341,9 @@ public class DhcpLeaseRepositoryTest {
         // First address freed when client requested a different one: another client can request it
         final DhcpLease lease3 = requestLeaseSelecting(TEST_MAC_2, TEST_INETADDR_1, HOSTNAME_NONE);
         assertEquals(TEST_INETADDR_1, lease3.getNetAddr());
+
+        verifyLeasesChangedCallback(lease2, lease3);
+        verifyNoMoreInteractions(mCallbacks);
     }
 
     @Test(expected = DhcpLeaseRepository.InvalidAddressException.class)
@@ -334,7 +372,8 @@ public class DhcpLeaseRepositoryTest {
     @Test
     public void testRequestLease_InitReboot() throws Exception {
         // Request address once
-        requestLeaseSelecting(TEST_MAC_1, TEST_INETADDR_1);
+        final DhcpLease oldLease = requestLeaseSelecting(TEST_MAC_1, TEST_INETADDR_1);
+        verifyLeasesChangedCallback(oldLease);
 
         final long newTime = TEST_TIME + 100;
         when(mClock.elapsedRealtime()).thenReturn(newTime);
@@ -343,6 +382,9 @@ public class DhcpLeaseRepositoryTest {
         final DhcpLease lease = requestLeaseInitReboot(TEST_MAC_1, TEST_INETADDR_1);
         assertEquals(TEST_INETADDR_1, lease.getNetAddr());
         assertEquals(newTime + TEST_LEASE_TIME_MS, lease.getExpTime());
+
+        verifyLeasesChangedCallback(lease);
+        verifyNoMoreInteractions(mCallbacks);
     }
 
     @Test(expected = DhcpLeaseRepository.InvalidAddressException.class)
@@ -370,7 +412,9 @@ public class DhcpLeaseRepositoryTest {
 
     @Test
     public void testRequestLease_Renewing() throws Exception {
-        requestLeaseSelecting(TEST_MAC_1, TEST_INETADDR_1);
+        final DhcpLease oldLease = requestLeaseSelecting(TEST_MAC_1, TEST_INETADDR_1);
+
+        verifyLeasesChangedCallback(oldLease);
 
         final long newTime = TEST_TIME + 100;
         when(mClock.elapsedRealtime()).thenReturn(newTime);
@@ -379,6 +423,9 @@ public class DhcpLeaseRepositoryTest {
 
         assertEquals(TEST_INETADDR_1, lease.getNetAddr());
         assertEquals(newTime + TEST_LEASE_TIME_MS, lease.getExpTime());
+
+        verifyLeasesChangedCallback(lease);
+        verifyNoMoreInteractions(mCallbacks);
     }
 
     @Test
@@ -391,10 +438,19 @@ public class DhcpLeaseRepositoryTest {
         assertEquals(newTime + TEST_LEASE_TIME_MS, lease.getExpTime());
     }
 
-    @Test(expected = DhcpLeaseRepository.InvalidAddressException.class)
+    @Test
     public void testRequestLease_RenewingAddrInUse() throws Exception {
-        requestLeaseSelecting(TEST_MAC_2, TEST_INETADDR_1);
-        requestLeaseRenewing(TEST_MAC_1, TEST_INETADDR_1);
+        final DhcpLease originalLease = requestLeaseSelecting(TEST_MAC_2, TEST_INETADDR_1);
+        verifyLeasesChangedCallback(originalLease);
+
+        try {
+            requestLeaseRenewing(TEST_MAC_1, TEST_INETADDR_1);
+            fail("Renewing with a different address should fail");
+        } catch (DhcpLeaseRepository.InvalidAddressException e) {
+            // fall through
+        }
+
+        verifyNoMoreInteractions(mCallbacks);
     }
 
     @Test(expected = DhcpLeaseRepository.InvalidAddressException.class)
@@ -541,4 +597,29 @@ public class DhcpLeaseRepositoryTest {
     private void assertDeclined(Inet4Address addr) {
         assertTrue("Address is not declined: " + addr, mRepo.getDeclinedAddresses().contains(addr));
     }
+
+    private void verifyLeasesChangedCallback(int times, DhcpLease... leases) {
+        final Set<DhcpLease> expected = new HashSet<>(Arrays.asList(leases));
+        try {
+            verify(mCallbacks, times(times)).onLeasesChanged(argThat(l ->
+                    l.stream().map(DhcpLeaseRepositoryTest::fromParcelable).collect(toSet())
+                            .equals(expected)));
+        } catch (RemoteException e) {
+            fail("Can't happen: " + e);
+        }
+    }
+
+    private void verifyLeasesChangedCallback(DhcpLease... leases) {
+        verifyLeasesChangedCallback(1 /* times */, leases);
+    }
+
+    private static DhcpLease fromParcelable(DhcpLeaseParcelable p) {
+        return new DhcpLease(
+                p.clientId,
+                p.hwAddr == null ? null : MacAddress.fromBytes(p.hwAddr),
+                intToInet4AddressHTH(p.netAddr),
+                p.prefixLength,
+                p.expTime,
+                p.hostname);
+    }
 }
diff --git a/tests/unit/src/android/net/dhcp/DhcpLeaseTest.kt b/tests/unit/src/android/net/dhcp/DhcpLeaseTest.kt
new file mode 100644
index 0000000..2971f65
--- /dev/null
+++ b/tests/unit/src/android/net/dhcp/DhcpLeaseTest.kt
@@ -0,0 +1,107 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.net.dhcp
+
+import android.net.InetAddresses.parseNumericAddress
+import android.net.MacAddress
+import android.net.shared.Inet4AddressUtils.intToInet4AddressHTH
+import androidx.test.filters.SmallTest
+import androidx.test.runner.AndroidJUnit4
+import com.android.testutils.assertFieldCountEquals
+import org.junit.Assert.assertArrayEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+import java.net.Inet4Address
+import kotlin.test.assertEquals
+import kotlin.test.assertNotEquals
+
+@RunWith(AndroidJUnit4::class)
+@SmallTest
+class DhcpLeaseTest {
+    companion object {
+        private val TEST_CLIENT_ID = byteArrayOf(0, 1, 2, 127)
+        private val TEST_HWADDR = MacAddress.fromString("01:23:45:67:8F:9A")
+        private val TEST_INETADDR = parseNumericAddress("192.168.42.123") as Inet4Address
+        private val TEST_PREFIXLEN = 23
+        private val TEST_EXPTIME = 1234L
+        private val TEST_HOSTNAME = "test_hostname"
+    }
+
+    @Test
+    fun testToParcelable() {
+        val lease = DhcpLease(TEST_CLIENT_ID, TEST_HWADDR, TEST_INETADDR, TEST_PREFIXLEN,
+                TEST_EXPTIME, TEST_HOSTNAME)
+
+        assertParcelEquals(lease, lease.toParcelable())
+    }
+
+    @Test
+    fun testToParcelable_NullFields() {
+        val lease = DhcpLease(null, TEST_HWADDR, TEST_INETADDR, TEST_PREFIXLEN, TEST_EXPTIME, null)
+        assertParcelEquals(lease, lease.toParcelable())
+    }
+
+    @Test
+    fun testEquals() {
+        val lease = DhcpLease(TEST_CLIENT_ID, TEST_HWADDR, TEST_INETADDR, TEST_PREFIXLEN,
+                TEST_EXPTIME, TEST_HOSTNAME)
+        assertEquals(lease, DhcpLease(TEST_CLIENT_ID, TEST_HWADDR, TEST_INETADDR, TEST_PREFIXLEN,
+                TEST_EXPTIME, TEST_HOSTNAME))
+
+        // Change client ID
+        assertNotEquals(lease, DhcpLease(null, TEST_HWADDR, TEST_INETADDR, TEST_PREFIXLEN,
+                TEST_EXPTIME, TEST_HOSTNAME))
+        assertNotEquals(lease, DhcpLease(byteArrayOf(42), TEST_HWADDR, TEST_INETADDR,
+                TEST_PREFIXLEN, TEST_EXPTIME, TEST_HOSTNAME))
+
+        // Change mac address
+        assertNotEquals(lease, DhcpLease(TEST_CLIENT_ID, MacAddress.fromString("12:34:56:78:9A:0B"),
+                TEST_INETADDR, TEST_PREFIXLEN, TEST_EXPTIME, TEST_HOSTNAME))
+
+        // Change address
+        assertNotEquals(lease, DhcpLease(TEST_CLIENT_ID, TEST_HWADDR,
+                parseNumericAddress("192.168.43.43") as Inet4Address, TEST_PREFIXLEN, TEST_EXPTIME,
+                TEST_HOSTNAME))
+
+        // Change prefix length
+        assertNotEquals(lease, DhcpLease(TEST_CLIENT_ID, TEST_HWADDR, TEST_INETADDR, 24,
+                TEST_EXPTIME, TEST_HOSTNAME))
+
+        // Change expiry time
+        assertNotEquals(lease, DhcpLease(TEST_CLIENT_ID, TEST_HWADDR, TEST_INETADDR, TEST_PREFIXLEN,
+                4567L, TEST_HOSTNAME))
+
+        // Change hostname
+        assertNotEquals(lease, DhcpLease(TEST_CLIENT_ID, TEST_HWADDR, TEST_INETADDR, TEST_PREFIXLEN,
+                TEST_EXPTIME, null))
+        assertNotEquals(lease, DhcpLease(TEST_CLIENT_ID, TEST_HWADDR, TEST_INETADDR, TEST_PREFIXLEN,
+                TEST_EXPTIME, "other_hostname"))
+
+        assertFieldCountEquals(6, DhcpLease::class.java)
+    }
+
+    private fun assertParcelEquals(expected: DhcpLease, p: DhcpLeaseParcelable) {
+        assertArrayEquals(expected.clientId, p.clientId)
+        assertEquals(expected.hwAddr, MacAddress.fromBytes(p.hwAddr))
+        assertEquals(expected.netAddr, intToInet4AddressHTH(p.netAddr))
+        assertEquals(expected.prefixLength, p.prefixLength)
+        assertEquals(expected.expTime, p.expTime)
+        assertEquals(expected.hostname, p.hostname)
+
+        assertFieldCountEquals(6, DhcpLease::class.java)
+    }
+}
\ No newline at end of file
diff --git a/tests/unit/src/android/net/dhcp/DhcpServerTest.java b/tests/unit/src/android/net/dhcp/DhcpServerTest.java
index aae9bc0..37ca833 100644
--- a/tests/unit/src/android/net/dhcp/DhcpServerTest.java
+++ b/tests/unit/src/android/net/dhcp/DhcpServerTest.java
@@ -81,7 +81,9 @@ public class DhcpServerTest {
     private static final String TEST_IFACE = "testiface";
 
     private static final Inet4Address TEST_SERVER_ADDR = parseAddr("192.168.0.2");
-    private static final LinkAddress TEST_SERVER_LINKADDR = new LinkAddress(TEST_SERVER_ADDR, 20);
+    private static final int TEST_PREFIX_LENGTH = 20;
+    private static final LinkAddress TEST_SERVER_LINKADDR = new LinkAddress(
+            TEST_SERVER_ADDR, TEST_PREFIX_LENGTH);
     private static final Set<Inet4Address> TEST_DEFAULT_ROUTERS = new HashSet<>(
             Arrays.asList(parseAddr("192.168.0.123"), parseAddr("192.168.0.124")));
     private static final Set<Inet4Address> TEST_DNS_SERVERS = new HashSet<>(
@@ -100,10 +102,11 @@ public class DhcpServerTest {
     private static final long TEST_CLOCK_TIME = 1234L;
     private static final int TEST_LEASE_EXPTIME_SECS = 3600;
     private static final DhcpLease TEST_LEASE = new DhcpLease(null, TEST_CLIENT_MAC,
-            TEST_CLIENT_ADDR, TEST_LEASE_EXPTIME_SECS * 1000L + TEST_CLOCK_TIME,
+            TEST_CLIENT_ADDR, TEST_PREFIX_LENGTH, TEST_LEASE_EXPTIME_SECS * 1000L + TEST_CLOCK_TIME,
             null /* hostname */);
     private static final DhcpLease TEST_LEASE_WITH_HOSTNAME = new DhcpLease(null, TEST_CLIENT_MAC,
-            TEST_CLIENT_ADDR, TEST_LEASE_EXPTIME_SECS * 1000L + TEST_CLOCK_TIME, TEST_HOSTNAME);
+            TEST_CLIENT_ADDR, TEST_PREFIX_LENGTH, TEST_LEASE_EXPTIME_SECS * 1000L + TEST_CLOCK_TIME,
+            TEST_HOSTNAME);
 
     @NonNull @Mock
     private Context mContext;