Readability improvements to the Vpn class with no logic changes

Test: runtests frameworks-net
Change-Id: I18de9dc9801b578e5a71adea5c38e49aa40cd2b1
This commit is contained in:
Chalard Jean
2017-12-20 13:23:32 +09:00
parent e4e5291743
commit dd59eced79

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.
*/
@@ -480,7 +481,6 @@ public class Vpn {
}
private void unregisterPackageChangeReceiverLocked() {
// register previous intent filter
if (mIsPackageIntentReceiverRegistered) {
mContext.unregisterReceiver(mPackageIntentReceiver);
mIsPackageIntentReceiverRegistered = false;
@@ -581,7 +581,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);
@@ -611,9 +611,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:
*
@@ -629,7 +630,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) {
@@ -638,7 +639,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.
@@ -860,8 +861,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.
@@ -935,7 +936,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);
}
@@ -943,7 +944,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);
}
@@ -1336,7 +1337,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");
@@ -1416,7 +1417,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() {
@@ -1767,7 +1768,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 {
@@ -1780,7 +1781,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.
@@ -1860,8 +1861,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 {
@@ -1882,30 +1883,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);
}
}
@@ -1942,7 +1940,7 @@ public class Vpn {
// Wait for the daemon to start.
while (!SystemService.isRunning(daemon)) {
checkpoint(true);
checkInterruptAndDelay(true);
}
// Create the control socket.
@@ -1958,7 +1956,7 @@ public class Vpn {
} catch (Exception e) {
// ignore
}
checkpoint(true);
checkInterruptAndDelay(true);
}
mSockets[i].setSoTimeout(500);
@@ -1972,7 +1970,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);
@@ -1988,7 +1986,7 @@ public class Vpn {
} catch (Exception e) {
// ignore
}
checkpoint(true);
checkInterruptAndDelay(true);
}
}
@@ -2001,7 +1999,7 @@ public class Vpn {
throw new IllegalStateException(daemon + " is dead");
}
}
checkpoint(true);
checkInterruptAndDelay(true);
}
// Now we are connected. Read and parse the new state.
@@ -2057,8 +2055,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) {
@@ -2081,10 +2079,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;
}