#8857 fix recreation / restore of ConfigurationWizard error dialogs

6 files changed, 82 insertions, 56 deletions

diff --git a/app/src/main/java/se/leap/bitmaskclient/ConfigWizardBaseActivity.java b/app/src/main/java/se/leap/bitmaskclient/ConfigWizardBaseActivity.java
index 7fcb5816..ea328216 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ConfigWizardBaseActivity.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ConfigWizardBaseActivity.java
@@ -2,6 +2,7 @@ package se.leap.bitmaskclient;
 
 import android.content.SharedPreferences;
 import android.os.Bundle;
+import android.os.PersistableBundle;
 import android.support.annotation.DrawableRes;
 import android.support.annotation.Nullable;
 import android.support.annotation.StringRes;
@@ -74,6 +75,20 @@ public abstract class ConfigWizardBaseActivity extends ButterKnifeActivity {
             setProviderHeaderText(provider.getName());
     }
 
+    @Override
+    public void onSaveInstanceState(Bundle outState) {
+        super.onSaveInstanceState(outState);
+        if (provider != null) {
+            outState.putParcelable(PROVIDER_KEY, provider);
+        }
+    }
+
+    protected void restoreState(Bundle savedInstanceState) {
+        if (savedInstanceState != null && savedInstanceState.containsKey(PROVIDER_KEY)) {
+            provider = savedInstanceState.getParcelable(PROVIDER_KEY);
+        }
+    }
+
     protected void setProviderHeaderLogo(@DrawableRes int providerHeaderLogo) {
         this.providerHeaderLogo.setImageResource(providerHeaderLogo);
     }
diff --git a/app/src/main/java/se/leap/bitmaskclient/MainActivityErrorDialog.java b/app/src/main/java/se/leap/bitmaskclient/MainActivityErrorDialog.java
index 7756bcfd..23bc8427 100644
--- a/app/src/main/java/se/leap/bitmaskclient/MainActivityErrorDialog.java
+++ b/app/src/main/java/se/leap/bitmaskclient/MainActivityErrorDialog.java
@@ -23,7 +23,6 @@ import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.v4.app.DialogFragment;
 import android.support.v7.app.AlertDialog;
-import android.util.Log;
 
 import org.json.JSONObject;
 
diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java b/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java
index ba10ae24..6faf8bb8 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ProviderCredentialsBaseActivity.java
@@ -102,9 +102,7 @@ public abstract class ProviderCredentialsBaseActivity extends ConfigWizardBaseAc
         LocalBroadcastManager.getInstance(this).registerReceiver(providerAPIBroadcastReceiver, updateIntentFilter);
 
         setUpListeners();
-        if(savedInstanceState != null) {
-            restoreState(savedInstanceState);
-        }
+        restoreState(savedInstanceState);
 
         String userMessageString = getIntent().getStringExtra(USER_MESSAGE);
         if (userMessageString != null) {
@@ -127,7 +125,11 @@ public abstract class ProviderCredentialsBaseActivity extends ConfigWizardBaseAc
         }
     }
 
