Fix secret EditText display and other fixes.

+ Now it shows shadowed "(unchanged)" (as in wifi settings) and "(not set)"
  instead of two dots.

Patch Set 2:
+ Show cancellation confirmation dialog only when there's change in
  profile.

Patch Set 3:
+ Re-enable profile preferences when the reconnect dialog is cancelled.

res/values/strings.xml

@@ -1829,7 +1829,7 @@ found in the list of installed applications.</string>
     <string name="vpn_yes_button">Yes</string>
     <string name="vpn_no_button">No</string>
     <string name="vpn_back_button">Back</string>
-    <string name="vpn_mistake_button">No, it's a mistake</string>
+    <string name="vpn_mistake_button">No</string>
 
     <string name="vpn_menu_done">Save</string>
     <string name="vpn_menu_cancel">Cancel</string>
@@ -1935,6 +1935,10 @@ found in the list of installed applications.</string>
     <string name="vpn_settings_title">VPN settings</string>
     <!-- Summary of preference to enter the VPN settings activity -->
     <string name="vpn_settings_summary">Set up &amp; manage Virtual Private Networks (VPNs)</string>
+    <!-- A secret edit field's grayed out value when it has not been modified -->
+    <string name="vpn_secret_unchanged">(unchanged)</string>
+    <!-- A secret edit field's grayed out value when it has not been set -->
+    <string name="vpn_secret_not_set">(not set)</string>
 
     <!-- Title of preference group for credential storage settings -->
     <string name="cstor_settings_category">Credential storage</string>

src/com/android/settings/vpn/L2tpEditor.java

