Option edits are lost whenever the options screen is recreated

app/src/main/java/com/httrack/android/HTTrackActivity.java

@@ -123,6 +123,12 @@ public class HTTrackActivity extends FragmentActivity {
   protected static final int LAYOUT_MIRROR_PROGRESS = 3;
   protected static final int LAYOUT_FINISHED = 4;
 
+  // The options map: carried on the intent both ways, and saved in either activity's bundle.
+  protected static final String MAP_NAME = "com.httrack.android.map";
+  // Build stamp of a bundle; another build's R.id values key that same map differently.
+  protected static final String VERSION_CODE_NAME = "com.httrack.android.version";
+  protected static final String PANE_NAME = "com.httrack.android.pane_id";
+
   // Preferences
   protected static final String PREFS_NAME = "HTTrackPreferences";
   protected static final String BASE_NAME = "BasePath";
@@ -669,6 +675,16 @@ protected boolean ensureExternalStorage() {
     }
   }
 
+  /** This build's own PackageInfo; not finding our own package is unrecoverable. **/
+  protected static PackageInfo packageInfo(final Context context) {
+    try {
+      return context.getPackageManager().getPackageInfo(
+          context.getPackageName(), 0);
+    } catch (final NameNotFoundException e) {
+      throw new RuntimeException(e);
+    }
+  }
+
   @Override
   protected void onCreate(final Bundle savedInstanceState) {
     Log.d(getClass().getSimpleName(), "onCreate");
@@ -686,14 +702,9 @@ protected void onCreate(final Bundle savedInstanceState) {
     }
 
     // Android package version code
-    try {
-      final PackageInfo info = getPackageManager().getPackageInfo(
-          getPackageName(), 0);
-      versionCode = info.versionCode;
-      versionName = info.versionName;
-    } catch (final NameNotFoundException e) {
-      throw new RuntimeException(e);
-    }
+    final PackageInfo info = packageInfo(this);
+    versionCode = info.versionCode;
+    versionName = info.versionName;
 
     // Compute target directory on external storage
     ensureExternalStorage();
@@ -2639,7 +2650,7 @@ public void onClickOptions(final View view) {
     // Then start new activity
     final Intent intent = new Intent(this, OptionsActivity.class);
     fillExtra(intent);
-    intent.putExtra("com.httrack.android.map", mapper.serialize());
+    intent.putExtra(MAP_NAME, mapper.serialize());
     Log.d(getClass().getSimpleName(), "map size: " + mapper.size());
     startActivityForResult(intent, ACTIVITY_OPTIONS);
   }
@@ -2678,7 +2689,7 @@ protected void onActivityResult(final int requestCode, final int resultCode,
     case ACTIVITY_OPTIONS:
       if (resultCode == Activity.RESULT_OK) {
         // Load modified map
-        loadParcelable(data.getParcelableExtra("com.httrack.android.map"));
+        loadParcelable(data.getParcelableExtra(MAP_NAME));
       }
       break;
     case ACTIVITY_FILE_CHOOSER:
@@ -2936,16 +2947,16 @@ protected void saveInstanceState(final Bundle outState) {
     outState.putString("com.httrack.android.sessionID", sessionID);
 
     // Version ID
-    outState.putInt("com.httrack.android.version", versionCode);
+    outState.putInt(VERSION_CODE_NAME, versionCode);
 
     // Map keys
-    outState.putParcelable("com.httrack.android.map", mapper.serialize());
+    outState.putParcelable(MAP_NAME, mapper.serialize());
 
     // Which project's profile the map holds, so the reload guard survives recreation.
     outState.putString("com.httrack.android.loadedProjectName", loadedProjectName);
 
     // Current pane
-    outState.putInt("com.httrack.android.pane_id", pane_id);
+    outState.putInt(PANE_NAME, pane_id);
 
     // Current focus id
     outState.putIntArray("com.httrack.android.focus_id", getCurrentFocusId());
@@ -3059,8 +3070,7 @@ protected void sendSystemNotification(final CharSequence title,
   /** Restore a saved instance state. **/
   protected void restoreInstanceState(final Bundle savedInstanceState) {
     // Check version ID
-    final int version = savedInstanceState
-        .getInt("com.httrack.android.version");
+    final int version = savedInstanceState.getInt(VERSION_CODE_NAME);
     if (version != versionCode) {
       Log.d(getClass().getSimpleName(), "refused bundle version " + version);
       return;
@@ -3070,15 +3080,14 @@ protected void restoreInstanceState(final Bundle savedInstanceState) {
     sessionID = savedInstanceState.getString("com.httrack.android.sessionID");
 
     // Switch pane id
-    final int id = savedInstanceState.getInt("com.httrack.android.pane_id");
+    final int id = savedInstanceState.getInt(PANE_NAME);
 
     // Current focus
     final int[] focus_ids = savedInstanceState
         .getIntArray("com.httrack.android.focus_id");
 
     // Load map
-    final Parcelable data = savedInstanceState
-        .getParcelable("com.httrack.android.map");
+    final Parcelable data = savedInstanceState.getParcelable(MAP_NAME);
 
     // Load map
     if (data != null) {

app/src/main/java/com/httrack/android/OptionsActivity.java

@@ -54,7 +54,8 @@
  * FragmentActivity rather than Activity: predictive back is dispatched through the AndroidX
  * OnBackPressedDispatcher, which a plain Activity does not have.
  */
-public class OptionsActivity extends FragmentActivity implements View.OnClickListener {
+public class OptionsActivity extends FragmentActivity implements
+    View.OnClickListener, OptionsInstanceState.Screen {
   /* List of all tabs. */
   @SuppressWarnings("unchecked")
   protected static Class<? extends Tab>[] tabClasses = new Class[] {
@@ -84,6 +85,9 @@ public class OptionsActivity extends FragmentActivity implements View.OnClickLis
   // use large screen ? (tablets)
   protected boolean isTabletMode;
 
+  // Build this instance belongs to, stamped on the bundle it saves
+  protected int versionCode;
+
   /**
    * The tab activit(ies) common interface.
    */
@@ -370,6 +374,8 @@ private void setViewMenu() {
   protected void onCreate(final Bundle savedInstanceState) {
     super.onCreate(savedInstanceState);
 
+    versionCode = HTTrackActivity.packageInfo(this).versionCode;
+
     getOnBackPressedDispatcher().addCallback(this, backCallback);
 
     // Large screen ? Enable special tablet features in such case...
@@ -396,14 +402,77 @@ protected void onCreate(final Bundle savedInstanceState) {
     // Pinned to Parcelable: inlined, T infers as File & Parcelable and unserialize(File)
     // matches just as well, which javac rejects as ambiguous.
     final Parcelable savedMap =
-        getIntent().getParcelableExtra("com.httrack.android.map");
+        getIntent().getParcelableExtra(HTTrackActivity.MAP_NAME);
     mapper.unserialize(savedMap);
     Log.d(getClass().getSimpleName(), "map size: " + mapper.size());
 
     // Create tabs
     setViewMenu();
   }
 
+  @Override
+  public void flushVisibleTab() {
+    saveIfNeeded();
+  }
+
+  @Override
+  public Parcelable serializeMap() {
+    return mapper.serialize();
+  }
+
+  @Override
+  public void unserializeMap(final Parcelable map) {
+    mapper.unserialize(map);
+  }
+
+  /** Index of CLS in tabClasses, NO_PANE when it names no tab. **/
+  protected static int paneIndexOf(final Class<?> cls) {
+    for (int i = 0; i < tabClasses.length; i++) {
+      if (tabClasses[i] == cls) {
+        return i;
+      }
+    }
+    return OptionsInstanceState.NO_PANE;
+  }
+
+  /** Number of tabs, as a static so a test can pin it without an Activity. **/
+  protected static int tabCount() {
+    return tabClasses.length;
+  }
+
+  @Override
+  public int paneCount() {
+    return tabCount();
+  }
+
+  @Override
+  public int visiblePane() {
+    return paneIndexOf(activityClass);
+  }
+
+  @Override
+  public void openPane(final int index) {
+    setPane(index);
+  }
+
+  @Override
+  protected void onSaveInstanceState(final Bundle outState) {
+    Log.d(getClass().getSimpleName(), "onSaveInstanceState");
+    super.onSaveInstanceState(outState);
+    OptionsInstanceState.save(this, new OptionsInstanceState.BundleStore(
+        outState), versionCode);
+  }
+
+  @Override
+  protected void onRestoreInstanceState(final Bundle savedInstanceState) {
+    Log.d(getClass().getSimpleName(), "onRestoreInstanceState");
+    super.onRestoreInstanceState(savedInstanceState);
+    if (!OptionsInstanceState.restore(this,
+        new OptionsInstanceState.BundleStore(savedInstanceState), versionCode)) {
+      Log.d(getClass().getSimpleName(), "refused bundle");
+    }
+  }
+
   /*
    * Map getter.
    */
@@ -425,7 +494,7 @@ public void finish() {
 
     // Declare result
     final Intent intent = new Intent();
-    intent.putExtra("com.httrack.android.map", mapper.serialize());
+    intent.putExtra(HTTrackActivity.MAP_NAME, mapper.serialize());
     setResult(Activity.RESULT_OK, intent);
     super.finish();
   }

app/src/main/java/com/httrack/android/OptionsInstanceState.java

@@ -0,0 +1,131 @@
+/*
+HTTrack Android Java Interface.
+
+HTTrack Website Copier, Offline Browser for Windows and Unix
+Copyright (C) Xavier Roche and other contributors
+
+This program is free software; you can redistribute it and/or
+modify it under the terms of the GNU General Public License
+as published by the Free Software Foundation; either version 3
+of the License, or any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with this program; if not, write to the Free Software
+Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
+ */
+
+package com.httrack.android;
+
+import android.os.Bundle;
+import android.os.Parcelable;
+
+/**
+ * What the options screen saves across a recreation, kept out of OptionsActivity so both
+ * directions run over seams: production stores into a Bundle, the tests into a plain map.
+ */
+final class OptionsInstanceState {
+  /** No tab is open: the menu is showing. **/
+  static final int NO_PANE = -1;
+
+  private OptionsInstanceState() {
+  }
+
+  /** The bundle slots the state occupies. **/
+  interface Store {
+    void putInt(final String key, final int value);
+
+    int getInt(final String key, final int defaultValue);
+
+    void putParcelable(final String key, final Parcelable value);
+
+    Parcelable getParcelable(final String key);
+  }
+
+  /** The options screen the state is taken from and given back to. **/
+  interface Screen {
+    /** Flush the visible tab's widgets into the map, which is what gets saved. **/
+    void flushVisibleTab();
+
+    Parcelable serializeMap();
+
+    void unserializeMap(final Parcelable map);
+
+    /** Number of tabs, so a saved index naming none can be refused. **/
+    int paneCount();
+
+    /** Index in tabClasses of the open tab, NO_PANE on the menu. **/
+    int visiblePane();
+
+    void openPane(final int index);
+  }
+
+  /** Store over the real thing. **/
+  static final class BundleStore implements Store {
+    private final Bundle bundle;
+
+    BundleStore(final Bundle bundle) {
+      this.bundle = bundle;
+    }
+
+    @Override
+    public void putInt(final String key, final int value) {
+      bundle.putInt(key, value);
+    }
+
+    @Override
+    public int getInt(final String key, final int defaultValue) {
+      return bundle.getInt(key, defaultValue);
+    }
+
+    @Override
+    public void putParcelable(final String key, final Parcelable value) {
+      bundle.putParcelable(key, value);
+    }
+
+    @Override
+    public Parcelable getParcelable(final String key) {
+      return bundle.getParcelable(key);
+    }
+  }
+
+  /** Save what it takes to re-open SCREEN as the user left it. **/
+  static void save(final Screen screen, final Store store,
+      final int versionCode) {
+    // Edits on the visible tab only reach the map when that tab is left.
+    screen.flushVisibleTab();
+
+    store.putInt(HTTrackActivity.VERSION_CODE_NAME, versionCode);
+    // The live map, as HTTrackActivity does; safe only because the restoring mapper is a new one.
+    store.putParcelable(HTTrackActivity.MAP_NAME, screen.serializeMap());
+    store.putInt(HTTrackActivity.PANE_NAME, screen.visiblePane());
+  }
+
+  /** Restore SCREEN from STORE; false when the bundle held nothing usable. **/
+  static boolean restore(final Screen screen, final Store store,
+      final int versionCode) {
+    // Another build renumbers R.id, so its map keys and pane index name other fields and tabs.
+    if (store.getInt(HTTrackActivity.VERSION_CODE_NAME, 0) != versionCode) {
+      return false;
+    }
+
+    // Without a map, keep the one onCreate took from the intent.
+    final Parcelable map = store.getParcelable(HTTrackActivity.MAP_NAME);
+    if (map == null) {
+      return false;
+    }
+
+    // The map first: opening a tab loads its fields from it.
+    screen.unserializeMap(map);
+    // A rebuild at the same versionCode may drop or reorder tabs, so the index can name none.
+    final int pane = store.getInt(HTTrackActivity.PANE_NAME, NO_PANE);
+    if (pane >= 0 && pane < screen.paneCount()) {
+      screen.openPane(pane);
+    }
+    return true;
+  }
+}