Merge "Fix a bug where the PAC proxy port is not set correctly."

framework/src/android/net/ProxyInfo.java

@@ -47,6 +47,8 @@ public class ProxyInfo implements Parcelable {
     private final int mPort;
     private final String mExclusionList;
     private final String[] mParsedExclusionList;
+    // Uri.EMPTY if none.
+    @NonNull
     private final Uri mPacFileUrl;
 
     /**
@@ -256,6 +258,14 @@ public class ProxyInfo implements Parcelable {
         return proxy;
     }
 
+    /**
+     * @hide
+     * @return whether this proxy uses a Proxy Auto Configuration URL.
+     */
+    public boolean isPacProxy() {
+        return !Uri.EMPTY.equals(mPacFileUrl);
+    }
+
     @Override
     public String toString() {
         StringBuilder sb = new StringBuilder();

service/src/com/android/server/ConnectivityService.java

@@ -34,6 +34,7 @@ import static android.net.ConnectivityDiagnosticsManager.DataStallReport.KEY_TCP
 import static android.net.ConnectivityManager.BLOCKED_METERED_REASON_MASK;
 import static android.net.ConnectivityManager.BLOCKED_REASON_LOCKDOWN_VPN;
 import static android.net.ConnectivityManager.BLOCKED_REASON_NONE;
+import static android.net.ConnectivityManager.CALLBACK_IP_CHANGED;
 import static android.net.ConnectivityManager.CONNECTIVITY_ACTION;
 import static android.net.ConnectivityManager.FIREWALL_RULE_ALLOW;
 import static android.net.ConnectivityManager.FIREWALL_RULE_DEFAULT;
@@ -551,7 +552,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     /**
      * PAC manager has received new port.
      */
-    private static final int EVENT_PROXY_HAS_CHANGED = 16;
+    private static final int EVENT_PAC_PROXY_HAS_CHANGED = 16;
 
     /**
      * used internally when registering NetworkProviders
@@ -1325,7 +1326,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
          */
         public ProxyTracker makeProxyTracker(@NonNull Context context,
                 @NonNull Handler connServiceHandler) {
-            return new ProxyTracker(context, connServiceHandler, EVENT_PROXY_HAS_CHANGED);
+            return new ProxyTracker(context, connServiceHandler, EVENT_PAC_PROXY_HAS_CHANGED);
         }
 
         /**
@@ -1894,7 +1895,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     @VisibleForTesting
     void simulateUpdateProxyInfo(@Nullable final Network network,
             @NonNull final ProxyInfo proxyInfo) {
-        Message.obtain(mHandler, EVENT_PROXY_HAS_CHANGED,
+        Message.obtain(mHandler, EVENT_PAC_PROXY_HAS_CHANGED,
                 new Pair<>(network, proxyInfo)).sendToTarget();
     }
 
@@ -4680,6 +4681,21 @@ public class ConnectivityService extends IConnectivityManager.Stub
         rematchAllNetworksAndRequests();
         mLingerMonitor.noteDisconnect(nai);
 
+        if (null == getDefaultNetwork() && nai.linkProperties.getHttpProxy() != null) {
+            // The obvious place to do this would be in makeDefault(), however makeDefault() is
+            // not called by the rematch in this case. This is because the code above unset
+            // this network from the default request's satisfier, and that is what the rematch
+            // is using as its source data to know what the old satisfier was. So as far as the
+            // rematch above is concerned, the old default network was null.
+            // Therefore if there is no new default, the default network was null and is still
+            // null, thus there was no change so makeDefault() is not called. So if the old
+            // network had a proxy and there is no new default, the proxy tracker should be told
+            // that there is no longer a default proxy.
+            // Strictly speaking this is not essential because having a proxy setting when
+            // there is no network is harmless, but it's still counter-intuitive so reset to null.
+            mProxyTracker.setDefaultProxy(null);
+        }
+
         // Immediate teardown.
         if (nai.teardownDelayMs == 0) {
             destroyNetwork(nai);
@@ -5702,9 +5718,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
                     mProxyTracker.loadDeprecatedGlobalHttpProxy();
                     break;
                 }
-                case EVENT_PROXY_HAS_CHANGED: {
+                case EVENT_PAC_PROXY_HAS_CHANGED: {
                     final Pair<Network, ProxyInfo> arg = (Pair<Network, ProxyInfo>) msg.obj;
-                    handleApplyDefaultProxy(arg.second);
+                    handlePacProxyServiceStarted(arg.first, arg.second);
                     break;
                 }
                 case EVENT_REGISTER_NETWORK_PROVIDER: {
@@ -6159,12 +6175,19 @@ public class ConnectivityService extends IConnectivityManager.Stub
         return mProxyTracker.getGlobalProxy();
     }
 
-    private void handleApplyDefaultProxy(@Nullable ProxyInfo proxy) {
+    private void handlePacProxyServiceStarted(@Nullable Network net, @Nullable ProxyInfo proxy) {
-        if (proxy != null && TextUtils.isEmpty(proxy.getHost())
-                && Uri.EMPTY.equals(proxy.getPacFileUrl())) {
-            proxy = null;
-        }
         mProxyTracker.setDefaultProxy(proxy);
+        final NetworkAgentInfo nai = getDefaultNetwork();
+        // TODO : this method should check that net == nai.network, unfortunately at this point
+        // 'net' is always null in practice (see PacProxyService#sendPacBroadcast). PAC proxy
+        // is only ever installed on the default network so in practice this is okay.
+        if (null == nai) return;
+        // PAC proxies only work on the default network. Therefore, only the default network
+        // should have its link properties fixed up for PAC proxies.
+        mProxyTracker.updateDefaultNetworkProxyPortForPAC(nai.linkProperties, nai.network);
+        if (nai.everConnected()) {
+            notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_IP_CHANGED);
+        }
     }
 
     // If the proxy has changed from oldLp to newLp, resend proxy broadcast. This method gets called
@@ -7990,6 +8013,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
 //            updateMtu(lp, null);
 //        }
         if (isDefaultNetwork(networkAgent)) {
+            mProxyTracker.updateDefaultNetworkProxyPortForPAC(newLp, null);
             updateTcpBufferSizes(newLp.getTcpBufferSizes());
         }
 
@@ -8002,7 +8026,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         mDnsManager.updatePrivateDnsStatus(netId, newLp);
 
         if (isDefaultNetwork(networkAgent)) {
-            handleApplyDefaultProxy(newLp.getHttpProxy());
+            mProxyTracker.setDefaultProxy(newLp.getHttpProxy());
         } else if (networkAgent.everConnected()) {
             updateProxy(newLp, oldLp);
         }
@@ -9054,6 +9078,17 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
     }
 
+    private void resetHttpProxyForNonDefaultNetwork(NetworkAgentInfo oldDefaultNetwork) {
+        if (null == oldDefaultNetwork) return;
+        // The network stopped being the default. If it was using a PAC proxy, then the
+        // proxy needs to be reset, otherwise HTTP requests on this network may be sent
+        // to the local proxy server, which would forward them over the newly default network.
+        final ProxyInfo proxyInfo = oldDefaultNetwork.linkProperties.getHttpProxy();
+        if (null == proxyInfo || !proxyInfo.isPacProxy()) return;
+        oldDefaultNetwork.linkProperties.setHttpProxy(new ProxyInfo(proxyInfo.getPacFileUrl()));
+        notifyNetworkCallbacks(oldDefaultNetwork, CALLBACK_IP_CHANGED);
+    }
+
     private void makeDefault(@NonNull final NetworkRequestInfo nri,
             @Nullable final NetworkAgentInfo oldDefaultNetwork,
             @Nullable final NetworkAgentInfo newDefaultNetwork) {
@@ -9079,8 +9114,9 @@ public class ConnectivityService extends IConnectivityManager.Stub
             mLingerMonitor.noteLingerDefaultNetwork(oldDefaultNetwork, newDefaultNetwork);
         }
         mNetworkActivityTracker.updateDataActivityTracking(newDefaultNetwork, oldDefaultNetwork);
-        handleApplyDefaultProxy(null != newDefaultNetwork
+        mProxyTracker.setDefaultProxy(null != newDefaultNetwork
                 ? newDefaultNetwork.linkProperties.getHttpProxy() : null);
+        resetHttpProxyForNonDefaultNetwork(oldDefaultNetwork);
         updateTcpBufferSizes(null != newDefaultNetwork
                 ? newDefaultNetwork.linkProperties.getTcpBufferSizes() : null);
         notifyIfacesChangedForNetworkStats();

service/src/com/android/server/connectivity/ProxyTracker.java

@@ -27,6 +27,7 @@ import android.annotation.Nullable;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.content.Intent;
+import android.net.LinkProperties;
 import android.net.Network;
 import android.net.PacProxyManager;
 import android.net.Proxy;
@@ -95,6 +96,7 @@ public class ProxyTracker {
         }
 
         public void onPacProxyInstalled(@Nullable Network network, @NonNull ProxyInfo proxy) {
+            Log.i(TAG, "PAC proxy installed on network " + network + " : " + proxy);
             mConnectivityServiceHandler
                     .sendMessage(mConnectivityServiceHandler
                     .obtainMessage(mEvent, new Pair<>(network, proxy)));
@@ -328,6 +330,12 @@ public class ProxyTracker {
      * @param proxyInfo the proxy spec, or null for no proxy.
      */
     public void setDefaultProxy(@Nullable ProxyInfo proxyInfo) {
+        // The code has been accepting empty proxy objects forever, so for backward
+        // compatibility it should continue doing so.
+        if (proxyInfo != null && TextUtils.isEmpty(proxyInfo.getHost())
+                && Uri.EMPTY.equals(proxyInfo.getPacFileUrl())) {
+            proxyInfo = null;
+        }
         synchronized (mProxyLock) {
             if (Objects.equals(mDefaultProxy, proxyInfo)) return;
             if (proxyInfo != null && !proxyInfo.isValid()) {
@@ -355,4 +363,51 @@ public class ProxyTracker {
             }
         }
     }
+
+    private boolean isPacProxy(@Nullable final ProxyInfo info) {
+        return null != info && info.isPacProxy();
+    }
+
+    /**
+     * Adjust the proxy in the link properties if necessary.
+     *
+     * It is necessary when the proxy in the passed property is for PAC, and the default proxy
+     * is also for PAC. This is because the original LinkProperties from the network agent don't
+     * include the port for the local proxy as it's not known at creation time, but this class
+     * knows it after the proxy service is started.
+     *
+     * This is safe because there can only ever be one proxy service running on the device, so
+     * if the ProxyInfo in the LinkProperties is for PAC, then the port is necessarily the one
+     * ProxyTracker knows about.
+     *
+     * @param lp the LinkProperties to fix up.
+     * @param network the network of the local proxy server.
+     */
+    // TODO: Leave network unused to support local proxy server per network in the future.
+    public void updateDefaultNetworkProxyPortForPAC(@NonNull final LinkProperties lp,
+            @Nullable Network network) {
+        final ProxyInfo defaultProxy = getDefaultProxy();
+        if (isPacProxy(lp.getHttpProxy()) && isPacProxy(defaultProxy)) {
+            synchronized (mProxyLock) {
+                // At this time, this method can only be called for the default network's LP.
+                // Therefore the PAC file URL in the LP must match the one in the default proxy,
+                // and we just update the port.
+                // Note that the global proxy, if any, is set out of band by the DPM and becomes
+                // the default proxy (it overrides it, see {@link getDefaultProxy}). The PAC URL
+                // in the global proxy might not be the one in the LP of the default
+                // network, so discount this case.
+                if (null == mGlobalProxy && !lp.getHttpProxy().getPacFileUrl()
+                        .equals(defaultProxy.getPacFileUrl())) {
+                    throw new IllegalStateException("Unexpected discrepancy between proxy in LP of "
+                            + "default network and default proxy. The former has a PAC URL of "
+                            + lp.getHttpProxy().getPacFileUrl() + " while the latter has "
+                            + defaultProxy.getPacFileUrl());
+                }
+            }
+            // If this network has a PAC proxy and proxy tracker already knows about
+            // it, now is the right time to patch it in. If proxy tracker does not know
+            // about it yet, then it will be patched in when it learns about it.
+            lp.setHttpProxy(defaultProxy);
+        }
+    }
 }

tests/unit/java/com/android/server/ConnectivityServiceTest.java

@@ -2505,10 +2505,14 @@ public class ConnectivityServiceTest {
     }
 
     private ExpectedBroadcast expectProxyChangeAction(ProxyInfo proxy) {
+        return expectProxyChangeAction(actualProxy -> proxy.equals(actualProxy));
+    }
+
+    private ExpectedBroadcast expectProxyChangeAction(Predicate<ProxyInfo> tester) {
         return registerBroadcastReceiverThat(PROXY_CHANGE_ACTION, 1, intent -> {
             final ProxyInfo actualProxy = (ProxyInfo) intent.getExtra(Proxy.EXTRA_PROXY_INFO,
                     ProxyInfo.buildPacProxy(Uri.EMPTY));
-            return proxy.equals(actualProxy);
+            return tester.test(actualProxy);
         });
     }
 
@@ -11397,8 +11401,16 @@ public class ConnectivityServiceTest {
     @Test
     public void testPacProxy() throws Exception {
         final Uri pacUrl = Uri.parse("https://pac-url");
+
+        final TestNetworkCallback cellCallback = new TestNetworkCallback();
+        final NetworkRequest cellRequest = new NetworkRequest.Builder()
+                .addTransportType(TRANSPORT_CELLULAR).build();
+        // Request cell to make sure it doesn't disconnect at an arbitrary point in the test,
+        // which would make testing the callbacks on it difficult.
+        mCm.requestNetwork(cellRequest, cellCallback);
         mCellAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR);
         mCellAgent.connect(true);
+        cellCallback.expectAvailableThenValidatedCallbacks(mCellAgent);
 
         final TestNetworkCallback wifiCallback = new TestNetworkCallback();
         final NetworkRequest wifiRequest = new NetworkRequest.Builder()
@@ -11408,12 +11420,14 @@ public class ConnectivityServiceTest {
         mWiFiAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI);
         mWiFiAgent.connect(true);
         wifiCallback.expectAvailableThenValidatedCallbacks(mWiFiAgent);
+        cellCallback.assertNoCallback();
 
         final ProxyInfo initialProxyInfo = ProxyInfo.buildPacProxy(pacUrl);
         final LinkProperties testLinkProperties = new LinkProperties();
         testLinkProperties.setHttpProxy(initialProxyInfo);
         mWiFiAgent.sendLinkProperties(testLinkProperties);
         wifiCallback.expect(CallbackEntry.LINK_PROPERTIES_CHANGED, mWiFiAgent);
+        cellCallback.assertNoCallback();
 
         // At first the local PAC proxy server is unstarted (see the description of what the local
         // server is in the comment at the top of this method). It will contain the PAC URL and a
@@ -11443,98 +11457,154 @@ public class ConnectivityServiceTest {
 
         // Simulate PacManager sending the notification that the local server has started
         final ProxyInfo servingProxyInfo = new ProxyInfo(pacUrl, 2097);
-        final ExpectedBroadcast b1 = expectProxyChangeAction(servingProxyInfo);
+        final ExpectedBroadcast servingProxyBroadcast = expectProxyChangeAction(servingProxyInfo);
         mService.simulateUpdateProxyInfo(mWiFiAgent.getNetwork(), servingProxyInfo);
-//      wifiCallback.expect(CallbackEntry.LINK_PROPERTIES_CHANGED, mWiFiAgent);
+        wifiCallback.expect(CallbackEntry.LINK_PROPERTIES_CHANGED, mWiFiAgent);
-        b1.expectBroadcast();
+        cellCallback.assertNoCallback();
+        servingProxyBroadcast.expectBroadcast();
 
         final ProxyInfo startedDefaultProxyInfo = mService.getProxyForNetwork(null);
         final ProxyInfo startedWifiProxyInfo = mService.getProxyForNetwork(
                 mWiFiAgent.getNetwork());
         final LinkProperties startedLp = mService.getLinkProperties(mWiFiAgent.getNetwork());
-        // TODO : activate these tests when b/138810051 is fixed.
+        assertEquals(servingProxyInfo, startedDefaultProxyInfo);
-//      assertEquals(servingProxyInfo, startedDefaultProxyInfo);
+        assertEquals(servingProxyInfo, startedWifiProxyInfo);
-//      assertEquals(servingProxyInfo, startedWifiProxyInfo);
+        assertEquals(servingProxyInfo, startedLp.getHttpProxy());
-//      assertEquals(servingProxyInfo, startedLp.getHttpProxy());
+        // Make sure the cell network is still unaffected
-//      // Make sure the cell network is still unaffected
+        assertNull(mService.getLinkProperties(mCellAgent.getNetwork()).getHttpProxy());
-//      assertNull(mService.getLinkProperties(mCellAgent.getNetwork()).getHttpProxy());
+        assertNull(mService.getProxyForNetwork(mCellAgent.getNetwork()));
-//      assertNull(mService.getProxyForNetwork(mCellAgent.getNetwork()));
+
-//
+        // Start an ethernet network which will become the default, in order to test what happens
-//      final Uri ethPacUrl = Uri.parse("https://ethernet-pac-url");
+        // to the proxy of wifi while manipulating the proxy of ethernet.
-//      final TestableNetworkCallback ethernetCallback = new TestableNetworkCallback();
+        final Uri ethPacUrl = Uri.parse("https://ethernet-pac-url");
-//      final NetworkRequest ethernetRequest = new NetworkRequest.Builder()
+        final TestableNetworkCallback ethernetCallback = new TestableNetworkCallback();
-//              .addTransportType(TRANSPORT_ETHERNET).build();
+        final NetworkRequest ethernetRequest = new NetworkRequest.Builder()
-//      mEthernetAgent = new TestNetworkAgentWrapper(TRANSPORT_ETHERNET);
+                .addTransportType(TRANSPORT_ETHERNET).build();
-//      mCm.registerNetworkCallback(ethernetRequest, ethernetCallback);
+        mEthernetAgent = new TestNetworkAgentWrapper(TRANSPORT_ETHERNET);
-//      mEthernetAgent.connect(true);
+        mCm.registerNetworkCallback(ethernetRequest, ethernetCallback);
-//      ethernetCallback.expectAvailableThenValidatedCallbacks(mEthernetAgent);
+        mEthernetAgent.connect(true);
-//      // Wifi is no longer the default, so it should get a callback to LP changed with a PAC
+        ethernetCallback.expectAvailableThenValidatedCallbacks(mEthernetAgent);
-//      // proxy but a port of -1 (since the local proxy doesn't serve wifi now)
+
-//      wifiCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent,
+        // Wifi is no longer the default, so it should get a callback to LP changed with a PAC
-//              lp -> lp.getLp().getHttpProxy().getPort() == -1
+        // proxy but a port of -1 (since the local proxy doesn't serve wifi now)
-//                      && lp.getLp().getHttpProxy().isPacProxy());
+        wifiCallback.expect(LINK_PROPERTIES_CHANGED, mWiFiAgent,
-//      wifiCallback.expect(CallbackEntry.LOSING, mWiFiAgent);
+                lp -> lp.getLp().getHttpProxy().getPort() == -1
-//
+                        && lp.getLp().getHttpProxy().isPacProxy());
-//      final ProxyInfo ethProxy = ProxyInfo.buildPacProxy(ethPacUrl);
+        // Wifi is lingered as it was the default but is no longer serving any request.
-//      final LinkProperties ethLinkProperties = new LinkProperties();
+        wifiCallback.expect(CallbackEntry.LOSING, mWiFiAgent);
-//      ethLinkProperties.setHttpProxy(ethProxy);
+
-//      mEthernetAgent.sendLinkProperties(ethLinkProperties);
+        // Now arrange for Ethernet to have a PAC proxy.
-//      ethernetCallback.expect(CallbackEntry.LINK_PROPERTIES_CHANGED, mEthernetAgent);
+        final ProxyInfo ethProxy = ProxyInfo.buildPacProxy(ethPacUrl);
-//      // Default network is Ethernet
+        final LinkProperties ethLinkProperties = new LinkProperties();
-//      assertEquals(ethProxy, mService.getProxyForNetwork(null));
+        ethLinkProperties.setHttpProxy(ethProxy);
-//      assertEquals(ethProxy, mService.getProxyForNetwork(mEthernetAgent.getNetwork()));
+        mEthernetAgent.sendLinkProperties(ethLinkProperties);
-//      // Proxy info for WiFi ideally should be the old one with the old server still running,
+        ethernetCallback.expect(CallbackEntry.LINK_PROPERTIES_CHANGED, mEthernetAgent);
-//      // but as the PAC code only handles one server at any given time, this can't work. Wifi
+        // Default network is Ethernet
-//      // having null as a proxy also won't work (apps using WiFi will try to access the network
+        assertEquals(ethProxy, mService.getProxyForNetwork(null));
-//      // without proxy) but is not as bad as having the new proxy (that would send requests
+        assertEquals(ethProxy, mService.getProxyForNetwork(mEthernetAgent.getNetwork()));
-//      // over the default network).
+        // Proxy info for WiFi ideally should be the old one with the old server still running,
-//      assertEquals(initialProxyInfo, mService.getProxyForNetwork(mWiFiAgent.getNetwork()));
+        // but as the PAC code only handles one server at any given time, this can't work. Wifi
-//      assertNull(mService.getProxyForNetwork(mCellAgent.getNetwork()));
+        // having null as a proxy also won't work (apps using WiFi will try to access the network
-//
+        // without proxy) but is not as bad as having the new proxy (that would send requests
-//      final ProxyInfo servingEthProxy = new ProxyInfo(pacUrl, 2099);
+        // over the default network).
-//      final ExpectedBroadcast b2 = expectProxyChangeAction(servingEthProxy);
+        assertEquals(initialProxyInfo, mService.getProxyForNetwork(mWiFiAgent.getNetwork()));
-//      final ExpectedBroadcast b3 = expectProxyChangeAction(servingProxyInfo);
+        assertNull(mService.getProxyForNetwork(mCellAgent.getNetwork()));
-//
+
-//      mService.simulateUpdateProxyInfo(mEthernetAgent.getNetwork(), servingEthProxy);
+        // Expect the local PAC proxy server starts to serve the proxy on Ethernet. Use
-//      ethernetCallback.expect(CallbackEntry.LINK_PROPERTIES_CHANGED, mEthernetAgent);
+        // simulateUpdateProxyInfo to simulate this, and check that the callback is called to
-//      assertEquals(servingEthProxy, mService.getProxyForNetwork(null));
+        // inform apps of the port and that the various networks have the expected proxies in
-//      assertEquals(servingEthProxy, mService.getProxyForNetwork(
+        // their link properties.
-//              mEthernetAgent.getNetwork()));
+        final ProxyInfo servingEthProxy = new ProxyInfo(ethPacUrl, 2099);
-//      assertEquals(initialProxyInfo,
+        final ExpectedBroadcast servingEthProxyBroadcast = expectProxyChangeAction(servingEthProxy);
-//              mService.getProxyForNetwork(mWiFiAgent.getNetwork()));
+        final ExpectedBroadcast servingProxyBroadcast2 = expectProxyChangeAction(servingProxyInfo);
-//      assertNull(mService.getProxyForNetwork(mCellAgent.getNetwork()));
+        mService.simulateUpdateProxyInfo(mEthernetAgent.getNetwork(), servingEthProxy);
-//      b2.expectBroadcast();
+        ethernetCallback.expect(CallbackEntry.LINK_PROPERTIES_CHANGED, mEthernetAgent);
-//
+        assertEquals(servingEthProxy, mService.getProxyForNetwork(null));
-//      // Ethernet disconnects, back to WiFi
+        assertEquals(servingEthProxy, mService.getProxyForNetwork(mEthernetAgent.getNetwork()));
-//      mEthernetAgent.disconnect();
+        assertEquals(initialProxyInfo, mService.getProxyForNetwork(mWiFiAgent.getNetwork()));
-//      ethernetCallback.expect(CallbackEntry.LOST, mEthernetAgent);
+        assertNull(mService.getProxyForNetwork(mCellAgent.getNetwork()));
-//      wifiCallback.assertNoCallback();
+        servingEthProxyBroadcast.expectBroadcast();
-//
+
-//      assertEquals(initialProxyInfo, mService.getProxyForNetwork(null));
+        // Ethernet disconnects, back to WiFi
-//      assertEquals(initialProxyInfo,
+        mEthernetAgent.disconnect();
-//              mService.getProxyForNetwork(mWiFiAgent.getNetwork()));
+        ethernetCallback.expect(CallbackEntry.LOST, mEthernetAgent);
-//      assertNull(mService.getProxyForNetwork(mCellAgent.getNetwork()));
+
-//
+        // WiFi is now the default network again. However, the local proxy server does not serve
-//      // After a while the PAC file for wifi is resolved again
+        // WiFi at this time, so at this time a proxy with port -1 is still the correct value.
-//      mService.simulateUpdateProxyInfo(mWiFiAgent.getNetwork(), servingProxyInfo);
+        // In time, the local proxy server for ethernet will be downed and the local proxy server
-//      wifiCallback.expect(CallbackEntry.LINK_PROPERTIES_CHANGED, mWiFiAgent);
+        // for WiFi will be restarted, and WiFi will see an update to its LP with the new port,
-//      assertEquals(servingProxyInfo, mService.getProxyForNetwork(null));
+        // but in this test this won't happen until the test simulates the local proxy starting
-//      assertEquals(servingProxyInfo,
+        // up for WiFi (which is done just a few lines below). This is therefore the perfect place
-//              mService.getProxyForNetwork(mWiFiAgent.getNetwork()));
+        // to check that WiFi is unaffected until the new local proxy starts up.
-//      assertNull(mService.getProxyForNetwork(mCellAgent.getNetwork()));
+        wifiCallback.assertNoCallback();
-//      b3.expectBroadcast();
+        assertEquals(initialProxyInfo, mService.getProxyForNetwork(null));
-//
+        assertEquals(initialProxyInfo, mService.getProxyForNetwork(mWiFiAgent.getNetwork()));
-//      // Expect a broadcast after wifi disconnected. The proxy might be default proxy or an
+        assertNull(mService.getProxyForNetwork(mCellAgent.getNetwork()));
-//      // empty proxy built by buildDirectProxy. See {@link ProxyTracker.sendProxyBroadcast}.
+        // Note : strictly speaking, for correctness here apps should expect a broadcast with a
-//      // Thus here expects a broadcast will be received but does not verify the content of the
+        // port of -1, since that's the current default proxy. The code does not send this
-//      // proxy.
+        // broadcast. In practice though, not sending it is more useful since the new proxy will
-//      final ExpectedBroadcast b4 = expectProxyChangeAction();
+        // start momentarily, so broadcasting and getting all apps to update and retry once now
-//      mWiFiAgent.disconnect();
+        // and again in 250ms is kind of counter-productive, so don't fix this bug.
-//      b4.expectBroadcast();
+
-//      wifiCallback.expect(CallbackEntry.LOST, mWiFiAgent);
+        // After a while the PAC file for wifi is resolved again and the local proxy server
-//      assertNull(mService.getProxyForNetwork(null));
+        // starts up. This should cause a LP event to inform clients of the port to access the
+        // proxy server for wifi.
+        mService.simulateUpdateProxyInfo(mWiFiAgent.getNetwork(), servingProxyInfo);
+        wifiCallback.expect(CallbackEntry.LINK_PROPERTIES_CHANGED, mWiFiAgent);
+        assertEquals(servingProxyInfo, mService.getProxyForNetwork(null));
+        assertEquals(servingProxyInfo, mService.getProxyForNetwork(mWiFiAgent.getNetwork()));
+        assertNull(mService.getProxyForNetwork(mCellAgent.getNetwork()));
+        servingProxyBroadcast2.expectBroadcast();
+
+        // Expect a broadcast for an empty proxy after wifi disconnected, because cell is now
+        // the default network and it doesn't have a proxy. Whether "no proxy" is a null pointer
+        // or a ProxyInfo with an empty host doesn't matter because both are correct, so this test
+        // accepts both.
+        final ExpectedBroadcast emptyProxyBroadcast = expectProxyChangeAction(
+                proxy -> proxy == null || TextUtils.isEmpty(proxy.getHost()));
+        mWiFiAgent.disconnect();
+        emptyProxyBroadcast.expectBroadcast();
+        wifiCallback.expect(CallbackEntry.LOST, mWiFiAgent);
+        assertNull(mService.getProxyForNetwork(null));
         assertNull(mService.getLinkProperties(mCellAgent.getNetwork()).getHttpProxy());
         assertNull(mService.getGlobalProxy());
+
+        mCm.unregisterNetworkCallback(cellCallback);
+    }
+
+    @Test
+    public void testPacProxy_NetworkDisconnects_BroadcastSent() throws Exception {
+        // Make a WiFi network with a PAC URI.
+        final Uri pacUrl = Uri.parse("https://pac-url");
+        final ProxyInfo initialProxyInfo = ProxyInfo.buildPacProxy(pacUrl);
+        final LinkProperties testLinkProperties = new LinkProperties();
+        testLinkProperties.setHttpProxy(initialProxyInfo);
+
+        final TestNetworkCallback wifiCallback = new TestNetworkCallback();
+        final NetworkRequest wifiRequest = new NetworkRequest.Builder()
+                .addTransportType(TRANSPORT_WIFI).build();
+        mCm.registerNetworkCallback(wifiRequest, wifiCallback);
+
+        mWiFiAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, testLinkProperties);
+        mWiFiAgent.connect(true);
+        // Wifi is up, but the local proxy server hasn't started yet.
+        wifiCallback.expectAvailableThenValidatedCallbacks(mWiFiAgent);
+
+        // Simulate PacManager sending the notification that the local server has started
+        final ProxyInfo servingProxyInfo = new ProxyInfo(pacUrl, 2097);
+        final ExpectedBroadcast servingProxyBroadcast = expectProxyChangeAction(servingProxyInfo);
+        mService.simulateUpdateProxyInfo(mWiFiAgent.getNetwork(), servingProxyInfo);
+        wifiCallback.expect(CallbackEntry.LINK_PROPERTIES_CHANGED, mWiFiAgent);
+        servingProxyBroadcast.expectBroadcast();
+
+        // Now disconnect Wi-Fi and make sure there is a broadcast for some empty proxy. Whether
+        // the "empty" proxy is a null pointer or a ProxyInfo with an empty host doesn't matter
+        // because both are correct, so this test accepts both.
+        final ExpectedBroadcast emptyProxyBroadcast = expectProxyChangeAction(
+                proxy -> proxy == null || TextUtils.isEmpty(proxy.getHost()));
+        mWiFiAgent.disconnect();
+        wifiCallback.expect(CallbackEntry.LOST, mWiFiAgent);
+        emptyProxyBroadcast.expectBroadcast();
     }
 
     @Test