From e18083062eed92a39e777c277722d75ee5c308c9 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 7 Jun 2018 19:40:24 +0900 Subject: [PATCH 1/3] [PT07] Small cleanup of setGlobalProxy Test: runtest Change-Id: I4825e6326f7ce7bd45d625d57824f8d27c51d6f2 --- .../server/connectivity/ProxyTracker.java | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/ProxyTracker.java b/services/core/java/com/android/server/connectivity/ProxyTracker.java index daaedd8131..37221663db 100644 --- a/services/core/java/com/android/server/connectivity/ProxyTracker.java +++ b/services/core/java/com/android/server/connectivity/ProxyTracker.java @@ -139,14 +139,15 @@ public class ProxyTracker { public void setGlobalProxy(@Nullable ProxyInfo proxyProperties) { synchronized (mProxyLock) { + // ProxyInfo#equals is not commutative :( and is public API, so it can't be fixed. if (proxyProperties == mGlobalProxy) return; if (proxyProperties != null && proxyProperties.equals(mGlobalProxy)) return; if (mGlobalProxy != null && mGlobalProxy.equals(proxyProperties)) return; - String host = ""; - int port = 0; - String exclList = ""; - String pacFileUrl = ""; + final String host; + final int port; + final String exclList; + final String pacFileUrl; if (proxyProperties != null && (!TextUtils.isEmpty(proxyProperties.getHost()) || !Uri.EMPTY.equals(proxyProperties.getPacFileUrl()))) { if (!proxyProperties.isValid()) { @@ -157,10 +158,13 @@ public class ProxyTracker { host = mGlobalProxy.getHost(); port = mGlobalProxy.getPort(); exclList = mGlobalProxy.getExclusionListAsString(); - if (!Uri.EMPTY.equals(proxyProperties.getPacFileUrl())) { - pacFileUrl = proxyProperties.getPacFileUrl().toString(); - } + pacFileUrl = Uri.EMPTY.equals(proxyProperties.getPacFileUrl()) + ? "" : proxyProperties.getPacFileUrl().toString(); } else { + host = ""; + port = 0; + exclList = ""; + pacFileUrl = ""; mGlobalProxy = null; } final ContentResolver res = mContext.getContentResolver(); @@ -175,10 +179,7 @@ public class ProxyTracker { Binder.restoreCallingIdentity(token); } - if (mGlobalProxy == null) { - proxyProperties = mDefaultProxy; - } - sendProxyBroadcast(proxyProperties); + sendProxyBroadcast(mGlobalProxy == null ? mDefaultProxy : proxyProperties); } } } From d9e70ac17eff068ccc6abb97d38ecfd6c33dbbcd Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 8 Jun 2018 12:20:15 +0900 Subject: [PATCH 2/3] [PT08] Move setDefaultProxy to ProxyTracker Test: runtest Change-Id: Idb7d2f2895aac63d54e3a6481379b739a726eff6 --- .../android/server/ConnectivityService.java | 30 +--------------- .../server/connectivity/ProxyTracker.java | 34 ++++++++++++++++++- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index dadd3274ee..69b2bbf207 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3334,35 +3334,7 @@ public class ConnectivityService extends IConnectivityManager.Stub && Uri.EMPTY.equals(proxy.getPacFileUrl())) { proxy = null; } - synchronized (mProxyTracker.mProxyLock) { - if (mProxyTracker.mDefaultProxy != null && mProxyTracker.mDefaultProxy.equals(proxy)) { - return; - } - if (mProxyTracker.mDefaultProxy == proxy) return; // catches repeated nulls - if (proxy != null && !proxy.isValid()) { - if (DBG) log("Invalid proxy properties, ignoring: " + proxy.toString()); - return; - } - - // This call could be coming from the PacManager, containing the port of the local - // proxy. If this new proxy matches the global proxy then copy this proxy to the - // global (to get the correct local port), and send a broadcast. - // TODO: Switch PacManager to have its own message to send back rather than - // reusing EVENT_HAS_CHANGED_PROXY and this call to handleApplyDefaultProxy. - if ((mProxyTracker.mGlobalProxy != null) && (proxy != null) - && (!Uri.EMPTY.equals(proxy.getPacFileUrl())) - && proxy.getPacFileUrl().equals(mProxyTracker.mGlobalProxy.getPacFileUrl())) { - mProxyTracker.mGlobalProxy = proxy; - mProxyTracker.sendProxyBroadcast(mProxyTracker.mGlobalProxy); - return; - } - mProxyTracker.mDefaultProxy = proxy; - - if (mProxyTracker.mGlobalProxy != null) return; - if (!mProxyTracker.mDefaultProxyDisabled) { - mProxyTracker.sendProxyBroadcast(proxy); - } - } + mProxyTracker.setDefaultProxy(proxy); } // If the proxy has changed from oldLp to newLp, resend proxy broadcast with default proxy. diff --git a/services/core/java/com/android/server/connectivity/ProxyTracker.java b/services/core/java/com/android/server/connectivity/ProxyTracker.java index 37221663db..98c161dbba 100644 --- a/services/core/java/com/android/server/connectivity/ProxyTracker.java +++ b/services/core/java/com/android/server/connectivity/ProxyTracker.java @@ -120,7 +120,7 @@ public class ProxyTracker { return mPacManager.setCurrentProxyScriptUrl(proxy); } - // TODO : make the argument NonNull final + // TODO : make the argument NonNull final and the method private public void sendProxyBroadcast(@Nullable ProxyInfo proxy) { if (proxy == null) proxy = new ProxyInfo("", 0, ""); if (setCurrentProxyScriptUrl(proxy)) return; @@ -182,4 +182,36 @@ public class ProxyTracker { sendProxyBroadcast(mGlobalProxy == null ? mDefaultProxy : proxyProperties); } } + + public void setDefaultProxy(@Nullable ProxyInfo proxy) { + synchronized (mProxyLock) { + if (mDefaultProxy != null && mDefaultProxy.equals(proxy)) { + return; + } + if (mDefaultProxy == proxy) return; // catches repeated nulls + if (proxy != null && !proxy.isValid()) { + if (DBG) Slog.d(TAG, "Invalid proxy properties, ignoring: " + proxy); + return; + } + + // This call could be coming from the PacManager, containing the port of the local + // proxy. If this new proxy matches the global proxy then copy this proxy to the + // global (to get the correct local port), and send a broadcast. + // TODO: Switch PacManager to have its own message to send back rather than + // reusing EVENT_HAS_CHANGED_PROXY and this call to handleApplyDefaultProxy. + if ((mGlobalProxy != null) && (proxy != null) + && (!Uri.EMPTY.equals(proxy.getPacFileUrl())) + && proxy.getPacFileUrl().equals(mGlobalProxy.getPacFileUrl())) { + mGlobalProxy = proxy; + sendProxyBroadcast(mGlobalProxy); + return; + } + mDefaultProxy = proxy; + + if (mGlobalProxy != null) return; + if (!mDefaultProxyDisabled) { + sendProxyBroadcast(proxy); + } + } + } } From fe357814b4b0959957c003d5f1f07dd463912797 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 8 Jun 2018 12:41:21 +0900 Subject: [PATCH 3/3] [PT09] Add javadoc comments to all ProxyTracker methods/members. Also rename some vars and inline a function that is now private. Test: runtest Change-Id: I4b1bca8b29f46d97056973cd38ed8effc3f5b591 --- .../server/connectivity/ProxyTracker.java | 103 ++++++++++++------ 1 file changed, 71 insertions(+), 32 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/ProxyTracker.java b/services/core/java/com/android/server/connectivity/ProxyTracker.java index 98c161dbba..dc65e1e698 100644 --- a/services/core/java/com/android/server/connectivity/ProxyTracker.java +++ b/services/core/java/com/android/server/connectivity/ProxyTracker.java @@ -51,15 +51,25 @@ public class ProxyTracker { // possible @NonNull public final Object mProxyLock = new Object(); + // The global proxy is the proxy that is set device-wide, overriding any network-specific + // proxy. Note however that proxies are hints ; the system does not enforce their use. Hence + // this value is only for querying. @Nullable @GuardedBy("mProxyLock") public ProxyInfo mGlobalProxy = null; + // The default proxy is the proxy that applies to no particular network if the global proxy + // is not set. Individual networks have their own settings that override this. This member + // is set through setDefaultProxy, which is called when the default network changes proxies + // in its LinkProperties, or when ConnectivityService switches to a new default network, or + // when PacManager resolves the proxy. @Nullable @GuardedBy("mProxyLock") public volatile ProxyInfo mDefaultProxy = null; + // Whether the default proxy is disabled. TODO : make this mDefaultProxyEnabled @GuardedBy("mProxyLock") public boolean mDefaultProxyDisabled = false; + // The object responsible for Proxy Auto Configuration (PAC). @NonNull private final PacManager mPacManager; @@ -98,6 +108,16 @@ public class ProxyTracker { return Objects.equals(pa, pb) && (pa == null || Objects.equals(pa.getHost(), pb.getHost())); } + /** + * Gets the default system-wide proxy. + * + * This will return the global proxy if set, otherwise the default proxy if in use. Note + * that this is not necessarily the proxy that any given process should use, as the right + * proxy for a process is the proxy for the network this process will use, which may be + * different from this value. This value is simply the default in case there is no proxy set + * in the network that will be used by a specific process. + * @return The default system-wide proxy or null if none. + */ @Nullable public ProxyInfo getDefaultProxy() { // This information is already available as a world read/writable jvm property. @@ -108,6 +128,11 @@ public class ProxyTracker { } } + /** + * Gets the global proxy. + * + * @return The global proxy or null if none. + */ @Nullable public ProxyInfo getGlobalProxy() { // This information is already available as a world read/writable jvm property. @@ -116,19 +141,22 @@ public class ProxyTracker { } } - public boolean setCurrentProxyScriptUrl(@NonNull final ProxyInfo proxy) { - return mPacManager.setCurrentProxyScriptUrl(proxy); - } - + /** + * Sends the system broadcast informing apps about a new proxy configuration. + * + * Confusingly this method also sets the PAC file URL. TODO : separate this, it has nothing + * to do in a "sendProxyBroadcast" method. + * @param proxyInfo the proxy spec, or null for no proxy. + */ // TODO : make the argument NonNull final and the method private - public void sendProxyBroadcast(@Nullable ProxyInfo proxy) { - if (proxy == null) proxy = new ProxyInfo("", 0, ""); - if (setCurrentProxyScriptUrl(proxy)) return; - if (DBG) Slog.d(TAG, "sending Proxy Broadcast for " + proxy); + public void sendProxyBroadcast(@Nullable ProxyInfo proxyInfo) { + if (proxyInfo == null) proxyInfo = new ProxyInfo("", 0, ""); + if (mPacManager.setCurrentProxyScriptUrl(proxyInfo)) return; + if (DBG) Slog.d(TAG, "sending Proxy Broadcast for " + proxyInfo); Intent intent = new Intent(Proxy.PROXY_CHANGE_ACTION); intent.addFlags(Intent.FLAG_RECEIVER_REPLACE_PENDING | Intent.FLAG_RECEIVER_REGISTERED_ONLY_BEFORE_BOOT); - intent.putExtra(Proxy.EXTRA_PROXY_INFO, proxy); + intent.putExtra(Proxy.EXTRA_PROXY_INFO, proxyInfo); final long ident = Binder.clearCallingIdentity(); try { mContext.sendStickyBroadcastAsUser(intent, UserHandle.ALL); @@ -137,29 +165,34 @@ public class ProxyTracker { } } - public void setGlobalProxy(@Nullable ProxyInfo proxyProperties) { + /** + * Sets the global proxy in memory. Also writes the values to the global settings of the device. + * + * @param proxyInfo the proxy spec, or null for no proxy. + */ + public void setGlobalProxy(@Nullable ProxyInfo proxyInfo) { synchronized (mProxyLock) { // ProxyInfo#equals is not commutative :( and is public API, so it can't be fixed. - if (proxyProperties == mGlobalProxy) return; - if (proxyProperties != null && proxyProperties.equals(mGlobalProxy)) return; - if (mGlobalProxy != null && mGlobalProxy.equals(proxyProperties)) return; + if (proxyInfo == mGlobalProxy) return; + if (proxyInfo != null && proxyInfo.equals(mGlobalProxy)) return; + if (mGlobalProxy != null && mGlobalProxy.equals(proxyInfo)) return; final String host; final int port; final String exclList; final String pacFileUrl; - if (proxyProperties != null && (!TextUtils.isEmpty(proxyProperties.getHost()) || - !Uri.EMPTY.equals(proxyProperties.getPacFileUrl()))) { - if (!proxyProperties.isValid()) { - if (DBG) Slog.d(TAG, "Invalid proxy properties, ignoring: " + proxyProperties); + if (proxyInfo != null && (!TextUtils.isEmpty(proxyInfo.getHost()) || + !Uri.EMPTY.equals(proxyInfo.getPacFileUrl()))) { + if (!proxyInfo.isValid()) { + if (DBG) Slog.d(TAG, "Invalid proxy properties, ignoring: " + proxyInfo); return; } - mGlobalProxy = new ProxyInfo(proxyProperties); + mGlobalProxy = new ProxyInfo(proxyInfo); host = mGlobalProxy.getHost(); port = mGlobalProxy.getPort(); exclList = mGlobalProxy.getExclusionListAsString(); - pacFileUrl = Uri.EMPTY.equals(proxyProperties.getPacFileUrl()) - ? "" : proxyProperties.getPacFileUrl().toString(); + pacFileUrl = Uri.EMPTY.equals(proxyInfo.getPacFileUrl()) + ? "" : proxyInfo.getPacFileUrl().toString(); } else { host = ""; port = 0; @@ -179,18 +212,24 @@ public class ProxyTracker { Binder.restoreCallingIdentity(token); } - sendProxyBroadcast(mGlobalProxy == null ? mDefaultProxy : proxyProperties); + sendProxyBroadcast(mGlobalProxy == null ? mDefaultProxy : proxyInfo); } } - public void setDefaultProxy(@Nullable ProxyInfo proxy) { + /** + * Sets the default proxy for the device. + * + * The default proxy is the proxy used for networks that do not have a specific proxy. + * @param proxyInfo the proxy spec, or null for no proxy. + */ + public void setDefaultProxy(@Nullable ProxyInfo proxyInfo) { synchronized (mProxyLock) { - if (mDefaultProxy != null && mDefaultProxy.equals(proxy)) { + if (mDefaultProxy != null && mDefaultProxy.equals(proxyInfo)) { return; } - if (mDefaultProxy == proxy) return; // catches repeated nulls - if (proxy != null && !proxy.isValid()) { - if (DBG) Slog.d(TAG, "Invalid proxy properties, ignoring: " + proxy); + if (mDefaultProxy == proxyInfo) return; // catches repeated nulls + if (proxyInfo != null && !proxyInfo.isValid()) { + if (DBG) Slog.d(TAG, "Invalid proxy properties, ignoring: " + proxyInfo); return; } @@ -199,18 +238,18 @@ public class ProxyTracker { // global (to get the correct local port), and send a broadcast. // TODO: Switch PacManager to have its own message to send back rather than // reusing EVENT_HAS_CHANGED_PROXY and this call to handleApplyDefaultProxy. - if ((mGlobalProxy != null) && (proxy != null) - && (!Uri.EMPTY.equals(proxy.getPacFileUrl())) - && proxy.getPacFileUrl().equals(mGlobalProxy.getPacFileUrl())) { - mGlobalProxy = proxy; + if ((mGlobalProxy != null) && (proxyInfo != null) + && (!Uri.EMPTY.equals(proxyInfo.getPacFileUrl())) + && proxyInfo.getPacFileUrl().equals(mGlobalProxy.getPacFileUrl())) { + mGlobalProxy = proxyInfo; sendProxyBroadcast(mGlobalProxy); return; } - mDefaultProxy = proxy; + mDefaultProxy = proxyInfo; if (mGlobalProxy != null) return; if (!mDefaultProxyDisabled) { - sendProxyBroadcast(proxy); + sendProxyBroadcast(proxyInfo); } } }