From e06d757a0cc134053866c7f25d1e525138b1c169 Mon Sep 17 00:00:00 2001 From: Robin Lee Date: Tue, 19 Apr 2016 12:29:02 +0100 Subject: [PATCH 1/2] One updateSummary method called by all VPN prefs Having multiple methods means inevitably when new features are added to the preferences, the right calls aren't made so information on the screen lags between updates. Bug: 28257641 Change-Id: I336aeefd5941ccf808dc9070427209a7d2530032 (cherry picked from commit 903843e6f9ead6f17d3685a551dc85083b147038) --- .../android/settings/vpn2/AppPreference.java | 15 ++----------- .../settings/vpn2/LegacyVpnPreference.java | 16 +------------- .../settings/vpn2/ManageablePreference.java | 22 ++++++++++++++----- 3 files changed, 20 insertions(+), 33 deletions(-) diff --git a/src/com/android/settings/vpn2/AppPreference.java b/src/com/android/settings/vpn2/AppPreference.java index e3649a897b0..fa9daf6187a 100644 --- a/src/com/android/settings/vpn2/AppPreference.java +++ b/src/com/android/settings/vpn2/AppPreference.java @@ -32,9 +32,8 @@ import com.android.internal.net.VpnConfig; */ public class AppPreference extends ManageablePreference { public static final int STATE_CONNECTED = LegacyVpnInfo.STATE_CONNECTED; - public static final int STATE_DISCONNECTED = LegacyVpnInfo.STATE_DISCONNECTED; + public static final int STATE_DISCONNECTED = STATE_NONE; - private int mState = STATE_DISCONNECTED; private String mPackageName; private String mName; @@ -70,22 +69,11 @@ public class AppPreference extends ManageablePreference { update(); } - public int getState() { - return mState; - } - - public void setState(int state) { - mState = state; - update(); - } - private void update() { if (mPackageName == null || mUserId == UserHandle.USER_NULL) { return; } - setSummary(getSummaryString(mState == STATE_DISCONNECTED ? STATE_NONE : mState)); - mName = mPackageName; Drawable icon = null; @@ -113,6 +101,7 @@ public class AppPreference extends ManageablePreference { } setTitle(mName); setIcon(icon); + updateSummary(); notifyHierarchyChanged(); } diff --git a/src/com/android/settings/vpn2/LegacyVpnPreference.java b/src/com/android/settings/vpn2/LegacyVpnPreference.java index a9b8aece9a7..2ce22d487c5 100644 --- a/src/com/android/settings/vpn2/LegacyVpnPreference.java +++ b/src/com/android/settings/vpn2/LegacyVpnPreference.java @@ -31,9 +31,6 @@ import static com.android.internal.net.LegacyVpnInfo.STATE_CONNECTED; public class LegacyVpnPreference extends ManageablePreference { private VpnProfile mProfile; - /** One of the STATE_* fields from LegacyVpnInfo, or STATE_NONE */ - private int mState = STATE_NONE; - LegacyVpnPreference(Context context) { super(context, null /* attrs */); } @@ -44,16 +41,6 @@ public class LegacyVpnPreference extends ManageablePreference { public void setProfile(VpnProfile profile) { mProfile = profile; - update(); - } - - public void setState(int state) { - mState = state; - update(); - } - - private void update() { - setSummary(getSummaryString(mState)); if (mProfile != null) { setIcon(R.mipmap.ic_launcher_settings); setTitle(mProfile.name); @@ -93,5 +80,4 @@ public class LegacyVpnPreference extends ManageablePreference { } super.onClick(v); } - -} \ No newline at end of file +} diff --git a/src/com/android/settings/vpn2/ManageablePreference.java b/src/com/android/settings/vpn2/ManageablePreference.java index ad8a8a31168..7c07e2018bc 100644 --- a/src/com/android/settings/vpn2/ManageablePreference.java +++ b/src/com/android/settings/vpn2/ManageablePreference.java @@ -34,6 +34,7 @@ public abstract class ManageablePreference extends GearPreference { public static int STATE_NONE = -1; boolean mIsAlwaysOn = false; + int mState = STATE_NONE; int mUserId; public ManageablePreference(Context context, AttributeSet attrs) { @@ -56,24 +57,35 @@ public abstract class ManageablePreference extends GearPreference { return mIsAlwaysOn; } + public int getState() { + return mState; + } + + public void setState(int state) { + mState = state; + updateSummary(); + } + public void setAlwaysOn(boolean isEnabled) { mIsAlwaysOn = isEnabled; + updateSummary(); } /** - * State is not shown for {@code STATE_NONE} + * Update the preference summary string (see {@see Preference#setSummary}) with a string + * reflecting connection status and always-on setting. * - * @return summary string showing current connection state and always-on-vpn state + * State is not shown for {@code STATE_NONE}. */ - protected String getSummaryString(int state) { + protected void updateSummary() { final Resources res = getContext().getResources(); final String[] states = res.getStringArray(R.array.vpn_states); - String summary = state == STATE_NONE ? "" : states[state]; + String summary = (mState == STATE_NONE ? "" : states[mState]); if (mIsAlwaysOn) { final String alwaysOnString = res.getString(R.string.vpn_always_on_active); summary = TextUtils.isEmpty(summary) ? alwaysOnString : res.getString( R.string.join_two_unrelated_items, summary, alwaysOnString); } - return summary; + setSummary(summary); } } From b166ea26687f7b72cbc544438436ae85167ca2f1 Mon Sep 17 00:00:00 2001 From: Robin Lee Date: Thu, 21 Apr 2016 12:56:21 +0100 Subject: [PATCH 2/2] Be more aggressive caching Vpn preference attrs As any change to the preference title will cause it to lose focus, best not to do this too often. Change-Id: Ibac27ee1de42fd7ca05f3e3685b84f37dac39517 Fix: 28191965 --- .../android/settings/vpn2/AppPreference.java | 84 ++++++++----------- .../settings/vpn2/LegacyVpnPreference.java | 13 +-- .../settings/vpn2/ManageablePreference.java | 13 ++- .../android/settings/vpn2/VpnSettings.java | 48 +++++------ 4 files changed, 71 insertions(+), 87 deletions(-) diff --git a/src/com/android/settings/vpn2/AppPreference.java b/src/com/android/settings/vpn2/AppPreference.java index fa9daf6187a..3369970baf2 100644 --- a/src/com/android/settings/vpn2/AppPreference.java +++ b/src/com/android/settings/vpn2/AppPreference.java @@ -34,17 +34,43 @@ public class AppPreference extends ManageablePreference { public static final int STATE_CONNECTED = LegacyVpnInfo.STATE_CONNECTED; public static final int STATE_DISCONNECTED = STATE_NONE; - private String mPackageName; - private String mName; + private final String mPackageName; + private final String mName; - public AppPreference(Context context) { + public AppPreference(Context context, int userId, String packageName) { super(context, null /* attrs */); - } - - @Override - public void setUserId(int userId) { super.setUserId(userId); - update(); + + mPackageName = packageName; + + // Fetch icon and VPN label + String label = packageName; + Drawable icon = null; + try { + // Make all calls to the package manager as the appropriate user. + Context userContext = getUserContext(); + PackageManager pm = userContext.getPackageManager(); + // The nested catch block is for the case that the app doesn't exist, so we can fall + // back to the default activity icon. + try { + PackageInfo pkgInfo = pm.getPackageInfo(mPackageName, 0 /* flags */); + if (pkgInfo != null) { + icon = pkgInfo.applicationInfo.loadIcon(pm); + label = VpnConfig.getVpnLabel(userContext, mPackageName).toString(); + } + } catch (PackageManager.NameNotFoundException pkgNotFound) { + // Use default app label and icon as fallback + } + if (icon == null) { + icon = pm.getDefaultActivityIcon(); + } + } catch (PackageManager.NameNotFoundException userNotFound) { + // No user, no useful information to obtain. Quietly fail. + } + mName = label; + + setTitle(mName); + setIcon(icon); } public PackageInfo getPackageInfo() { @@ -64,48 +90,6 @@ public class AppPreference extends ManageablePreference { return mPackageName; } - public void setPackageName(String name) { - mPackageName = name; - update(); - } - - private void update() { - if (mPackageName == null || mUserId == UserHandle.USER_NULL) { - return; - } - - mName = mPackageName; - Drawable icon = null; - - try { - // Make all calls to the package manager as the appropriate user. - Context userContext = getUserContext(); - PackageManager pm = userContext.getPackageManager(); - // Fetch icon and VPN label- the nested catch block is for the case that the app doesn't - // exist, in which case we can fall back to the default activity icon for an activity in - // that user. - try { - PackageInfo pkgInfo = pm.getPackageInfo(mPackageName, 0 /* flags */); - if (pkgInfo != null) { - icon = pkgInfo.applicationInfo.loadIcon(pm); - mName = VpnConfig.getVpnLabel(userContext, mPackageName).toString(); - } - } catch (PackageManager.NameNotFoundException pkgNotFound) { - // Use default app label and icon as fallback - } - if (icon == null) { - icon = pm.getDefaultActivityIcon(); - } - } catch (PackageManager.NameNotFoundException userNotFound) { - // No user, no useful information to obtain. Quietly fail. - } - setTitle(mName); - setIcon(icon); - updateSummary(); - - notifyHierarchyChanged(); - } - private Context getUserContext() throws PackageManager.NameNotFoundException { UserHandle user = UserHandle.of(mUserId); return getContext().createPackageContextAsUser( diff --git a/src/com/android/settings/vpn2/LegacyVpnPreference.java b/src/com/android/settings/vpn2/LegacyVpnPreference.java index 2ce22d487c5..c1550e242ca 100644 --- a/src/com/android/settings/vpn2/LegacyVpnPreference.java +++ b/src/com/android/settings/vpn2/LegacyVpnPreference.java @@ -18,6 +18,7 @@ package com.android.settings.vpn2; import android.content.Context; import android.support.v7.preference.Preference; +import android.text.TextUtils; import android.view.View; import com.android.internal.net.VpnProfile; @@ -33,6 +34,7 @@ public class LegacyVpnPreference extends ManageablePreference { LegacyVpnPreference(Context context) { super(context, null /* attrs */); + setIcon(R.mipmap.ic_launcher_settings); } public VpnProfile getProfile() { @@ -40,12 +42,13 @@ public class LegacyVpnPreference extends ManageablePreference { } public void setProfile(VpnProfile profile) { - mProfile = profile; - if (mProfile != null) { - setIcon(R.mipmap.ic_launcher_settings); - setTitle(mProfile.name); + final String oldLabel = (mProfile != null ? mProfile.name : null); + final String newLabel = (profile != null ? profile.name : null); + if (!TextUtils.equals(oldLabel, newLabel)) { + setTitle(newLabel); + notifyHierarchyChanged(); } - notifyHierarchyChanged(); + mProfile = profile; } @Override diff --git a/src/com/android/settings/vpn2/ManageablePreference.java b/src/com/android/settings/vpn2/ManageablePreference.java index 7c07e2018bc..e31a396f7ec 100644 --- a/src/com/android/settings/vpn2/ManageablePreference.java +++ b/src/com/android/settings/vpn2/ManageablePreference.java @@ -62,13 +62,18 @@ public abstract class ManageablePreference extends GearPreference { } public void setState(int state) { - mState = state; - updateSummary(); + if (mState != state) { + mState = state; + updateSummary(); + notifyHierarchyChanged(); + } } public void setAlwaysOn(boolean isEnabled) { - mIsAlwaysOn = isEnabled; - updateSummary(); + if (mIsAlwaysOn != isEnabled) { + mIsAlwaysOn = isEnabled; + updateSummary(); + } } /** diff --git a/src/com/android/settings/vpn2/VpnSettings.java b/src/com/android/settings/vpn2/VpnSettings.java index 6c47b438f0f..a675779bf99 100644 --- a/src/com/android/settings/vpn2/VpnSettings.java +++ b/src/com/android/settings/vpn2/VpnSettings.java @@ -34,7 +34,6 @@ import android.os.Handler; import android.os.Message; import android.os.RemoteException; import android.os.ServiceManager; -import android.os.SystemProperties; import android.os.UserHandle; import android.os.UserManager; import android.security.Credentials; @@ -211,8 +210,8 @@ public class VpnSettings extends RestrictedSettingsFragment implements final List vpnProfiles = loadVpnProfiles(mKeyStore); final List vpnApps = getVpnApps(getActivity(), /* includeProfiles */ true); - final List connectedLegacyVpns = getConnectedLegacyVpns(); - final List connectedAppVpns = getConnectedAppVpns(); + final Map connectedLegacyVpns = getConnectedLegacyVpns(); + final Set connectedAppVpns = getConnectedAppVpns(); final Set alwaysOnAppVpnInfos = getAlwaysOnAppVpnInfos(); final String lockdownVpnKey = VpnUtils.getLockdownVpn(); @@ -231,18 +230,26 @@ public class VpnSettings extends RestrictedSettingsFragment implements for (VpnProfile profile : vpnProfiles) { LegacyVpnPreference p = findOrCreatePreference(profile); - p.setState(LegacyVpnPreference.STATE_NONE); + if (connectedLegacyVpns.containsKey(profile.key)) { + p.setState(connectedLegacyVpns.get(profile.key).state); + } else { + p.setState(LegacyVpnPreference.STATE_NONE); + } p.setAlwaysOn(lockdownVpnKey != null && lockdownVpnKey.equals(profile.key)); updates.add(p); } for (AppVpnInfo app : vpnApps) { AppPreference p = findOrCreatePreference(app); - p.setState(AppPreference.STATE_DISCONNECTED); + if (connectedAppVpns.contains(app)) { + p.setState(AppPreference.STATE_CONNECTED); + } else { + p.setState(AppPreference.STATE_DISCONNECTED); + } p.setAlwaysOn(alwaysOnAppVpnInfos.contains(app)); updates.add(p); } - // Trim preferences for deleted VPNs + // Trim out deleted VPN preferences mLegacyVpnPreferences.values().retainAll(updates); mAppPreferences.values().retainAll(updates); @@ -260,20 +267,6 @@ public class VpnSettings extends RestrictedSettingsFragment implements for (Preference pref : updates) { vpnGroup.addPreference(pref); } - - // Mark connected VPNs - for (LegacyVpnInfo info : connectedLegacyVpns) { - final LegacyVpnPreference preference = mLegacyVpnPreferences.get(info.key); - if (preference != null) { - preference.setState(info.state); - } - } - for (AppVpnInfo app : connectedAppVpns) { - final AppPreference preference = mAppPreferences.get(app); - if (preference != null) { - preference.setState(AppPreference.STATE_CONNECTED); - } - } } }); @@ -369,6 +362,7 @@ public class VpnSettings extends RestrictedSettingsFragment implements pref.setOnPreferenceClickListener(this); mLegacyVpnPreferences.put(profile.key, pref); } + // This may change as the profile can update and keep the same key. pref.setProfile(profile); return pref; } @@ -377,33 +371,31 @@ public class VpnSettings extends RestrictedSettingsFragment implements private AppPreference findOrCreatePreference(AppVpnInfo app) { AppPreference pref = mAppPreferences.get(app); if (pref == null) { - pref = new AppPreference(getPrefContext()); + pref = new AppPreference(getPrefContext(), app.userId, app.packageName); pref.setOnGearClickListener(mGearListener); pref.setOnPreferenceClickListener(this); mAppPreferences.put(app, pref); } - pref.setUserId(app.userId); - pref.setPackageName(app.packageName); return pref; } @WorkerThread - private List getConnectedLegacyVpns() { + private Map getConnectedLegacyVpns() { try { mConnectedLegacyVpn = mConnectivityService.getLegacyVpnInfo(UserHandle.myUserId()); if (mConnectedLegacyVpn != null) { - return Collections.singletonList(mConnectedLegacyVpn); + return Collections.singletonMap(mConnectedLegacyVpn.key, mConnectedLegacyVpn); } } catch (RemoteException e) { Log.e(LOG_TAG, "Failure updating VPN list with connected legacy VPNs", e); } - return Collections.emptyList(); + return Collections.emptyMap(); } @WorkerThread - private List getConnectedAppVpns() { + private Set getConnectedAppVpns() { // Mark connected third-party services - List connections = new ArrayList<>(); + Set connections = new ArraySet<>(); try { for (UserHandle profile : mUserManager.getUserProfiles()) { VpnConfig config = mConnectivityService.getVpnConfig(profile.getIdentifier());