@@ -30,9 +30,7 @@ import android.preference.PreferenceGroup;
  */
 class L2tpEditor extends VpnProfileEditor {
     private CheckBoxPreference mSecret;
-    private EditTextPreference mSecretString;
+    private SecretHandler mSecretHandler;
-    private String mOriginalSecret;
-    private boolean mOriginalSecretEnabled;
 
     public L2tpEditor(L2tpProfile p) {
         super(p);
@@ -43,11 +41,8 @@ class L2tpEditor extends VpnProfileEditor {
         Context c = subpanel.getContext();
         subpanel.addPreference(createSecretPreference(c));
         subpanel.addPreference(createSecretStringPreference(c));
-        mSecretString.setEnabled(mSecret.isChecked());
 
         L2tpProfile profile = (L2tpProfile) getProfile();
-        mOriginalSecret = profile.getSecretString();
-        mOriginalSecretEnabled = profile.isSecretEnabled();
     }
 
     @Override
@@ -55,9 +50,7 @@ class L2tpEditor extends VpnProfileEditor {
         String result = super.validate();
         if (!mSecret.isChecked()) return result;
 
-        return ((result != null)
+        return ((result != null) ? result : mSecretHandler.validate());
-                ? result
-                : validate(mSecretString, R.string.vpn_a_l2tp_secret));
     }
 
     private Preference createSecretPreference(Context c) {
@@ -73,7 +66,7 @@ class L2tpEditor extends VpnProfileEditor {
                             Preference pref, Object newValue) {
                         boolean enabled = (Boolean) newValue;
                         profile.setSecretEnabled(enabled);
-                        mSecretString.setEnabled(enabled);
+                        mSecretHandler.getPreference().setEnabled(enabled);
                         setSecretTitle(mSecret, R.string.vpn_l2tp_secret,
                                 enabled);
                         setSecretSummary(mSecret, enabled);
@@ -84,22 +77,22 @@ class L2tpEditor extends VpnProfileEditor {
     }
 
     private Preference createSecretStringPreference(Context c) {
-        final L2tpProfile profile = (L2tpProfile) getProfile();
+        SecretHandler sHandler = mSecretHandler = new SecretHandler(c,
-        mSecretString = createSecretPreference(c,
                 R.string.vpn_l2tp_secret_string_title,
-                R.string.vpn_l2tp_secret,
+                R.string.vpn_l2tp_secret) {
-                profile.getSecretString(),
+            @Override
-                new Preference.OnPreferenceChangeListener() {
+            protected String getSecretFromProfile() {
-                    public boolean onPreferenceChange(
+                return ((L2tpProfile) getProfile()).getSecretString();
-                            Preference pref, Object newValue) {
+            }
-                        profile.setSecretString((String) newValue);
+
-                        setSecretSummary(mSecretString,
+            @Override
-                                R.string.vpn_l2tp_secret,
+            protected void saveSecretToProfile(String secret) {
-                                (String) newValue);
+                ((L2tpProfile) getProfile()).setSecretString(secret);
-                        return true;
+            }
-                    }
+        };
-                });
+        Preference pref = sHandler.getPreference();
-        return mSecretString;
+        pref.setEnabled(mSecret.isChecked());
+        return pref;
     }
 
     private void setSecretSummary(CheckBoxPreference secret, boolean enabled) {

src/com/android/settings/vpn/L2tpIpsecPskEditor.java

@@ -29,6 +29,7 @@ import android.preference.PreferenceGroup;
  */
 class L2tpIpsecPskEditor extends L2tpEditor {
     private EditTextPreference mPresharedKey;
+    private SecretHandler mPskHandler;
 
     public L2tpIpsecPskEditor(L2tpIpsecPskProfile p) {
         super(p);
@@ -45,27 +46,23 @@ class L2tpIpsecPskEditor extends L2tpEditor {
     public String validate() {
         String result = super.validate();
 
-        return ((result != null)
+        return ((result != null) ? result : mPskHandler.validate());
-                ? result
-                : validate(mPresharedKey, R.string.vpn_a_ipsec_presharedkey));
     }
 
     private Preference createPresharedKeyPreference(Context c) {
-        final L2tpIpsecPskProfile profile = (L2tpIpsecPskProfile) getProfile();
+        SecretHandler pskHandler = mPskHandler = new SecretHandler(c,
-        mPresharedKey = createSecretPreference(c,
                 R.string.vpn_ipsec_presharedkey_title,
-                R.string.vpn_ipsec_presharedkey,
+                R.string.vpn_ipsec_presharedkey) {
-                profile.getPresharedKey(),
+            @Override
-                new Preference.OnPreferenceChangeListener() {
+            protected String getSecretFromProfile() {
-                    public boolean onPreferenceChange(
+                return ((L2tpIpsecPskProfile) getProfile()).getPresharedKey();
-                            Preference pref, Object newValue) {
+            }
-                        profile.setPresharedKey((String) newValue);
+
-                        setSecretSummary(mPresharedKey,
+            @Override
-                                R.string.vpn_ipsec_presharedkey,
+            protected void saveSecretToProfile(String secret) {
-                                (String) newValue);
+                ((L2tpIpsecPskProfile) getProfile()).setPresharedKey(secret);
-                        return true;
+            }
-                    }
+        };
-                });
+        return pskHandler.getPreference();
-        return mPresharedKey;
     }
 }

src/com/android/settings/vpn/VpnEditor.java

@@ -27,6 +27,7 @@ import android.net.vpn.L2tpProfile;
 import android.net.vpn.VpnProfile;
 import android.net.vpn.VpnType;
 import android.os.Bundle;
+import android.os.Parcel;
 import android.os.Parcelable;
 import android.preference.PreferenceActivity;
 import android.preference.PreferenceGroup;
@@ -47,6 +48,7 @@ public class VpnEditor extends PreferenceActivity {
 
     private VpnProfileEditor mProfileEditor;
     private boolean mAddingProfile;
+    private byte[] mOriginalProfileData;
 
     @Override
     public void onCreate(Bundle savedInstanceState) {
@@ -61,6 +63,10 @@ public class VpnEditor extends PreferenceActivity {
         addPreferencesFromResource(R.xml.vpn_edit);
 
         initViewFor(p);
+
+        Parcel parcel = Parcel.obtain();
+        p.writeToParcel(parcel, 0);
+        mOriginalProfileData = parcel.marshall();
     }
 
     @Override
@@ -90,7 +96,11 @@ public class VpnEditor extends PreferenceActivity {
                 return true;
 
             case MENU_CANCEL:
-                showCancellationConfirmDialog();
+                if (profileChanged()) {
+                    showCancellationConfirmDialog();
+                } else {
+                    finish();
+                }
                 return true;
         }
         return super.onOptionsItemSelected(item);
@@ -131,7 +141,7 @@ public class VpnEditor extends PreferenceActivity {
             return false;
         }
 
-        setResult(getProfile());
+        if (profileChanged()) setResult(getProfile());
         return true;
     }
 
@@ -177,4 +187,17 @@ public class VpnEditor extends PreferenceActivity {
     private VpnProfile getProfile() {
         return mProfileEditor.getProfile();
     }
+
+    private boolean profileChanged() {
+        Parcel newParcel = Parcel.obtain();
+        getProfile().writeToParcel(newParcel, 0);
+        byte[] newData = newParcel.marshall();
+        if (mOriginalProfileData.length == newData.length) {
+            for (int i = 0, n = mOriginalProfileData.length; i < n; i++) {
+                if (mOriginalProfileData[i] != newData[i]) return true;
+            }
+            return false;
+        }
+        return true;
+    }
 }

src/com/android/settings/vpn/VpnProfileEditor.java

@@ -33,9 +33,6 @@ import android.text.method.PasswordTransformationMethod;
  * The common class for editing {@link VpnProfile}.
  */
 class VpnProfileEditor {
-    static final String SECRET_SET_INDICATOR =
-            new String(new byte[] {(byte) 1, (byte) 0});
-
     private static final String KEY_VPN_NAME = "vpn_name";
 
     private EditTextPreference mName;
@@ -147,22 +144,6 @@ class VpnProfileEditor {
         return pref;
     }
 
-    protected EditTextPreference createSecretPreference(Context c, int titleId,
-            int fieldNameId, String value,
-            Preference.OnPreferenceChangeListener listener) {
-        EditTextPreference pref = new EditTextPreference(c);
-        pref.setTitle(titleId);
-        pref.setDialogTitle(titleId);
-        pref.getEditText().setInputType(InputType.TYPE_TEXT_VARIATION_PASSWORD);
-        pref.getEditText().setTransformationMethod(
-                new PasswordTransformationMethod());
-        pref.setText(TextUtils.isEmpty(value) ? "" : SECRET_SET_INDICATOR);
-        setSecretSummary(pref, fieldNameId, value);
-        pref.setPersistent(true);
-        pref.setOnPreferenceChangeListener(listener);
-        return pref;
-    }
-
     protected String validate(Preference pref, int fieldNameId) {
         Context c = pref.getContext();
         String value = (pref instanceof EditTextPreference)
@@ -191,15 +172,6 @@ class VpnProfileEditor {
                 : v);
     }
 
-    protected void setSecretSummary(Preference pref, int fieldNameId,
-            String value) {
-        Context c = pref.getContext();
-        String formatString = TextUtils.isEmpty(value)
-                ? c.getString(R.string.vpn_field_not_set)
-                : c.getString(R.string.vpn_field_is_set);
-        pref.setSummary(String.format(formatString, c.getString(fieldNameId)));
-    }
-
     protected void setSecretTitle(
             CheckBoxPreference pref, int fieldNameId, boolean enabled) {
         Context c = pref.getContext();
@@ -215,4 +187,69 @@ class VpnProfileEditor {
         getProfile().setName(newName);
         setSummary(mName, R.string.vpn_name, newName);
     }
+
+    // Secret is tricky to handle because empty field may mean "not set" or
+    // "unchanged". This class hides that logic from callers.
+    protected static abstract class SecretHandler {
+        private EditTextPreference mPref;
+        private int mFieldNameId;
+        private boolean mHadSecret;
+
+        protected SecretHandler(Context c, int titleId, int fieldNameId) {
+            String value = getSecretFromProfile();
+            mHadSecret = !TextUtils.isEmpty(value);
+            mFieldNameId = fieldNameId;
+
+            EditTextPreference pref = mPref = new EditTextPreference(c);
+            pref.setTitle(titleId);
+            pref.setDialogTitle(titleId);
+            pref.getEditText().setInputType(
+                    InputType.TYPE_TEXT_VARIATION_PASSWORD);
+            pref.getEditText().setTransformationMethod(
+                    new PasswordTransformationMethod());
+            pref.setText("");
+            pref.getEditText().setHint(mHadSecret
+                    ? R.string.vpn_secret_unchanged
+                    : R.string.vpn_secret_not_set);
+            setSecretSummary(value);
+            pref.setPersistent(true);
+            saveSecretToProfile("");
+            pref.setOnPreferenceChangeListener(
+                    new Preference.OnPreferenceChangeListener() {
+                        public boolean onPreferenceChange(
+                                Preference pref, Object newValue) {
+                            saveSecretToProfile((String) newValue);
+                            setSecretSummary((String) newValue);
+                            return true;
+                        }
+                    });
+        }
+
+        protected EditTextPreference getPreference() {
+            return mPref;
+        }
+
+        protected String validate() {
+            Context c = mPref.getContext();
+            String value = mPref.getText();
+            return ((TextUtils.isEmpty(value) && !mHadSecret)
+                    ? String.format(
+                            c.getString(R.string.vpn_error_miss_entering),
+                            c.getString(mFieldNameId))
+                    : null);
+        }
+
+        private void setSecretSummary(String value) {
+            EditTextPreference pref = mPref;
+            Context c = pref.getContext();
+            String formatString = (TextUtils.isEmpty(value) && !mHadSecret)
+                    ? c.getString(R.string.vpn_field_not_set)
+                    : c.getString(R.string.vpn_field_is_set);
+            pref.setSummary(
+                    String.format(formatString, c.getString(mFieldNameId)));
+        }
+
+        protected abstract String getSecretFromProfile();
+        protected abstract void saveSecretToProfile(String secret);
+    }
 }

src/com/android/settings/vpn/VpnSettings.java

@@ -18,7 +18,6 @@ package com.android.settings.vpn;
 
 import com.android.settings.R;
 import com.android.settings.SecuritySettings;
-import static com.android.settings.vpn.VpnProfileEditor.SECRET_SET_INDICATOR;
 
 import android.app.AlertDialog;
 import android.app.Dialog;
@@ -197,7 +196,7 @@ public class VpnSettings extends PreferenceActivity implements
                 return createConnectDialog();
 
             case DIALOG_RECONNECT:
-                return createReconnectDialogBuilder().create();
+                return createReconnectDialog();
 
             case DIALOG_AUTH_ERROR:
                 return createAuthErrorDialog();
@@ -219,14 +218,44 @@ public class VpnSettings extends PreferenceActivity implements
                         this)
                 .setNegativeButton(getString(android.R.string.cancel),
                         this)
+                .setOnCancelListener(new DialogInterface.OnCancelListener() {
+                            public void onCancel(DialogInterface dialog) {
+                                removeDialog(DIALOG_CONNECT);
+                                onIdle();
+                            }
+                        })
                 .create();
     }
 
-    private AlertDialog.Builder createReconnectDialogBuilder() {
+    private Dialog createReconnectDialog() {
+        return createCommonDialogBuilder()
+                .setMessage(R.string.vpn_confirm_reconnect)
+                .create();
+    }
+
+    private Dialog createAuthErrorDialog() {
+        return createCommonDialogBuilder()
+                .setMessage(R.string.vpn_auth_error_dialog_msg)
+                .create();
+    }
+    private Dialog createUnknownServerDialog() {
+        return createCommonDialogBuilder()
+                .setMessage(R.string.vpn_unknown_server_dialog_msg)
+                .setPositiveButton(R.string.vpn_yes_button,
+                        new DialogInterface.OnClickListener() {
+                            public void onClick(DialogInterface dialog, int w) {
+                                VpnProfile p = mConnectingActor.getProfile();
+                                onIdle();
+                                startVpnEditor(p);
+                            }
+                        })
+                .create();
+    }
+
+    private AlertDialog.Builder createCommonDialogBuilder() {
         return new AlertDialog.Builder(this)
                 .setTitle(android.R.string.dialog_alert_title)
                 .setIcon(android.R.drawable.ic_dialog_alert)
-                .setMessage(R.string.vpn_confirm_reconnect)
                 .setPositiveButton(R.string.vpn_yes_button,
                         new DialogInterface.OnClickListener() {
                             public void onClick(DialogInterface dialog, int w) {
@@ -246,26 +275,6 @@ public class VpnSettings extends PreferenceActivity implements
                         });
     }
 
-    private Dialog createAuthErrorDialog() {
-        return createReconnectDialogBuilder()
-                .setMessage(R.string.vpn_auth_error_dialog_msg)
-                .create();
-    }
-
-    private Dialog createUnknownServerDialog() {
-        return createReconnectDialogBuilder()
-                .setMessage(R.string.vpn_unknown_server_dialog_msg)
-                .setPositiveButton(R.string.vpn_yes_button,
-                        new DialogInterface.OnClickListener() {
-                            public void onClick(DialogInterface dialog, int w) {
-                                VpnProfile p = mConnectingActor.getProfile();
-                                onIdle();
-                                startVpnEditor(p);
-                            }
-                        })
-                .create();
-    }
-
     @Override
     public void onCreateContextMenu(ContextMenu menu, View v,
             ContextMenuInfo menuInfo) {
@@ -413,6 +422,7 @@ public class VpnSettings extends PreferenceActivity implements
             }
         } else {
             removeDialog(DIALOG_CONNECT);
+            onIdle();
         }
     }
 
@@ -707,6 +717,7 @@ public class VpnSettings extends PreferenceActivity implements
     }
 
     private void onIdle() {
+        Log.d(TAG, "   onIdle()");
         mActiveProfile = null;
         mConnectingActor = null;
         enableProfilePreferences();
@@ -853,28 +864,28 @@ public class VpnSettings extends PreferenceActivity implements
             case L2TP_IPSEC_PSK:
                 L2tpIpsecPskProfile pskProfile = (L2tpIpsecPskProfile) p;
                 String presharedKey = pskProfile.getPresharedKey();
-                if (!presharedKey.equals(SECRET_SET_INDICATOR)) {
+                String keyName = KEY_PREFIX_IPSEC_PSK + p.getId();
-                    String keyName = KEY_PREFIX_IPSEC_PSK + p.getId();
+                if (!TextUtils.isEmpty(presharedKey)) {
                     int ret = ks.put(NAMESPACE_VPN, keyName, presharedKey);
-                    if (ret < 0) {
+                    if (ret != 0) {
                         Log.e(TAG, "keystore write failed: key=" + keyName);
                     }
-                    pskProfile.setPresharedKey(keyNameForDaemon(keyName));
                 }
+                pskProfile.setPresharedKey(keyNameForDaemon(keyName));
                 // pass through
 
             case L2TP:
                 L2tpProfile l2tpProfile = (L2tpProfile) p;
-                String keyName = KEY_PREFIX_L2TP_SECRET + p.getId();
+                keyName = KEY_PREFIX_L2TP_SECRET + p.getId();
                 if (l2tpProfile.isSecretEnabled()) {
                     String secret = l2tpProfile.getSecretString();
-                    if (!secret.equals(SECRET_SET_INDICATOR)) {
+                    if (!TextUtils.isEmpty(secret)) {
                         int ret = ks.put(NAMESPACE_VPN, keyName, secret);
-                        if (ret < 0) {
+                        if (ret != 0) {
                             Log.e(TAG, "keystore write failed: key=" + keyName);
                         }
-                        l2tpProfile.setSecretString(keyNameForDaemon(keyName));
                     }
+                    l2tpProfile.setSecretString(keyNameForDaemon(keyName));
                 } else {
                     ks.remove(NAMESPACE_VPN, keyName);
                 }