-    private void restoreState(Bundle savedInstance) {
+    protected void restoreState(Bundle savedInstance) {
+        super.restoreState(savedInstance);
+        if (savedInstance == null) {
+            return;
+        }
         if (savedInstance.getString(USER_MESSAGE) != null) {
             userMessage.setText(savedInstance.getString(USER_MESSAGE));
             userMessage.setVisibility(VISIBLE);
diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java b/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java
index 3b855601..e961b0a2 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ProviderListBaseActivity.java
@@ -91,7 +91,8 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
     final protected static String PROVIDER_NOT_SET = "PROVIDER NOT SET";
     final protected static String SETTING_UP_PROVIDER = "PROVIDER GETS SET";
     final private static  String SHOWING_PROVIDER_DETAILS = "SHOWING PROVIDER DETAILS";
-    final private static String PENDING_SHOW_FAILED_DIALOG = "SHOW FAILED DIALOG";
+    final private static String PENDING_SHOW_FAILED_DIALOG = "SHOW FAILED DIALOG PENDING";
+    final private static String SHOW_FAILED_DIALOG = "SHOW FAILED DIALOG";
     final private static String REASON_TO_FAIL = "REASON TO FAIL";
     final protected static String SERVICES_RETRIEVED = "SERVICES RETRIEVED";
 
@@ -117,19 +118,24 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
     }
 
     @Override
-    protected void onSaveInstanceState(@NotNull Bundle outState) {
+    public void onSaveInstanceState(@NotNull Bundle outState) {
         outState.putString(ACTIVITY_STATE, mConfigState.getAction());
-        outState.putParcelable(PROVIDER_KEY, provider);
-
-        DialogFragment dialogFragment = (DialogFragment) fragmentManager.findFragmentByTag(ProviderSetupFailedDialog.TAG);
-        if (dialogFragment != null) {
-            outState.putString(REASON_TO_FAIL, reasonToFail);
-            dialogFragment.dismiss();
-        }
+        outState.putString(REASON_TO_FAIL, reasonToFail);
 
         super.onSaveInstanceState(outState);
     }
 
+    protected void restoreState(Bundle savedInstanceState) {
+        super.restoreState(savedInstanceState);
+        if (savedInstanceState == null) {
+            return;
+        }
+        mConfigState.setAction(savedInstanceState.getString(ACTIVITY_STATE, PROVIDER_NOT_SET));
+        if (savedInstanceState.containsKey(REASON_TO_FAIL)) {
+            reasonToFail = savedInstanceState.getString(REASON_TO_FAIL);
+        }
+    }
+
     @Override
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
@@ -137,28 +143,8 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
         providerManager = ProviderManager.getInstance(getAssets(), getExternalFilesDir(null));
 
         setUpInitialUI();
-
         initProviderList();
-
-        if (savedInstanceState != null)
-            restoreState(savedInstanceState);
-    }
-
-    private void restoreState(Bundle savedInstanceState) {
-
-        provider = savedInstanceState.getParcelable(Provider.KEY);
-        mConfigState.setAction(savedInstanceState.getString(ACTIVITY_STATE, PROVIDER_NOT_SET));
-
-        reasonToFail = savedInstanceState.getString(REASON_TO_FAIL);
-        if(reasonToFail != null) {
-            showDownloadFailedDialog();
-        }
-
-        if (SETTING_UP_PROVIDER.equals(mConfigState.getAction()) ||
-                         PENDING_SHOW_FAILED_DIALOG.equals(mConfigState.getAction())
-                ) {
-            showProgressBar();
-        }
+        restoreState(savedInstanceState);
     }
 
     @Override
@@ -166,15 +152,17 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
         Log.d(TAG, "resuming with ConfigState: " + mConfigState.getAction());
         super.onResume();
         setUpProviderAPIResultReceiver();
-        hideProgressBar();
         isActivityShowing = true;
         if (SETTING_UP_PROVIDER.equals(mConfigState.getAction())) {
             showProgressBar();
             checkProviderSetUp();
         } else if (PENDING_SHOW_FAILED_DIALOG.equals(mConfigState.getAction())) {
+            showProgressBar();
             showDownloadFailedDialog();
+        } else if (SHOW_FAILED_DIALOG.equals(mConfigState.getAction())) {
+            showProgressBar();
         } else if (SHOWING_PROVIDER_DETAILS.equals(mConfigState.getAction())) {
-            cancelAndShowAllProviders();
+            cancelSettingUpProvider();
         }
     }
 
@@ -257,7 +245,7 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
     @OnItemClick(R.id.provider_list)
     void onItemSelected(int position) {
         if (SETTING_UP_PROVIDER.equals(mConfigState.getAction()) ||
-                PENDING_SHOW_FAILED_DIALOG.equals(mConfigState.getAction())) {
+                SHOW_FAILED_DIALOG.equals(mConfigState.getAction())) {
             return;
         }
 
@@ -276,10 +264,9 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
     @Override
     public void onBackPressed() {
         if (SETTING_UP_PROVIDER.equals(mConfigState.getAction()) ||
-                PENDING_SHOW_FAILED_DIALOG.equals(mConfigState.getAction())) {
+                SHOW_FAILED_DIALOG.equals(mConfigState.getAction())) {
             stopSettingUpProvider();
         } else {
-            askDashboardToQuitApp();
             super.onBackPressed();
         }
     }
@@ -291,11 +278,13 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
     @Override
     public void cancelSettingUpProvider() {
         mConfigState.setAction(PROVIDER_NOT_SET);
+        provider = null;
         hideProgressBar();
     }
 
     @Override
     public void updateProviderDetails() {
+        mConfigState.setAction(SETTING_UP_PROVIDER);
         ProviderAPICommand.execute(this, UPDATE_PROVIDER_DETAILS, provider);
     }
 
@@ -303,12 +292,6 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
         ProviderAPICommand.execute(this, PROVIDER_SET_UP, provider, providerAPIResultReceiver);
     }
 
-    private void askDashboardToQuitApp() {
-        Intent askQuit = new Intent();
-        askQuit.putExtra(APP_ACTION_QUIT, APP_ACTION_QUIT);
-        setResult(RESULT_CANCELED, askQuit);
-    }
-
     /**
      * Asks ProviderApiService to download an anonymous (anon) VPN certificate.
      */
@@ -345,6 +328,7 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
      */
     public void showDownloadFailedDialog() {
         try {
+            mConfigState.setAction(SHOW_FAILED_DIALOG);
             FragmentTransaction fragmentTransaction = fragmentManager.removePreviousFragment(ProviderSetupFailedDialog.TAG);
             DialogFragment newFragment;
             try {
@@ -353,12 +337,14 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
             } catch (JSONException e) {
                 e.printStackTrace();
                 newFragment = ProviderSetupFailedDialog.newInstance(provider, reasonToFail);
+            } catch (NullPointerException e) {
+                //reasonToFail was null
+                return;
             }
             newFragment.show(fragmentTransaction, ProviderSetupFailedDialog.TAG);
         } catch (IllegalStateException e) {
             e.printStackTrace();
             mConfigState.setAction(PENDING_SHOW_FAILED_DIALOG);
-            mConfigState.putExtra(REASON_TO_FAIL, reasonToFail);
         }
 
     }
@@ -388,11 +374,6 @@ public abstract class ProviderListBaseActivity extends ConfigWizardBaseActivity
         return true;
     }
 
