From 110cb12040f1972062a6cff86e29c05b284d60b3 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 8 Jun 2018 19:46:44 +0900 Subject: [PATCH] [PT16] Simplification of sendProxyBroadcast. sendProxyBroadcast is always called with the same argument, and it would make no sense with another argument anyway. Remove it. This concludes the ProxyTracker refactoring with 227 lines removed from ConnectivityService, a lot clarified, and some bugs removed. Things can still be improved, but presumably at a much higher cost. Next steps are : write tests, now that ProxyTracker is both testable and mockable. And try to pour some gasoline on the PROXY_CHANGE_ACTION broadcast, see if it burns well. Test: runtest Change-Id: I66e40b4bf5cfd0b2dc4fa37ea97b3429fe1b7e6c --- .../com/android/server/ConnectivityService.java | 2 +- .../android/server/connectivity/ProxyTracker.java | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 4b77c69aba..d9394e810e 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3478,7 +3478,7 @@ public class ConnectivityService extends IConnectivityManager.Stub ProxyInfo oldProxyInfo = oldLp == null ? null : oldLp.getHttpProxy(); if (!ProxyTracker.proxyInfoEqual(newProxyInfo, oldProxyInfo)) { - mProxyTracker.sendProxyBroadcast(mProxyTracker.getDefaultProxy()); + mProxyTracker.sendProxyBroadcast(); } } diff --git a/services/core/java/com/android/server/connectivity/ProxyTracker.java b/services/core/java/com/android/server/connectivity/ProxyTracker.java index de8ae35226..15468ff05c 100644 --- a/services/core/java/com/android/server/connectivity/ProxyTracker.java +++ b/services/core/java/com/android/server/connectivity/ProxyTracker.java @@ -204,11 +204,10 @@ public class ProxyTracker { * * 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 proxyInfo) { - if (proxyInfo == null) proxyInfo = new ProxyInfo("", 0, ""); + public void sendProxyBroadcast() { + final ProxyInfo defaultProxy = getDefaultProxy(); + final ProxyInfo proxyInfo = null != defaultProxy ? defaultProxy : 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); @@ -269,7 +268,7 @@ public class ProxyTracker { Binder.restoreCallingIdentity(token); } - sendProxyBroadcast(getDefaultProxy()); + sendProxyBroadcast(); } } @@ -296,14 +295,14 @@ public class ProxyTracker { && (!Uri.EMPTY.equals(proxyInfo.getPacFileUrl())) && proxyInfo.getPacFileUrl().equals(mGlobalProxy.getPacFileUrl())) { mGlobalProxy = proxyInfo; - sendProxyBroadcast(getDefaultProxy()); + sendProxyBroadcast(); return; } mDefaultProxy = proxyInfo; if (mGlobalProxy != null) return; if (mDefaultProxyEnabled) { - sendProxyBroadcast(getDefaultProxy()); + sendProxyBroadcast(); } } } @@ -320,7 +319,7 @@ public class ProxyTracker { if (mDefaultProxyEnabled != enabled) { mDefaultProxyEnabled = enabled; if (mGlobalProxy == null && mDefaultProxy != null) { - sendProxyBroadcast(getDefaultProxy()); + sendProxyBroadcast(); } } }