Merge "Refactor setCurrentProxyScriptUrl to a void method" am: 361e0a91a6 am: a344012b59

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1513131

MUST ONLY BE SUBMITTED BY AUTOMERGER

Change-Id: Ibe8db1aa61ad9aea6719531634ef8a5d97197d8e
This commit is contained in:
Aaron Huang
2021-01-15 11:12:03 +00:00
committed by Automerger Merge Worker
2 changed files with 50 additions and 37 deletions

View File

@@ -16,6 +16,7 @@
package com.android.server.connectivity; package com.android.server.connectivity;
import android.annotation.NonNull;
import android.annotation.WorkerThread; import android.annotation.WorkerThread;
import android.app.AlarmManager; import android.app.AlarmManager;
import android.app.PendingIntent; import android.app.PendingIntent;
@@ -71,10 +72,6 @@ public class PacProxyInstaller {
private static final int DELAY_LONG = 4; private static final int DELAY_LONG = 4;
private static final long MAX_PAC_SIZE = 20 * 1000 * 1000; private static final long MAX_PAC_SIZE = 20 * 1000 * 1000;
// Return values for #setCurrentProxyScriptUrl
public static final boolean DONT_SEND_BROADCAST = false;
public static final boolean DO_SEND_BROADCAST = true;
private String mCurrentPac; private String mCurrentPac;
@GuardedBy("mProxyLock") @GuardedBy("mProxyLock")
private volatile Uri mPacUrl = Uri.EMPTY; private volatile Uri mPacUrl = Uri.EMPTY;
@@ -93,7 +90,7 @@ public class PacProxyInstaller {
private volatile boolean mHasSentBroadcast; private volatile boolean mHasSentBroadcast;
private volatile boolean mHasDownloaded; private volatile boolean mHasDownloaded;
private Handler mConnectivityHandler; private final Handler mConnectivityHandler;
private final int mProxyMessage; private final int mProxyMessage;
/** /**
@@ -101,6 +98,13 @@ public class PacProxyInstaller {
*/ */
private final Object mProxyLock = new Object(); private final Object mProxyLock = new Object();
/**
* Lock ensuring consistency between the values of mHasSentBroadcast, mHasDownloaded, the
* last URL and port, and the broadcast message being sent with the correct arguments.
* TODO : this should probably protect all instances of these variables
*/
private final Object mBroadcastStateLock = new Object();
/** /**
* Runnable to download PAC script. * Runnable to download PAC script.
* The behavior relies on the assumption it always runs on mNetThread to guarantee that the * The behavior relies on the assumption it always runs on mNetThread to guarantee that the
@@ -146,7 +150,7 @@ public class PacProxyInstaller {
} }
} }
public PacProxyInstaller(Context context, Handler handler, int proxyMessage) { public PacProxyInstaller(@NonNull Context context, @NonNull Handler handler, int proxyMessage) {
mContext = context; mContext = context;
mLastPort = -1; mLastPort = -1;
final HandlerThread netThread = new HandlerThread("android.pacproxyinstaller", final HandlerThread netThread = new HandlerThread("android.pacproxyinstaller",
@@ -176,31 +180,27 @@ public class PacProxyInstaller {
* PacProxyInstaller will trigger a new broadcast when it is ready. * PacProxyInstaller will trigger a new broadcast when it is ready.
* *
* @param proxy Proxy information that is about to be broadcast. * @param proxy Proxy information that is about to be broadcast.
* @return Returns whether the broadcast should be sent : either DO_ or DONT_SEND_BROADCAST
*/ */
public synchronized boolean setCurrentProxyScriptUrl(ProxyInfo proxy) { public void setCurrentProxyScriptUrl(@NonNull ProxyInfo proxy) {
if (!Uri.EMPTY.equals(proxy.getPacFileUrl())) { synchronized (mBroadcastStateLock) {
if (proxy.getPacFileUrl().equals(mPacUrl) && (proxy.getPort() > 0)) { if (!Uri.EMPTY.equals(proxy.getPacFileUrl())) {
// Allow to send broadcast, nothing to do. if (proxy.getPacFileUrl().equals(mPacUrl) && (proxy.getPort() > 0)) return;
return DO_SEND_BROADCAST; mPacUrl = proxy.getPacFileUrl();
} mCurrentDelay = DELAY_1;
mPacUrl = proxy.getPacFileUrl(); mHasSentBroadcast = false;
mCurrentDelay = DELAY_1; mHasDownloaded = false;
mHasSentBroadcast = false; getAlarmManager().cancel(mPacRefreshIntent);
mHasDownloaded = false; bind();
getAlarmManager().cancel(mPacRefreshIntent); } else {
bind(); getAlarmManager().cancel(mPacRefreshIntent);
return DONT_SEND_BROADCAST; synchronized (mProxyLock) {
} else { mPacUrl = Uri.EMPTY;
getAlarmManager().cancel(mPacRefreshIntent); mCurrentPac = null;
synchronized (mProxyLock) { if (mProxyService != null) {
mPacUrl = Uri.EMPTY; unbind();
mCurrentPac = null; }
if (mProxyService != null) {
unbind();
} }
} }
return DO_SEND_BROADCAST;
} }
} }
@@ -275,6 +275,7 @@ public class PacProxyInstaller {
getAlarmManager().set(AlarmManager.ELAPSED_REALTIME, timeTillTrigger, mPacRefreshIntent); getAlarmManager().set(AlarmManager.ELAPSED_REALTIME, timeTillTrigger, mPacRefreshIntent);
} }
@GuardedBy("mProxyLock")
private void setCurrentProxyScript(String script) { private void setCurrentProxyScript(String script) {
if (mProxyService == null) { if (mProxyService == null) {
Log.e(TAG, "setCurrentProxyScript: no proxy service"); Log.e(TAG, "setCurrentProxyScript: no proxy service");
@@ -347,6 +348,9 @@ public class PacProxyInstaller {
public void setProxyPort(int port) { public void setProxyPort(int port) {
if (mLastPort != -1) { if (mLastPort != -1) {
// Always need to send if port changed // Always need to send if port changed
// TODO: Here lacks synchronization because this write cannot
// guarantee that it's visible from sendProxyIfNeeded() when
// it's called by a Runnable which is post by mNetThread.
mHasSentBroadcast = false; mHasSentBroadcast = false;
} }
mLastPort = port; mLastPort = port;
@@ -386,13 +390,15 @@ public class PacProxyInstaller {
mConnectivityHandler.sendMessage(mConnectivityHandler.obtainMessage(mProxyMessage, proxy)); mConnectivityHandler.sendMessage(mConnectivityHandler.obtainMessage(mProxyMessage, proxy));
} }
private synchronized void sendProxyIfNeeded() { private void sendProxyIfNeeded() {
if (!mHasDownloaded || (mLastPort == -1)) { synchronized (mBroadcastStateLock) {
return; if (!mHasDownloaded || (mLastPort == -1)) {
} return;
if (!mHasSentBroadcast) { }
sendPacBroadcast(ProxyInfo.buildPacProxy(mPacUrl, mLastPort)); if (!mHasSentBroadcast) {
mHasSentBroadcast = true; sendPacBroadcast(ProxyInfo.buildPacProxy(mPacUrl, mLastPort));
mHasSentBroadcast = true;
}
} }
} }
} }

View File

@@ -226,9 +226,9 @@ public class ProxyTracker {
final ProxyInfo defaultProxy = getDefaultProxy(); final ProxyInfo defaultProxy = getDefaultProxy();
final ProxyInfo proxyInfo = null != defaultProxy ? final ProxyInfo proxyInfo = null != defaultProxy ?
defaultProxy : ProxyInfo.buildDirectProxy("", 0, Collections.emptyList()); defaultProxy : ProxyInfo.buildDirectProxy("", 0, Collections.emptyList());
mPacProxyInstaller.setCurrentProxyScriptUrl(proxyInfo);
if (mPacProxyInstaller.setCurrentProxyScriptUrl(proxyInfo) if (!shouldSendBroadcast(proxyInfo)) {
== PacProxyInstaller.DONT_SEND_BROADCAST) {
return; return;
} }
if (DBG) Log.d(TAG, "sending Proxy Broadcast for " + proxyInfo); if (DBG) Log.d(TAG, "sending Proxy Broadcast for " + proxyInfo);
@@ -244,6 +244,13 @@ public class ProxyTracker {
} }
} }
private boolean shouldSendBroadcast(ProxyInfo proxy) {
if (Uri.EMPTY.equals(proxy.getPacFileUrl())) return false;
if (proxy.getPacFileUrl().equals(proxy.getPacFileUrl())
&& (proxy.getPort() > 0)) return true;
return true;
}
/** /**
* Sets the global proxy in memory. Also writes the values to the global settings of the device. * Sets the global proxy in memory. Also writes the values to the global settings of the device.
* *