Merge "Remove IpClient usage of NetworkManagementService"

packages/NetworkStack/src/android/net/ip/IpClient.java

@@ -40,15 +40,12 @@ import android.net.ip.IIpClientCallbacks;
 import android.net.metrics.IpConnectivityLog;
 import android.net.metrics.IpManagerEvent;
 import android.net.shared.InitialConfiguration;
-import android.net.shared.NetdService;
 import android.net.shared.ProvisioningConfiguration;
 import android.net.util.InterfaceParams;
 import android.net.util.SharedLog;
 import android.os.ConditionVariable;
-import android.os.INetworkManagementService;
 import android.os.Message;
 import android.os.RemoteException;
-import android.os.ServiceManager;
 import android.os.SystemClock;
 import android.text.TextUtils;
 import android.util.LocalLog;
@@ -64,7 +61,7 @@ import com.android.internal.util.Preconditions;
 import com.android.internal.util.State;
 import com.android.internal.util.StateMachine;
 import com.android.internal.util.WakeupMessage;
-import com.android.server.net.NetlinkTracker;
+import com.android.server.NetworkObserverRegistry;
 
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
@@ -338,8 +335,9 @@ public class IpClient extends StateMachine {
     private final Dependencies mDependencies;
     private final CountDownLatch mShutdownLatch;
     private final ConnectivityManager mCm;
-    private final INetworkManagementService mNwService;
+    private final INetd mNetd;
-    private final NetlinkTracker mNetlinkTracker;
+    private final NetworkObserverRegistry mObserverRegistry;
+    private final IpClientLinkObserver mLinkObserver;
     private final WakeupMessage mProvisioningTimeoutAlarm;
     private final WakeupMessage mDhcpActionTimeoutAlarm;
     private final SharedLog mLog;
@@ -373,15 +371,6 @@ public class IpClient extends StateMachine {
     private final ConditionVariable mApfDataSnapshotComplete = new ConditionVariable();
 
     public static class Dependencies {
-        public INetworkManagementService getNMS() {
-            return INetworkManagementService.Stub.asInterface(
-                    ServiceManager.getService(Context.NETWORKMANAGEMENT_SERVICE));
-        }
-
-        public INetd getNetd() {
-            return NetdService.getInstance();
-        }
-
         /**
          * Get interface parameters for the specified interface.
          */
@@ -390,26 +379,14 @@ public class IpClient extends StateMachine {
         }
     }
 
-    public IpClient(Context context, String ifName, IIpClientCallbacks callback) {
-        this(context, ifName, callback, new Dependencies());
-    }
-
-    /**
-     * An expanded constructor, useful for dependency injection.
-     * TODO: migrate all test users to mock IpClient directly and remove this ctor.
-     */
     public IpClient(Context context, String ifName, IIpClientCallbacks callback,
-            INetworkManagementService nwService) {
+            NetworkObserverRegistry observerRegistry) {
-        this(context, ifName, callback, new Dependencies() {
+        this(context, ifName, callback, observerRegistry, new Dependencies());
-            @Override
-            public INetworkManagementService getNMS() {
-                return nwService;
-            }
-        });
     }
 
     @VisibleForTesting
-    IpClient(Context context, String ifName, IIpClientCallbacks callback, Dependencies deps) {
+    IpClient(Context context, String ifName, IIpClientCallbacks callback,
+            NetworkObserverRegistry observerRegistry, Dependencies deps) {
         super(IpClient.class.getSimpleName() + "." + ifName);
         Preconditions.checkNotNull(ifName);
         Preconditions.checkNotNull(callback);
@@ -422,7 +399,7 @@ public class IpClient extends StateMachine {
         mDependencies = deps;
         mShutdownLatch = new CountDownLatch(1);
         mCm = mContext.getSystemService(ConnectivityManager.class);
-        mNwService = deps.getNMS();
+        mObserverRegistry = observerRegistry;
 
         sSmLogs.putIfAbsent(mInterfaceName, new SharedLog(MAX_LOG_RECORDS, mTag));
         mLog = sSmLogs.get(mInterfaceName);
@@ -433,19 +410,15 @@ public class IpClient extends StateMachine {
 
         // TODO: Consider creating, constructing, and passing in some kind of
         // InterfaceController.Dependencies class.
-        mInterfaceCtrl = new InterfaceController(mInterfaceName, deps.getNetd(), mLog);
+        mNetd = mContext.getSystemService(INetd.class);
+        mInterfaceCtrl = new InterfaceController(mInterfaceName, mNetd, mLog);
 
-        mNetlinkTracker = new NetlinkTracker(
+        mLinkObserver = new IpClientLinkObserver(
                 mInterfaceName,
-                new NetlinkTracker.Callback() {
+                () -> sendMessage(EVENT_NETLINK_LINKPROPERTIES_CHANGED)) {
-                    @Override
-                    public void update() {
-                        sendMessage(EVENT_NETLINK_LINKPROPERTIES_CHANGED);
-                    }
-                }) {
             @Override
-            public void interfaceAdded(String iface) {
+            public void onInterfaceAdded(String iface) {
-                super.interfaceAdded(iface);
+                super.onInterfaceAdded(iface);
                 if (mClatInterfaceName.equals(iface)) {
                     mCallback.setNeighborDiscoveryOffload(false);
                 } else if (!mInterfaceName.equals(iface)) {
@@ -457,8 +430,8 @@ public class IpClient extends StateMachine {
             }
 
             @Override
-            public void interfaceRemoved(String iface) {
+            public void onInterfaceRemoved(String iface) {
-                super.interfaceRemoved(iface);
+                super.onInterfaceRemoved(iface);
                 // TODO: Also observe mInterfaceName going down and take some
                 // kind of appropriate action.
                 if (mClatInterfaceName.equals(iface)) {
@@ -570,19 +543,11 @@ public class IpClient extends StateMachine {
     }
 
     private void startStateMachineUpdaters() {
-        try {
+        mObserverRegistry.registerObserver(mLinkObserver, getHandler());
-            mNwService.registerObserver(mNetlinkTracker);
-        } catch (RemoteException e) {
-            logError("Couldn't register NetlinkTracker: %s", e);
-        }
     }
 
     private void stopStateMachineUpdaters() {
-        try {
+        mObserverRegistry.unregisterObserver(mLinkObserver);
-            mNwService.unregisterObserver(mNetlinkTracker);
-        } catch (RemoteException e) {
-            logError("Couldn't unregister NetlinkTracker: %s", e);
-        }
     }
 
     @Override
@@ -805,7 +770,7 @@ public class IpClient extends StateMachine {
     // we should only call this if we know for sure that there are no IP addresses
     // assigned to the interface, etc.
     private void resetLinkProperties() {
-        mNetlinkTracker.clearLinkProperties();
+        mLinkObserver.clearLinkProperties();
         mConfiguration = null;
         mDhcpResults = null;
         mTcpBufferSizes = "";
@@ -984,10 +949,10 @@ public class IpClient extends StateMachine {
         //         - IPv6 DNS servers
         //
         // N.B.: this is fundamentally race-prone and should be fixed by
-        // changing NetlinkTracker from a hybrid edge/level model to an
+        // changing IpClientLinkObserver from a hybrid edge/level model to an
         // edge-only model, or by giving IpClient its own netlink socket(s)
         // so as to track all required information directly.
-        LinkProperties netlinkLinkProperties = mNetlinkTracker.getLinkProperties();
+        LinkProperties netlinkLinkProperties = mLinkObserver.getLinkProperties();
         newLp.setLinkAddresses(netlinkLinkProperties.getLinkAddresses());
         for (RouteInfo route : netlinkLinkProperties.getRoutes()) {
             newLp.addRoute(route);
@@ -1166,8 +1131,7 @@ public class IpClient extends StateMachine {
             // necessary or does reading from settings at startup suffice?).
             final int numSolicits = 5;
             final int interSolicitIntervalMs = 750;
-            setNeighborParameters(mDependencies.getNetd(), mInterfaceName,
+            setNeighborParameters(mNetd, mInterfaceName, numSolicits, interSolicitIntervalMs);
-                    numSolicits, interSolicitIntervalMs);
         } catch (Exception e) {
             mLog.e("Failed to adjust neighbor parameters", e);
             // Carry on using the system defaults (currently: 3, 1000);

packages/NetworkStack/src/android/net/ip/IpClientLinkObserver.java

@@ -0,0 +1,378 @@
+/*
+ * Copyright (C) 2014 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.ip;
+
+import android.net.InetAddresses;
+import android.net.LinkAddress;
+import android.net.LinkProperties;
+import android.net.RouteInfo;
+import android.util.Log;
+
+import com.android.server.NetworkObserver;
+
+import java.net.InetAddress;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Keeps track of link configuration received from Netd.
+ *
+ * An instance of this class is constructed by passing in an interface name and a callback. The
+ * owner is then responsible for registering the tracker with NetworkObserverRegistry. When the
+ * class receives update notifications, it applies the update to its local LinkProperties, and if
+ * something has changed, notifies its owner of the update via the callback.
+ *
+ * The owner can then call {@code getLinkProperties()} in order to find out
+ * what changed. If in the meantime the LinkProperties stored here have changed,
+ * this class will return the current LinkProperties. Because each change
+ * triggers an update callback after the change is made, the owner may get more
+ * callbacks than strictly necessary (some of which may be no-ops), but will not
+ * be out of sync once all callbacks have been processed.
+ *
+ * Threading model:
+ *
+ * - The owner of this class is expected to create it, register it, and call
+ *   getLinkProperties or clearLinkProperties on its thread.
+ * - Most of the methods in the class are implementing NetworkObserver and are called
+ *   on the handler used to register the observer.
+ * - All accesses to mLinkProperties must be synchronized(this). All the other
+ *   member variables are immutable once the object is constructed.
+ *
+ * @hide
+ */
+public class IpClientLinkObserver implements NetworkObserver {
+    private final String mTag;
+
+    /**
+     * Callback used by {@link IpClientLinkObserver} to send update notifications.
+     */
+    public interface Callback {
+        /**
+         * Called when some properties of the link were updated.
+         */
+        void update();
+    }
+
+    private final String mInterfaceName;
+    private final Callback mCallback;
+    private final LinkProperties mLinkProperties;
+    private DnsServerRepository mDnsServerRepository;
+
+    private static final boolean DBG = false;
+
+    public IpClientLinkObserver(String iface, Callback callback) {
+        mTag = "NetlinkTracker/" + iface;
+        mInterfaceName = iface;
+        mCallback = callback;
+        mLinkProperties = new LinkProperties();
+        mLinkProperties.setInterfaceName(mInterfaceName);
+        mDnsServerRepository = new DnsServerRepository();
+    }
+
+    private void maybeLog(String operation, String iface, LinkAddress address) {
+        if (DBG) {
+            Log.d(mTag, operation + ": " + address + " on " + iface
+                    + " flags " + address.getFlags() + " scope " + address.getScope());
+        }
+    }
+
+    private void maybeLog(String operation, Object o) {
+        if (DBG) {
+            Log.d(mTag, operation + ": " + o.toString());
+        }
+    }
+
+    @Override
+    public void onInterfaceRemoved(String iface) {
+        maybeLog("interfaceRemoved", iface);
+        if (mInterfaceName.equals(iface)) {
+            // Our interface was removed. Clear our LinkProperties and tell our owner that they are
+            // now empty. Note that from the moment that the interface is removed, any further
+            // interface-specific messages (e.g., RTM_DELADDR) will not reach us, because the netd
+            // code that parses them will not be able to resolve the ifindex to an interface name.
+            clearLinkProperties();
+            mCallback.update();
+        }
+    }
+
+    @Override
+    public void onInterfaceAddressUpdated(LinkAddress address, String iface) {
+        if (mInterfaceName.equals(iface)) {
+            maybeLog("addressUpdated", iface, address);
+            boolean changed;
+            synchronized (this) {
+                changed = mLinkProperties.addLinkAddress(address);
+            }
+            if (changed) {
+                mCallback.update();
+            }
+        }
+    }
+
+    @Override
+    public void onInterfaceAddressRemoved(LinkAddress address, String iface) {
+        if (mInterfaceName.equals(iface)) {
+            maybeLog("addressRemoved", iface, address);
+            boolean changed;
+            synchronized (this) {
+                changed = mLinkProperties.removeLinkAddress(address);
+            }
+            if (changed) {
+                mCallback.update();
+            }
+        }
+    }
+
+    @Override
+    public void onRouteUpdated(RouteInfo route) {
+        if (mInterfaceName.equals(route.getInterface())) {
+            maybeLog("routeUpdated", route);
+            boolean changed;
+            synchronized (this) {
+                changed = mLinkProperties.addRoute(route);
+            }
+            if (changed) {
+                mCallback.update();
+            }
+        }
+    }
+
+    @Override
+    public void onRouteRemoved(RouteInfo route) {
+        if (mInterfaceName.equals(route.getInterface())) {
+            maybeLog("routeRemoved", route);
+            boolean changed;
+            synchronized (this) {
+                changed = mLinkProperties.removeRoute(route);
+            }
+            if (changed) {
+                mCallback.update();
+            }
+        }
+    }
+
+    @Override
+    public void onInterfaceDnsServerInfo(String iface, long lifetime, String[] addresses) {
+        if (mInterfaceName.equals(iface)) {
+            maybeLog("interfaceDnsServerInfo", Arrays.toString(addresses));
+            boolean changed = mDnsServerRepository.addServers(lifetime, addresses);
+            if (changed) {
+                synchronized (this) {
+                    mDnsServerRepository.setDnsServersOn(mLinkProperties);
+                }
+                mCallback.update();
+            }
+        }
+    }
+
+    /**
+     * Returns a copy of this object's LinkProperties.
+     */
+    public synchronized LinkProperties getLinkProperties() {
+        return new LinkProperties(mLinkProperties);
+    }
+
+    /**
+     * Reset this object's LinkProperties.
+     */
+    public synchronized void clearLinkProperties() {
+        // Clear the repository before clearing mLinkProperties. That way, if a clear() happens
+        // while interfaceDnsServerInfo() is being called, we'll end up with no DNS servers in
+        // mLinkProperties, as desired.
+        mDnsServerRepository = new DnsServerRepository();
+        mLinkProperties.clear();
+        mLinkProperties.setInterfaceName(mInterfaceName);
+    }
+
+    /**
+     * Tracks DNS server updates received from Netlink.
+     *
+     * The network may announce an arbitrary number of DNS servers in Router Advertisements at any
+     * time. Each announcement has a lifetime; when the lifetime expires, the servers should not be
+     * used any more. In this way, the network can gracefully migrate clients from one set of DNS
+     * servers to another. Announcements can both raise and lower the lifetime, and an announcement
+     * can expire servers by announcing them with a lifetime of zero.
+     *
+     * Typically the system will only use a small number (2 or 3; {@code NUM_CURRENT_SERVERS}) of
+     * DNS servers at any given time. These are referred to as the current servers. In case all the
+     * current servers expire, the class also keeps track of a larger (but limited) number of
+     * servers that are promoted to current servers when the current ones expire. In order to
+     * minimize updates to the rest of the system (and potentially expensive cache flushes) this
+     * class attempts to keep the list of current servers constant where possible. More
+     * specifically, the list of current servers is only updated if a new server is learned and
+     * there are not yet {@code NUM_CURRENT_SERVERS} current servers, or if one or more of the
+     * current servers expires or is pushed out of the set. Therefore, the current servers will not
+     * necessarily be the ones with the highest lifetime, but the ones learned first.
+     *
+     * This is by design: if instead the class always preferred the servers with the highest
+     * lifetime, a (misconfigured?) network where two or more routers announce more than
+     * {@code NUM_CURRENT_SERVERS} unique servers would cause persistent oscillations.
+     *
+     * TODO: Currently servers are only expired when a new DNS update is received.
+     * Update them using timers, or possibly on every notification received by NetlinkTracker.
+     *
+     * Threading model: run by NetlinkTracker. Methods are synchronized(this) just in case netlink
+     * notifications are sent by multiple threads. If future threads use alarms to expire, those
+     * alarms must also be synchronized(this).
+     *
+     */
+    private static class DnsServerRepository {
+
+        /** How many DNS servers we will use. 3 is suggested by RFC 6106. */
+        static final int NUM_CURRENT_SERVERS = 3;
+
+        /** How many DNS servers we'll keep track of, in total. */
+        static final int NUM_SERVERS = 12;
+
+        /** Stores up to {@code NUM_CURRENT_SERVERS} DNS servers we're currently using. */
+        private Set<InetAddress> mCurrentServers;
+
+        public static final String TAG = "DnsServerRepository";
+
+        /**
+         * Stores all the DNS servers we know about, for use when the current servers expire.
+         * Always sorted in order of decreasing expiry. The elements in this list are also the
+         * values of mIndex, and may be elements in mCurrentServers.
+         */
+        private ArrayList<DnsServerEntry> mAllServers;
+
+        /**
+         * Indexes the servers so we can update their lifetimes more quickly in the common case
+         * where servers are not being added, but only being refreshed.
+         */
+        private HashMap<InetAddress, DnsServerEntry> mIndex;
+
+        DnsServerRepository() {
+            mCurrentServers = new HashSet<>();
+            mAllServers = new ArrayList<>(NUM_SERVERS);
+            mIndex = new HashMap<>(NUM_SERVERS);
+        }
+
+        /** Sets the DNS servers of the provided LinkProperties object to the current servers. */
+        public synchronized void setDnsServersOn(LinkProperties lp) {
+            lp.setDnsServers(mCurrentServers);
+        }
+
+        /**
+         * Notifies the class of new DNS server information.
+         * @param lifetime the time in seconds that the DNS servers are valid.
+         * @param addresses the string representations of the IP addresses of DNS servers to use.
+         */
+        public synchronized boolean addServers(long lifetime, String[] addresses) {
+            // The lifetime is actually an unsigned 32-bit number, but Java doesn't have unsigned.
+            // Technically 0xffffffff (the maximum) is special and means "forever", but 2^32 seconds
+            // (136 years) is close enough.
+            long now = System.currentTimeMillis();
+            long expiry = now + 1000 * lifetime;
+
+            // Go through the list of servers. For each one, update the entry if one exists, and
+            // create one if it doesn't.
+            for (String addressString : addresses) {
+                InetAddress address;
+                try {
+                    address = InetAddresses.parseNumericAddress(addressString);
+                } catch (IllegalArgumentException ex) {
+                    continue;
+                }
+
+                if (!updateExistingEntry(address, expiry)) {
+                    // There was no entry for this server. Create one, unless it's already expired
+                    // (i.e., if the lifetime is zero; it cannot be < 0 because it's unsigned).
+                    if (expiry > now) {
+                        DnsServerEntry entry = new DnsServerEntry(address, expiry);
+                        mAllServers.add(entry);
+                        mIndex.put(address, entry);
+                    }
+                }
+            }
+
+            // Sort the servers by expiry.
+            Collections.sort(mAllServers);
+
+            // Prune excess entries and update the current server list.
+            return updateCurrentServers();
+        }
+
+        private synchronized boolean updateExistingEntry(InetAddress address, long expiry) {
+            DnsServerEntry existing = mIndex.get(address);
+            if (existing != null) {
+                existing.expiry = expiry;
+                return true;
+            }
+            return false;
+        }
+
+        private synchronized boolean updateCurrentServers() {
+            long now = System.currentTimeMillis();
+            boolean changed = false;
+
+            // Prune excess or expired entries.
+            for (int i = mAllServers.size() - 1; i >= 0; i--) {
+                if (i >= NUM_SERVERS || mAllServers.get(i).expiry < now) {
+                    DnsServerEntry removed = mAllServers.remove(i);
+                    mIndex.remove(removed.address);
+                    changed |= mCurrentServers.remove(removed.address);
+                } else {
+                    break;
+                }
+            }
+
+            // Add servers to the current set, in order of decreasing lifetime, until it has enough.
+            // Prefer existing servers over new servers in order to minimize updates to the rest of
+            // the system and avoid persistent oscillations.
+            for (DnsServerEntry entry : mAllServers) {
+                if (mCurrentServers.size() < NUM_CURRENT_SERVERS) {
+                    changed |= mCurrentServers.add(entry.address);
+                } else {
+                    break;
+                }
+            }
+            return changed;
+        }
+    }
+
+    /**
+     * Represents a DNS server entry with an expiry time.
+     *
+     * Implements Comparable so DNS server entries can be sorted by lifetime, longest-lived first.
+     * The ordering of entries with the same lifetime is unspecified, because given two servers with
+     * identical lifetimes, we don't care which one we use, and only comparing the lifetime is much
+     * faster than comparing the IP address as well.
+     *
+     * Note: this class has a natural ordering that is inconsistent with equals.
+     */
+    private static class DnsServerEntry implements Comparable<DnsServerEntry> {
+        /** The IP address of the DNS server. */
+        public final InetAddress address;
+        /** The time until which the DNS server may be used. A Java millisecond time as might be
+         * returned by currentTimeMillis(). */
+        public long expiry;
+
+        DnsServerEntry(InetAddress address, long expiry) throws IllegalArgumentException {
+            this.address = address;
+            this.expiry = expiry;
+        }
+
+        public int compareTo(DnsServerEntry other) {
+            return Long.compare(other.expiry, this.expiry);
+        }
+    }
+}

packages/NetworkStack/src/com/android/server/NetworkObserver.java

@@ -17,6 +17,7 @@
 package com.android.server;
 
 import android.net.LinkAddress;
+import android.net.RouteInfo;
 
 /**
  * Observer for network events, to use with {@link NetworkObserverRegistry}.
@@ -77,11 +78,11 @@ public interface NetworkObserver {
      * @see android.net.INetdUnsolicitedEventListener
      *          #onRouteChanged(boolean, String, String, String)
      */
-    default void onRouteUpdated(String route, String gateway, String ifName) {}
+    default void onRouteUpdated(RouteInfo route) {}
 
     /**
      * @see android.net.INetdUnsolicitedEventListener
      *          #onRouteChanged(boolean, String, String, String)
      */
-    default void onRouteRemoved(String route, String gateway, String ifName) {}
+    default void onRouteRemoved(RouteInfo route) {}
 }

packages/NetworkStack/src/com/android/server/NetworkObserverRegistry.java

@@ -18,7 +18,10 @@ package com.android.server;
 import android.annotation.NonNull;
 import android.net.INetd;
 import android.net.INetdUnsolicitedEventListener;
+import android.net.InetAddresses;
+import android.net.IpPrefix;
 import android.net.LinkAddress;
+import android.net.RouteInfo;
 import android.os.Handler;
 import android.os.RemoteException;
 
@@ -138,10 +141,13 @@ public class NetworkObserverRegistry extends INetdUnsolicitedEventListener.Stub
 
     @Override
     public void onRouteChanged(boolean updated, String route, String gateway, String ifName) {
+        final RouteInfo processRoute = new RouteInfo(new IpPrefix(route),
+                ("".equals(gateway)) ? null : InetAddresses.parseNumericAddress(gateway),
+                ifName);
         if (updated) {
-            invokeForAllObservers(o -> o.onRouteUpdated(route, gateway, ifName));
+            invokeForAllObservers(o -> o.onRouteUpdated(processRoute));
         } else {
-            invokeForAllObservers(o -> o.onRouteRemoved(route, gateway, ifName));
+            invokeForAllObservers(o -> o.onRouteRemoved(processRoute));
         }
     }
 

packages/NetworkStack/src/com/android/server/NetworkStackService.java

@@ -117,7 +117,11 @@ public class NetworkStackService extends Service {
             mObserverRegistry = new NetworkObserverRegistry();
             mCm = context.getSystemService(ConnectivityManager.class);
 
-            // TODO: call mObserverRegistry here after adding sepolicy changes
+            try {
+                mObserverRegistry.register(mNetd);
+            } catch (RemoteException e) {
+                mLog.e("Error registering observer on Netd", e);
+            }
         }
 
         @NonNull
@@ -158,7 +162,7 @@ public class NetworkStackService extends Service {
 
         @Override
         public void makeIpClient(String ifName, IIpClientCallbacks cb) throws RemoteException {
-            final IpClient ipClient = new IpClient(mContext, ifName, cb);
+            final IpClient ipClient = new IpClient(mContext, ifName, cb, mObserverRegistry);
 
             synchronized (mIpClients) {
                 final Iterator<WeakReference<IpClient>> it = mIpClients.iterator();

packages/NetworkStack/tests/src/android/net/ip/IpClientTest.java

@@ -46,7 +46,6 @@ import android.net.RouteInfo;
 import android.net.shared.InitialConfiguration;
 import android.net.shared.ProvisioningConfiguration;
 import android.net.util.InterfaceParams;
-import android.os.INetworkManagementService;
 import android.provider.Settings;
 import android.support.test.filters.SmallTest;
 import android.support.test.runner.AndroidJUnit4;
@@ -54,7 +53,8 @@ import android.test.mock.MockContentResolver;
 
 import com.android.internal.R;
 import com.android.internal.util.test.FakeSettingsProvider;
-import com.android.server.net.BaseNetworkObserver;
+import com.android.server.NetworkObserver;
+import com.android.server.NetworkObserverRegistry;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -87,15 +87,15 @@ public class IpClientTest {
 
     @Mock private Context mContext;
     @Mock private ConnectivityManager mCm;
-    @Mock private INetworkManagementService mNMService;
+    @Mock private NetworkObserverRegistry mObserverRegistry;
     @Mock private INetd mNetd;
     @Mock private Resources mResources;
     @Mock private IIpClientCallbacks mCb;
     @Mock private AlarmManager mAlarm;
-    @Mock private IpClient.Dependencies mDependecies;
+    @Mock private IpClient.Dependencies mDependencies;
     private MockContentResolver mContentResolver;
 
-    private BaseNetworkObserver mObserver;
+    private NetworkObserver mObserver;
     private InterfaceParams mIfParams;
 
     @Before
@@ -104,6 +104,7 @@ public class IpClientTest {
 
         when(mContext.getSystemService(eq(Context.ALARM_SERVICE))).thenReturn(mAlarm);
         when(mContext.getSystemService(eq(ConnectivityManager.class))).thenReturn(mCm);
+        when(mContext.getSystemService(INetd.class)).thenReturn(mNetd);
         when(mContext.getResources()).thenReturn(mResources);
         when(mResources.getInteger(R.integer.config_networkAvoidBadWifi))
                 .thenReturn(DEFAULT_AVOIDBADWIFI_CONFIG_VALUE);
@@ -113,28 +114,24 @@ public class IpClientTest {
         when(mContext.getContentResolver()).thenReturn(mContentResolver);
 
         mIfParams = null;
-
-        when(mDependecies.getNMS()).thenReturn(mNMService);
-        when(mDependecies.getNetd()).thenReturn(mNetd);
     }
 
     private void setTestInterfaceParams(String ifname) {
         mIfParams = (ifname != null)
                 ? new InterfaceParams(ifname, TEST_IFINDEX, TEST_MAC)
                 : null;
-        when(mDependecies.getInterfaceParams(anyString())).thenReturn(mIfParams);
+        when(mDependencies.getInterfaceParams(anyString())).thenReturn(mIfParams);
     }
 
     private IpClient makeIpClient(String ifname) throws Exception {
         setTestInterfaceParams(ifname);
-        final IpClient ipc = new IpClient(mContext, ifname, mCb, mDependecies);
+        final IpClient ipc = new IpClient(mContext, ifname, mCb, mObserverRegistry, mDependencies);
         verify(mNetd, timeout(TEST_TIMEOUT_MS).times(1)).interfaceSetEnableIPv6(ifname, false);
         verify(mNetd, timeout(TEST_TIMEOUT_MS).times(1)).interfaceClearAddrs(ifname);
-        ArgumentCaptor<BaseNetworkObserver> arg =
+        ArgumentCaptor<NetworkObserver> arg = ArgumentCaptor.forClass(NetworkObserver.class);
-                ArgumentCaptor.forClass(BaseNetworkObserver.class);
+        verify(mObserverRegistry, times(1)).registerObserver(arg.capture(), any());
-        verify(mNMService, times(1)).registerObserver(arg.capture());
         mObserver = arg.getValue();
-        reset(mNMService);
+        reset(mObserverRegistry);
         reset(mNetd);
         // Verify IpClient doesn't call onLinkPropertiesChange() when it starts.
         verify(mCb, never()).onLinkPropertiesChange(any());
@@ -152,7 +149,8 @@ public class IpClientTest {
     public void testNullInterfaceNameMostDefinitelyThrows() throws Exception {
         setTestInterfaceParams(null);
         try {
-            final IpClient ipc = new IpClient(mContext, null, mCb, mDependecies);
+            final IpClient ipc = new IpClient(
+                    mContext, null, mCb, mObserverRegistry, mDependencies);
             ipc.shutdown();
             fail();
         } catch (NullPointerException npe) {
@@ -165,7 +163,8 @@ public class IpClientTest {
         final String ifname = "lo";
         setTestInterfaceParams(ifname);
         try {
-            final IpClient ipc = new IpClient(mContext, ifname, null, mDependecies);
+            final IpClient ipc = new IpClient(
+                    mContext, ifname, null, mObserverRegistry, mDependencies);
             ipc.shutdown();
             fail();
         } catch (NullPointerException npe) {
@@ -176,14 +175,16 @@ public class IpClientTest {
     @Test
     public void testInvalidInterfaceDoesNotThrow() throws Exception {
         setTestInterfaceParams(TEST_IFNAME);
-        final IpClient ipc = new IpClient(mContext, TEST_IFNAME, mCb, mDependecies);
+        final IpClient ipc = new IpClient(
+                mContext, TEST_IFNAME, mCb, mObserverRegistry, mDependencies);
         ipc.shutdown();
     }
 
     @Test
     public void testInterfaceNotFoundFailsImmediately() throws Exception {
         setTestInterfaceParams(null);
-        final IpClient ipc = new IpClient(mContext, TEST_IFNAME, mCb, mDependecies);
+        final IpClient ipc = new IpClient(
+                mContext, TEST_IFNAME, mCb, mObserverRegistry, mDependencies);
         ipc.startProvisioning(new ProvisioningConfiguration());
         verify(mCb, times(1)).onProvisioningFailure(any());
         ipc.shutdown();
@@ -247,13 +248,13 @@ public class IpClientTest {
 
         // Add N - 1 addresses
         for (int i = 0; i < lastAddr; i++) {
-            mObserver.addressUpdated(iface, new LinkAddress(addresses[i]));
+            mObserver.onInterfaceAddressUpdated(new LinkAddress(addresses[i]), iface);
             verify(mCb, timeout(TEST_TIMEOUT_MS)).onLinkPropertiesChange(any());
             reset(mCb);
         }
 
         // Add Nth address
-        mObserver.addressUpdated(iface, new LinkAddress(addresses[lastAddr]));
+        mObserver.onInterfaceAddressUpdated(new LinkAddress(addresses[lastAddr]), iface);
         LinkProperties want = linkproperties(links(addresses), routes(prefixes));
         want.setInterfaceName(iface);
         verify(mCb, timeout(TEST_TIMEOUT_MS).times(1)).onProvisioningSuccess(argThat(