-    public void cancelAndShowAllProviders() {
-        mConfigState.setAction(PROVIDER_NOT_SET);
-        provider = null;
-    }
-
     public class ProviderAPIBroadcastReceiver extends BroadcastReceiver {
         @Override
         public void onReceive(Context context, Intent intent) {
diff --git a/app/src/main/java/se/leap/bitmaskclient/ProviderSetupFailedDialog.java b/app/src/main/java/se/leap/bitmaskclient/ProviderSetupFailedDialog.java
index 9e77452c..5bd9575e 100644
--- a/app/src/main/java/se/leap/bitmaskclient/ProviderSetupFailedDialog.java
+++ b/app/src/main/java/se/leap/bitmaskclient/ProviderSetupFailedDialog.java
@@ -22,6 +22,7 @@ import android.content.Context;
 import android.content.DialogInterface;
 import android.os.Bundle;
 import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
 import android.support.v4.app.DialogFragment;
 
 import org.json.JSONObject;
@@ -39,6 +40,9 @@ import static se.leap.bitmaskclient.ProviderAPI.ERRORS;
 public class ProviderSetupFailedDialog extends DialogFragment {
 
     public static String TAG = "downloaded_failed_dialog";
+    private final static String KEY_PROVIDER = "key provider";
+    private final static String KEY_REASON_TO_FAIL = "key reason to fail";
+    private final static String KEY_DOWNLOAD_ERROR = "key download error";
     private String reasonToFail;
     private DOWNLOAD_ERRORS downloadError = DEFAULT;
 
@@ -86,6 +90,12 @@ public class ProviderSetupFailedDialog extends DialogFragment {
     }
 
     @Override
+    public void onCreate(@Nullable Bundle savedInstanceState) {
+        super.onCreate(savedInstanceState);
+        restoreFromSavedInstance(savedInstanceState);
+    }
+
+    @Override
     @NonNull
     public Dialog onCreateDialog(Bundle savedInstanceState) {
         AlertDialog.Builder builder = new AlertDialog.Builder(getActivity());
@@ -93,7 +103,6 @@ public class ProviderSetupFailedDialog extends DialogFragment {
                 .setNegativeButton(R.string.cancel, new DialogInterface.OnClickListener() {
             public void onClick(DialogInterface dialog, int id) {
                 interfaceWithConfigurationWizard.cancelSettingUpProvider();
-                dialog.dismiss();
             }
         });
         switch (downloadError) {
@@ -101,7 +110,6 @@ public class ProviderSetupFailedDialog extends DialogFragment {
                 builder.setPositiveButton(R.string.update_provider_details, new DialogInterface.OnClickListener() {
                     @Override
                     public void onClick(DialogInterface dialog, int which) {
-                        dismiss();
                         interfaceWithConfigurationWizard.updateProviderDetails();
                     }
                 });
@@ -111,7 +119,6 @@ public class ProviderSetupFailedDialog extends DialogFragment {
                 builder.setPositiveButton(R.string.update_certificate, new DialogInterface.OnClickListener() {
                     @Override
                     public void onClick(DialogInterface dialog, int which) {
-                        dismiss();
                         interfaceWithConfigurationWizard.updateProviderDetails();
                     }
                 });
@@ -119,7 +126,6 @@ public class ProviderSetupFailedDialog extends DialogFragment {
             default:
                 builder.setPositiveButton(R.string.retry, new DialogInterface.OnClickListener() {
                             public void onClick(DialogInterface dialog, int id) {
-                                dismiss();
                                 interfaceWithConfigurationWizard.retrySetUpProvider(provider);
                             }
                         });
@@ -157,4 +163,26 @@ public class ProviderSetupFailedDialog extends DialogFragment {
         dialog.dismiss();
     }
 
+    @Override
+    public void onSaveInstanceState(Bundle outState) {
+        super.onSaveInstanceState(outState);
+        outState.putParcelable(KEY_PROVIDER, provider);
+        outState.putString(KEY_REASON_TO_FAIL, reasonToFail);
+        outState.putString(KEY_DOWNLOAD_ERROR, downloadError.toString());
+    }
+
+    private void restoreFromSavedInstance(Bundle savedInstanceState) {
+        if (savedInstanceState == null) {
+            return;
+        }
+        if (savedInstanceState.containsKey(KEY_PROVIDER)) {
+            this.provider = savedInstanceState.getParcelable(KEY_PROVIDER);
+        }
+        if (savedInstanceState.containsKey(KEY_REASON_TO_FAIL)) {
+            this.reasonToFail = savedInstanceState.getString(KEY_REASON_TO_FAIL);
+        }
+        if (savedInstanceState.containsKey(KEY_DOWNLOAD_ERROR)) {
+            this.downloadError = valueOf(savedInstanceState.getString(KEY_DOWNLOAD_ERROR));
+        }
+    }
 }
diff --git a/app/src/production/java/se/leap/bitmaskclient/ProviderListActivity.java b/app/src/production/java/se/leap/bitmaskclient/ProviderListActivity.java
index 725ede3e..b6e67331 100644
--- a/app/src/production/java/se/leap/bitmaskclient/ProviderListActivity.java
+++ b/app/src/production/java/se/leap/bitmaskclient/ProviderListActivity.java
@@ -67,6 +67,7 @@ public class ProviderListActivity extends ProviderListBaseActivity {
 
     @Override
     public void retrySetUpProvider(@NonNull Provider provider) {
+        mConfigState.setAction(SETTING_UP_PROVIDER);
         ProviderAPICommand.execute(this, SET_UP_PROVIDER, provider);
     }