Merge "Readability improvements to the Vpn class with no logic changes"

This commit is contained in:
Chalard Jean
2018-01-12 05:11:29 +00:00
committed by Gerrit Code Review

View File

@@ -128,7 +128,7 @@ public class Vpn {
// Length of time (in milliseconds) that an app hosting an always-on VPN is placed on
// the device idle whitelist during service launch and VPN bootstrap.
private static final long VPN_LAUNCH_IDLE_WHITELIST_DURATION = 60 * 1000;
private static final long VPN_LAUNCH_IDLE_WHITELIST_DURATION_MS = 60 * 1000;
// TODO: create separate trackers for each unique VPN to support
// automated reconnection
@@ -183,10 +183,10 @@ public class Vpn {
@GuardedBy("this")
private Set<UidRange> mBlockedUsers = new ArraySet<>();
// Handle of user initiating VPN.
// Handle of the user initiating VPN.
private final int mUserHandle;
// Listen to package remove and change event in this user
// Listen to package removal and change events (update/uninstall) for this user
private final BroadcastReceiver mPackageIntentReceiver = new BroadcastReceiver() {
@Override
public void onReceive(Context context, Intent intent) {
@@ -197,14 +197,14 @@ public class Vpn {
}
synchronized (Vpn.this) {
// Avoid race that always-on package has been unset
// Avoid race where always-on package has been unset
if (!packageName.equals(getAlwaysOnPackage())) {
return;
}
final String action = intent.getAction();
Log.i(TAG, "Received broadcast " + action + " for always-on package " + packageName
+ " in user " + mUserHandle);
Log.i(TAG, "Received broadcast " + action + " for always-on VPN package "
+ packageName + " in user " + mUserHandle);
switch(action) {
case Intent.ACTION_PACKAGE_REPLACED:
@@ -248,7 +248,8 @@ public class Vpn {
Log.wtf(TAG, "Problem registering observer", e);
}
mNetworkInfo = new NetworkInfo(ConnectivityManager.TYPE_VPN, 0, NETWORKTYPE, "");
mNetworkInfo = new NetworkInfo(ConnectivityManager.TYPE_VPN, 0 /* subtype */, NETWORKTYPE,
"" /* subtypeName */);
mNetworkCapabilities = new NetworkCapabilities();
mNetworkCapabilities.addTransportType(NetworkCapabilities.TRANSPORT_VPN);
mNetworkCapabilities.removeCapability(NetworkCapabilities.NET_CAPABILITY_NOT_VPN);
@@ -258,7 +259,7 @@ public class Vpn {
}
/**
* Set if this object is responsible for watching for {@link NetworkInfo}
* Set whether this object is responsible for watching for {@link NetworkInfo}
* teardown. When {@code false}, teardown is handled externally by someone
* else.
*/
@@ -481,7 +482,6 @@ public class Vpn {
}
private void unregisterPackageChangeReceiverLocked() {
// register previous intent filter
if (mIsPackageIntentReceiverRegistered) {
mContext.unregisterReceiver(mPackageIntentReceiver);
mIsPackageIntentReceiverRegistered = false;
@@ -582,7 +582,7 @@ public class Vpn {
DeviceIdleController.LocalService idleController =
LocalServices.getService(DeviceIdleController.LocalService.class);
idleController.addPowerSaveTempWhitelistApp(Process.myUid(), alwaysOnPackage,
VPN_LAUNCH_IDLE_WHITELIST_DURATION, mUserHandle, false, "vpn");
VPN_LAUNCH_IDLE_WHITELIST_DURATION_MS, mUserHandle, false, "vpn");
// Start the VPN service declared in the app's manifest.
Intent serviceIntent = new Intent(VpnConfig.SERVICE_INTERFACE);
@@ -612,9 +612,10 @@ public class Vpn {
* It uses {@link VpnConfig#LEGACY_VPN} as its package name, and
* it can be revoked by itself.
*
* Note: when we added VPN pre-consent in http://ag/522961 the names oldPackage
* and newPackage become misleading, because when an app is pre-consented, we
* actually prepare oldPackage, not newPackage.
* Note: when we added VPN pre-consent in
* https://android.googlesource.com/platform/frameworks/base/+/0554260
* the names oldPackage and newPackage became misleading, because when
* an app is pre-consented, we actually prepare oldPackage, not newPackage.
*
* Their meanings actually are:
*
@@ -630,7 +631,7 @@ public class Vpn {
* @param oldPackage The package name of the old VPN application
* @param newPackage The package name of the new VPN application
*
* @return true if the operation is succeeded.
* @return true if the operation succeeded.
*/
public synchronized boolean prepare(String oldPackage, String newPackage) {
if (oldPackage != null) {
@@ -639,7 +640,7 @@ public class Vpn {
return false;
}
// Package is not same or old package was reinstalled.
// Package is not the same or old package was reinstalled.
if (!isCurrentPreparedPackage(oldPackage)) {
// The package doesn't match. We return false (to obtain user consent) unless the
// user has already consented to that VPN package.
@@ -861,8 +862,8 @@ public class Vpn {
long token = Binder.clearCallingIdentity();
try {
mNetworkAgent = new NetworkAgent(mLooper, mContext, NETWORKTYPE,
mNetworkInfo, mNetworkCapabilities, lp, 0, networkMisc) {
mNetworkAgent = new NetworkAgent(mLooper, mContext, NETWORKTYPE /* logtag */,
mNetworkInfo, mNetworkCapabilities, lp, 0 /* score */, networkMisc) {
@Override
public void unwanted() {
// We are user controlled, not driven by NetworkRequest.
@@ -936,7 +937,7 @@ public class Vpn {
}
ResolveInfo info = AppGlobals.getPackageManager().resolveService(intent,
null, 0, mUserHandle);
null, 0, mUserHandle);
if (info == null) {
throw new SecurityException("Cannot find " + config.user);
}
@@ -944,7 +945,7 @@ public class Vpn {
throw new SecurityException(config.user + " does not require " + BIND_VPN_SERVICE);
}
} catch (RemoteException e) {
throw new SecurityException("Cannot find " + config.user);
throw new SecurityException("Cannot find " + config.user);
} finally {
Binder.restoreCallingIdentity(token);
}
@@ -1337,7 +1338,7 @@ public class Vpn {
}
private void enforceControlPermissionOrInternalCaller() {
// Require caller to be either an application with CONTROL_VPN permission or a process
// Require the caller to be either an application with CONTROL_VPN permission or a process
// in the system server.
mContext.enforceCallingOrSelfPermission(Manifest.permission.CONTROL_VPN,
"Unauthorized Caller");
@@ -1417,7 +1418,7 @@ public class Vpn {
}
/**
* This method should only be called by ConnectivityService. Because it doesn't
* This method should only be called by ConnectivityService because it doesn't
* have enough data to fill VpnInfo.primaryUnderlyingIface field.
*/
public synchronized VpnInfo getVpnInfo() {
@@ -1768,7 +1769,7 @@ public class Vpn {
* Bringing up a VPN connection takes time, and that is all this thread
* does. Here we have plenty of time. The only thing we need to take
* care of is responding to interruptions as soon as possible. Otherwise
* requests will be piled up. This can be done in a Handler as a state
* requests will pile up. This could be done in a Handler as a state
* machine, but it is much easier to read in the current form.
*/
private class LegacyVpnRunner extends Thread {
@@ -1781,7 +1782,7 @@ public class Vpn {
private final AtomicInteger mOuterConnection =
new AtomicInteger(ConnectivityManager.TYPE_NONE);
private long mTimer = -1;
private long mBringupStartTime = -1;
/**
* Watch for the outer connection (passing in the constructor) going away.
@@ -1861,8 +1862,8 @@ public class Vpn {
synchronized (TAG) {
Log.v(TAG, "Executing");
try {
execute();
monitorDaemons();
bringup();
waitForDaemonsToStop();
interrupted(); // Clear interrupt flag if execute called exit.
} catch (InterruptedException e) {
} finally {
@@ -1883,30 +1884,27 @@ public class Vpn {
}
}
private void checkpoint(boolean yield) throws InterruptedException {
private void checkInterruptAndDelay(boolean sleepLonger) throws InterruptedException {
long now = SystemClock.elapsedRealtime();
if (mTimer == -1) {
mTimer = now;
Thread.sleep(1);
} else if (now - mTimer <= 60000) {
Thread.sleep(yield ? 200 : 1);
if (now - mBringupStartTime <= 60000) {
Thread.sleep(sleepLonger ? 200 : 1);
} else {
updateState(DetailedState.FAILED, "checkpoint");
throw new IllegalStateException("Time is up");
throw new IllegalStateException("VPN bringup took too long");
}
}
private void execute() {
// Catch all exceptions so we can clean up few things.
private void bringup() {
// Catch all exceptions so we can clean up a few things.
boolean initFinished = false;
try {
// Initialize the timer.
checkpoint(false);
mBringupStartTime = SystemClock.elapsedRealtime();
// Wait for the daemons to stop.
for (String daemon : mDaemons) {
while (!SystemService.isStopped(daemon)) {
checkpoint(true);
checkInterruptAndDelay(true);
}
}
@@ -1943,7 +1941,7 @@ public class Vpn {
// Wait for the daemon to start.
while (!SystemService.isRunning(daemon)) {
checkpoint(true);
checkInterruptAndDelay(true);
}
// Create the control socket.
@@ -1959,7 +1957,7 @@ public class Vpn {
} catch (Exception e) {
// ignore
}
checkpoint(true);
checkInterruptAndDelay(true);
}
mSockets[i].setSoTimeout(500);
@@ -1973,7 +1971,7 @@ public class Vpn {
out.write(bytes.length >> 8);
out.write(bytes.length);
out.write(bytes);
checkpoint(false);
checkInterruptAndDelay(false);
}
out.write(0xFF);
out.write(0xFF);
@@ -1989,7 +1987,7 @@ public class Vpn {
} catch (Exception e) {
// ignore
}
checkpoint(true);
checkInterruptAndDelay(true);
}
}
@@ -2002,7 +2000,7 @@ public class Vpn {
throw new IllegalStateException(daemon + " is dead");
}
}
checkpoint(true);
checkInterruptAndDelay(true);
}
// Now we are connected. Read and parse the new state.
@@ -2058,8 +2056,8 @@ public class Vpn {
// Set the start time
mConfig.startTime = SystemClock.elapsedRealtime();
// Check if the thread is interrupted while we are waiting.
checkpoint(false);
// Check if the thread was interrupted while we were waiting on the lock.
checkInterruptAndDelay(false);
// Check if the interface is gone while we are waiting.
if (jniCheck(mConfig.interfaze) == 0) {
@@ -2082,10 +2080,11 @@ public class Vpn {
}
/**
* Monitor the daemons we started, moving to disconnected state if the
* underlying services fail.
* Check all daemons every two seconds. Return when one of them is stopped.
* The caller will move to the disconnected state when this function returns,
* which can happen if a daemon failed or if the VPN was torn down.
*/
private void monitorDaemons() throws InterruptedException{
private void waitForDaemonsToStop() throws InterruptedException {
if (!mNetworkInfo.isConnected()) {
return;
}