From d9c0b54497c3fbcb2bbde62ca1c2cb5cdf46b39b Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Mon, 17 Aug 2026 17:54:19 +0200 Subject: [PATCH 1/5] Keep option edits across an activity recreation OptionsActivity rebuilt its state from the launching intent on every recreation, so a rotation or a low-memory kill while the user was off in Help or the file chooser reverted every unsaved edit and handed the stale map back on Back. Save the option map and the open tab into the instance bundle and read them back, using the same hooks HTTrackActivity has. The visible tab is flushed into the map first, because a tab's widgets only reach the map when that tab is left; restore goes through setPane(), which covers the tablet detail pane as well as the phone path. Closes #129 Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- .../com/httrack/android/OptionsActivity.java | 57 +++++++++++ .../android/OptionsInstanceStateTest.java | 94 +++++++++++++++++++ 2 files changed, 151 insertions(+) create mode 100644 app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java diff --git a/app/src/main/java/com/httrack/android/OptionsActivity.java b/app/src/main/java/com/httrack/android/OptionsActivity.java index 2d90aeec..0a0393fb 100755 --- a/app/src/main/java/com/httrack/android/OptionsActivity.java +++ b/app/src/main/java/com/httrack/android/OptionsActivity.java @@ -404,6 +404,63 @@ protected void onCreate(final Bundle savedInstanceState) { setViewMenu(); } + /** Index of a tab class in tabClasses, -1 when the menu is shown. */ + protected static int paneIndexOf(final Class cls) { + for (int i = 0; i < tabClasses.length; i++) { + if (tabClasses[i] == cls) { + return i; + } + } + return -1; + } + + /** Whether index designates a tab; a bundle from another build may name one we no longer have. */ + protected static boolean isPaneIndex(final int index) { + return index >= 0 && index < tabClasses.length; + } + + /** Save instance state. **/ + protected void saveInstanceState(final Bundle outState) { + // Edits on the visible tab only reach the map when that tab is left. + saveIfNeeded(); + + outState.putParcelable("com.httrack.android.map", mapper.serialize()); + outState.putInt("com.httrack.android.pane_id", paneIndexOf(activityClass)); + } + + @Override + protected void onSaveInstanceState(final Bundle outState) { + Log.d(getClass().getSimpleName(), "onSaveInstanceState"); + super.onSaveInstanceState(outState); + saveInstanceState(outState); + } + + /** Restore a saved instance state. **/ + protected void restoreInstanceState(final Bundle savedInstanceState) { + final Parcelable data = savedInstanceState + .getParcelable("com.httrack.android.map"); + + // Nothing saved: keep the map onCreate took from the intent. + if (data == null) { + return; + } + mapper.unserialize(data); + + // Re-open the tab the user was on; setPane() reloads its fields from the restored map. + final int pane = savedInstanceState + .getInt("com.httrack.android.pane_id", -1); + if (isPaneIndex(pane)) { + setPane(pane); + } + } + + @Override + protected void onRestoreInstanceState(final Bundle savedInstanceState) { + Log.d(getClass().getSimpleName(), "onRestoreInstanceState"); + super.onRestoreInstanceState(savedInstanceState); + restoreInstanceState(savedInstanceState); + } + /* * Map getter. */ diff --git a/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java b/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java new file mode 100644 index 00000000..46f5b359 --- /dev/null +++ b/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java @@ -0,0 +1,94 @@ +package com.httrack.android; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.util.LinkedHashSet; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.junit.Test; + +/** + * OptionsActivity's instance-state hooks (issue #129): without them, any recreation reverted + * every unsaved option edit. The Bundle half is read from the source, since neither Bundle nor + * the activity can be instantiated against the stub android.jar. + */ +public class OptionsInstanceStateTest { + private static final Pattern BUNDLE_KEY = Pattern + .compile("\"(com\\.httrack\\.android\\.\\w+)\""); + + /** Body of an OptionsActivity method, up to the closing brace at method indent. */ + private static String body(final String signature) throws IOException { + final String source = TestSources.javaSource("OptionsActivity"); + final int start = source.indexOf(signature); + assertTrue("not declared: " + signature, start >= 0); + final int end = source.indexOf("\n }", start); + assertTrue("unterminated: " + signature, end > start); + return source.substring(start, end); + } + + private static Set bundleKeys(final String body) { + final Set keys = new LinkedHashSet(); + final Matcher m = BUNDLE_KEY.matcher(body); + while (m.find()) { + keys.add(m.group(1)); + } + return keys; + } + + @Test + public void everyTabRoundTripsThroughItsPaneIndex() { + for (final Class cls : OptionsActivity.tabClasses) { + final int index = OptionsActivity.paneIndexOf(cls); + assertTrue("no pane index: " + cls.getSimpleName(), + OptionsActivity.isPaneIndex(index)); + assertEquals(cls, OptionsActivity.tabClasses[index]); + } + } + + @Test + public void theMenuIsNotAPane() { + // activityClass is null on the menu, and an index from another build must not reopen a tab. + assertEquals(-1, OptionsActivity.paneIndexOf(null)); + assertFalse(OptionsActivity.isPaneIndex(-1)); + assertFalse(OptionsActivity.isPaneIndex(OptionsActivity.tabClasses.length)); + } + + @Test + public void bothLifecycleHooksAreOverridden() throws IOException { + assertTrue("no onSaveInstanceState", + body("protected void onSaveInstanceState(").contains( + "saveInstanceState(outState)")); + assertTrue("no onRestoreInstanceState", + body("protected void onRestoreInstanceState(").contains( + "restoreInstanceState(savedInstanceState)")); + } + + @Test + public void theVisibleTabIsFlushedBeforeTheMapIsSerialized() throws IOException { + // A tab's widgets only reach the map when that tab is left. + final String saved = body("protected void saveInstanceState("); + assertTrue("no flush", saved.contains("saveIfNeeded()")); + assertTrue("flushed after serializing", + saved.indexOf("saveIfNeeded()") < saved.indexOf("mapper.serialize()")); + } + + @Test + public void restoreReloadsTheMapAndTheOpenTab() throws IOException { + final String restored = body("protected void restoreInstanceState("); + assertTrue("map not restored", restored.contains("mapper.unserialize(")); + assertTrue("open tab not restored", restored.contains("setPane(")); + } + + @Test + public void saveAndRestoreAgreeOnEveryBundleKey() throws IOException { + // A key written but never read loses that state silently; nothing else pairs them up. + final Set written = bundleKeys(body("protected void saveInstanceState(")); + assertFalse("nothing saved", written.isEmpty()); + assertEquals(written, bundleKeys(body("protected void restoreInstanceState("))); + } +} From 3be2bcea3b866b277b65f7bb48df91054a7b48b2 Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Mon, 17 Aug 2026 18:08:12 +0200 Subject: [PATCH 2/5] Pair the instance state keys, and test the pane decision for real The bundle keys were literals repeated on the write and the read side, so a mismatch could only be caught by scanning the source; name them instead and let the compiler pair them. That test is dropped rather than strengthened. Fold the "no map, restore nothing" and stale-index rules into paneToRestore(), the one part of the round trip that can run under the stub android.jar, and test it against real values. Neither Bundle nor SparseArray is mocked there, so the map round trip itself stays source-read. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- .../com/httrack/android/OptionsActivity.java | 35 ++++++---- .../android/OptionsInstanceStateTest.java | 70 +++++++++---------- 2 files changed, 55 insertions(+), 50 deletions(-) diff --git a/app/src/main/java/com/httrack/android/OptionsActivity.java b/app/src/main/java/com/httrack/android/OptionsActivity.java index 0a0393fb..b2302c34 100755 --- a/app/src/main/java/com/httrack/android/OptionsActivity.java +++ b/app/src/main/java/com/httrack/android/OptionsActivity.java @@ -62,6 +62,10 @@ public class OptionsActivity extends FragmentActivity implements View.OnClickLis LinksTab.class, BuildTab.class, BrowserId.class, Spider.class, Proxy.class, LogIndexCache.class, MimeDefs.class, ExpertsOnly.class }; + /* Instance state keys; naming them pairs each write with its read. */ + private static final String KEY_MAP = "com.httrack.android.map"; + private static final String KEY_PANE = "com.httrack.android.pane_id"; + /* List of all tabs instances. */ protected Tab[] tabInstances; @@ -414,18 +418,22 @@ protected static int paneIndexOf(final Class cls) { return -1; } - /** Whether index designates a tab; a bundle from another build may name one we no longer have. */ + /** Whether index designates a tab; a bundle from another build can name one we no longer have. */ protected static boolean isPaneIndex(final int index) { return index >= 0 && index < tabClasses.length; } - /** Save instance state. **/ + /** Pane to re-open on restore, -1 for none: a mapless bundle or a stale index leaves the menu. */ + protected static int paneToRestore(final boolean hasMap, final int savedPane) { + return hasMap && isPaneIndex(savedPane) ? savedPane : -1; + } + protected void saveInstanceState(final Bundle outState) { // Edits on the visible tab only reach the map when that tab is left. saveIfNeeded(); - outState.putParcelable("com.httrack.android.map", mapper.serialize()); - outState.putInt("com.httrack.android.pane_id", paneIndexOf(activityClass)); + outState.putParcelable(KEY_MAP, mapper.serialize()); + outState.putInt(KEY_PANE, paneIndexOf(activityClass)); } @Override @@ -435,21 +443,18 @@ protected void onSaveInstanceState(final Bundle outState) { saveInstanceState(outState); } - /** Restore a saved instance state. **/ protected void restoreInstanceState(final Bundle savedInstanceState) { - final Parcelable data = savedInstanceState - .getParcelable("com.httrack.android.map"); + final Parcelable data = savedInstanceState.getParcelable(KEY_MAP); + final int pane = paneToRestore(data != null, + savedInstanceState.getInt(KEY_PANE, -1)); - // Nothing saved: keep the map onCreate took from the intent. - if (data == null) { - return; + // Without a map, keep the one onCreate took from the intent. + if (data != null) { + mapper.unserialize(data); } - mapper.unserialize(data); - // Re-open the tab the user was on; setPane() reloads its fields from the restored map. - final int pane = savedInstanceState - .getInt("com.httrack.android.pane_id", -1); - if (isPaneIndex(pane)) { + // Re-open the tab the user was on. + if (pane != -1) { setPane(pane); } } diff --git a/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java b/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java index 46f5b359..44bc35f0 100644 --- a/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java +++ b/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java @@ -5,22 +5,15 @@ import static org.junit.Assert.assertTrue; import java.io.IOException; -import java.util.LinkedHashSet; -import java.util.Set; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import org.junit.Test; /** * OptionsActivity's instance-state hooks (issue #129): without them, any recreation reverted - * every unsaved option edit. The Bundle half is read from the source, since neither Bundle nor - * the activity can be instantiated against the stub android.jar. + * every unsaved option edit. Only the pane decisions run here; neither Bundle nor SparseArray + * is mocked by the stub android.jar, so the map round trip itself is read from the source. */ public class OptionsInstanceStateTest { - private static final Pattern BUNDLE_KEY = Pattern - .compile("\"(com\\.httrack\\.android\\.\\w+)\""); - /** Body of an OptionsActivity method, up to the closing brace at method indent. */ private static String body(final String signature) throws IOException { final String source = TestSources.javaSource("OptionsActivity"); @@ -31,33 +24,46 @@ private static String body(final String signature) throws IOException { return source.substring(start, end); } - private static Set bundleKeys(final String body) { - final Set keys = new LinkedHashSet(); - final Matcher m = BUNDLE_KEY.matcher(body); - while (m.find()) { - keys.add(m.group(1)); - } - return keys; - } - @Test - public void everyTabRoundTripsThroughItsPaneIndex() { - for (final Class cls : OptionsActivity.tabClasses) { - final int index = OptionsActivity.paneIndexOf(cls); - assertTrue("no pane index: " + cls.getSimpleName(), - OptionsActivity.isPaneIndex(index)); - assertEquals(cls, OptionsActivity.tabClasses[index]); + public void eachTabHasItsOwnPaneIndex() { + for (int i = 0; i < OptionsActivity.tabClasses.length; i++) { + // A tab listed twice would report the first occurrence for both. + assertEquals(i, OptionsActivity.paneIndexOf(OptionsActivity.tabClasses[i])); } } @Test public void theMenuIsNotAPane() { - // activityClass is null on the menu, and an index from another build must not reopen a tab. + // activityClass is null on the menu. assertEquals(-1, OptionsActivity.paneIndexOf(null)); assertFalse(OptionsActivity.isPaneIndex(-1)); assertFalse(OptionsActivity.isPaneIndex(OptionsActivity.tabClasses.length)); } + @Test + public void restoreReopensTheTabTheUserWasOn() { + for (int i = 0; i < OptionsActivity.tabClasses.length; i++) { + assertEquals(i, OptionsActivity.paneToRestore(true, i)); + } + } + + @Test + public void aBundleWithoutAMapRestoresNothing() { + // The map is what the pane's fields are loaded from, so a pane alone must not reopen a tab. + assertEquals(-1, OptionsActivity.paneToRestore(false, 0)); + assertEquals(-1, OptionsActivity.paneToRestore(false, + OptionsActivity.tabClasses.length - 1)); + } + + @Test + public void aStalePaneIndexLeavesTheMenu() { + assertEquals(-1, OptionsActivity.paneToRestore(true, -1)); + assertEquals(-1, OptionsActivity.paneToRestore(true, + OptionsActivity.tabClasses.length)); + assertEquals(-1, OptionsActivity.paneToRestore(true, Integer.MAX_VALUE)); + assertEquals(-1, OptionsActivity.paneToRestore(true, Integer.MIN_VALUE)); + } + @Test public void bothLifecycleHooksAreOverridden() throws IOException { assertTrue("no onSaveInstanceState", @@ -78,17 +84,11 @@ public void theVisibleTabIsFlushedBeforeTheMapIsSerialized() throws IOException } @Test - public void restoreReloadsTheMapAndTheOpenTab() throws IOException { + public void theRestoredMapIsInPlaceBeforeTheTabIsReopened() throws IOException { + // setPane() loads the tab's fields from the map, so a reopen first would load the stale one. final String restored = body("protected void restoreInstanceState("); assertTrue("map not restored", restored.contains("mapper.unserialize(")); - assertTrue("open tab not restored", restored.contains("setPane(")); - } - - @Test - public void saveAndRestoreAgreeOnEveryBundleKey() throws IOException { - // A key written but never read loses that state silently; nothing else pairs them up. - final Set written = bundleKeys(body("protected void saveInstanceState(")); - assertFalse("nothing saved", written.isEmpty()); - assertEquals(written, bundleKeys(body("protected void restoreInstanceState("))); + assertTrue("tab reopened before the map was restored", + restored.indexOf("mapper.unserialize(") < restored.indexOf("setPane(")); } } From b8c1c636ff27698ce4b698d507b46490f7c057c0 Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Mon, 17 Aug 2026 19:20:04 +0200 Subject: [PATCH 3/5] Refuse an options bundle from another build, and test the round trip A saved options bundle carried no build stamp, so a bundle written before a Play update was restored into the new build, where R.id has been renumbered: the map came back keyed by the old build's ints and finish() handed it to HTTrackActivity, which can write it to the project profile. Stamp the bundle with versionCode and refuse it on mismatch, mirroring what HTTrackActivity already does. The pane bounds check goes with it -- a stale index can no longer arrive, and it could never have caught a reordered tab list anyway. The save/restore pair moves to OptionsInstanceState, behind two seams: a Store over the Bundle and a Screen over the activity. Both directions now run in a plain JUnit test, over a HashMap Store that answers the default on a type mismatch the way Bundle does, so the key constants are load-bearing there. Dropping the map write, swapping the map and pane keys, and dropping the version guard each fail it. The three tests that only matched source text are gone, except the one holding the two Activity overrides to calling in: no seam reaches those. Also names the map's intent-extra key, shared by both ends of the contract, and lifts the PackageInfo lookup out of HTTrackActivity.onCreate. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- .../com/httrack/android/HTTrackActivity.java | 45 ++-- .../com/httrack/android/OptionsActivity.java | 80 +++---- .../httrack/android/OptionsInstanceState.java | 127 +++++++++++ .../android/OptionsInstanceStateTest.java | 210 +++++++++++++----- 4 files changed, 348 insertions(+), 114 deletions(-) create mode 100644 app/src/main/java/com/httrack/android/OptionsInstanceState.java diff --git a/app/src/main/java/com/httrack/android/HTTrackActivity.java b/app/src/main/java/com/httrack/android/HTTrackActivity.java index 87a786ca..34814960 100755 --- a/app/src/main/java/com/httrack/android/HTTrackActivity.java +++ b/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) { diff --git a/app/src/main/java/com/httrack/android/OptionsActivity.java b/app/src/main/java/com/httrack/android/OptionsActivity.java index b2302c34..033d3f05 100755 --- a/app/src/main/java/com/httrack/android/OptionsActivity.java +++ b/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[] tabClasses = new Class[] { @@ -62,10 +63,6 @@ public class OptionsActivity extends FragmentActivity implements View.OnClickLis LinksTab.class, BuildTab.class, BrowserId.class, Spider.class, Proxy.class, LogIndexCache.class, MimeDefs.class, ExpertsOnly.class }; - /* Instance state keys; naming them pairs each write with its read. */ - private static final String KEY_MAP = "com.httrack.android.map"; - private static final String KEY_PANE = "com.httrack.android.pane_id"; - /* List of all tabs instances. */ protected Tab[] tabInstances; @@ -88,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. */ @@ -374,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... @@ -400,7 +402,7 @@ 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()); @@ -408,62 +410,52 @@ protected void onCreate(final Bundle savedInstanceState) { setViewMenu(); } - /** Index of a tab class in tabClasses, -1 when the menu is shown. */ - protected static int paneIndexOf(final Class cls) { - for (int i = 0; i < tabClasses.length; i++) { - if (tabClasses[i] == cls) { - return i; - } - } - return -1; + @Override + public void flushVisibleTab() { + saveIfNeeded(); } - /** Whether index designates a tab; a bundle from another build can name one we no longer have. */ - protected static boolean isPaneIndex(final int index) { - return index >= 0 && index < tabClasses.length; + @Override + public Parcelable serializeMap() { + return mapper.serialize(); } - /** Pane to re-open on restore, -1 for none: a mapless bundle or a stale index leaves the menu. */ - protected static int paneToRestore(final boolean hasMap, final int savedPane) { - return hasMap && isPaneIndex(savedPane) ? savedPane : -1; + @Override + public void unserializeMap(final Parcelable map) { + mapper.unserialize(map); } - protected void saveInstanceState(final Bundle outState) { - // Edits on the visible tab only reach the map when that tab is left. - saveIfNeeded(); + @Override + public int visiblePane() { + for (int i = 0; i < tabClasses.length; i++) { + if (tabClasses[i] == activityClass) { + return i; + } + } + return OptionsInstanceState.NO_PANE; + } - outState.putParcelable(KEY_MAP, mapper.serialize()); - outState.putInt(KEY_PANE, 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); - saveInstanceState(outState); - } - - protected void restoreInstanceState(final Bundle savedInstanceState) { - final Parcelable data = savedInstanceState.getParcelable(KEY_MAP); - final int pane = paneToRestore(data != null, - savedInstanceState.getInt(KEY_PANE, -1)); - - // Without a map, keep the one onCreate took from the intent. - if (data != null) { - mapper.unserialize(data); - } - - // Re-open the tab the user was on. - if (pane != -1) { - setPane(pane); - } + OptionsInstanceState.save(this, new OptionsInstanceState.BundleStore( + outState), versionCode); } @Override protected void onRestoreInstanceState(final Bundle savedInstanceState) { Log.d(getClass().getSimpleName(), "onRestoreInstanceState"); super.onRestoreInstanceState(savedInstanceState); - restoreInstanceState(savedInstanceState); + if (!OptionsInstanceState.restore(this, + new OptionsInstanceState.BundleStore(savedInstanceState), versionCode)) { + Log.d(getClass().getSimpleName(), "refused bundle"); + } } /* @@ -487,7 +479,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(); } diff --git a/app/src/main/java/com/httrack/android/OptionsInstanceState.java b/app/src/main/java/com/httrack/android/OptionsInstanceState.java new file mode 100644 index 00000000..88924663 --- /dev/null +++ b/app/src/main/java/com/httrack/android/OptionsInstanceState.java @@ -0,0 +1,127 @@ +/* +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. + */ +public final class OptionsInstanceState { + /** No tab is open: the menu is showing. */ + public static final int NO_PANE = -1; + + private OptionsInstanceState() { + } + + /** The bundle slots the state occupies. **/ + public 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. **/ + public 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); + + /** Index in tabClasses of the open tab, NO_PANE on the menu. */ + int visiblePane(); + + void openPane(final int index); + } + + /** Store over the real thing. **/ + public static class BundleStore implements Store { + private final Bundle bundle; + + public 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. **/ + public 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. **/ + public 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); + final int pane = store.getInt(HTTrackActivity.PANE_NAME, NO_PANE); + if (pane != NO_PANE) { + screen.openPane(pane); + } + return true; + } +} diff --git a/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java b/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java index 44bc35f0..fba88d77 100644 --- a/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java +++ b/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java @@ -2,93 +2,199 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.junit.Test; +import android.os.Parcel; +import android.os.Parcelable; + /** - * OptionsActivity's instance-state hooks (issue #129): without them, any recreation reverted - * every unsaved option edit. Only the pane decisions run here; neither Bundle nor SparseArray - * is mocked by the stub android.jar, so the map round trip itself is read from the source. + * OptionsActivity's instance state (issue #129): without it, any recreation reverted every + * unsaved option edit. Both directions run here, over the seams the activity itself uses. */ public class OptionsInstanceStateTest { - /** Body of an OptionsActivity method, up to the closing brace at method indent. */ - private static String body(final String signature) throws IOException { - final String source = TestSources.javaSource("OptionsActivity"); - final int start = source.indexOf(signature); - assertTrue("not declared: " + signature, start >= 0); - final int end = source.indexOf("\n }", start); - assertTrue("unterminated: " + signature, end > start); - return source.substring(start, end); + private static final int VERSION = 95; + + /** Stands in for the serialized map; only its identity is looked at. */ + private static class FakeMap implements Parcelable { + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel(final Parcel dest, final int flags) { + } + } + + private static class FakeStore implements OptionsInstanceState.Store { + private final Map values = new HashMap(); + + @Override + public void putInt(final String key, final int value) { + values.put(key, value); + } + + // Like Bundle, answer the default when the slot holds another type: that is how a getter + // reaching for the wrong key stays silent. + @Override + public int getInt(final String key, final int defaultValue) { + final Object value = values.get(key); + return value instanceof Integer ? Integer.class.cast(value) + : defaultValue; + } + + @Override + public void putParcelable(final String key, final Parcelable value) { + values.put(key, value); + } + + @Override + public Parcelable getParcelable(final String key) { + final Object value = values.get(key); + return value instanceof Parcelable ? Parcelable.class.cast(value) : null; + } + } + + /** Screen recording what was asked of it, in order. */ + private static class FakeScreen implements OptionsInstanceState.Screen { + private final List calls = new ArrayList(); + private Parcelable map; + private int pane; + private boolean restored; + + FakeScreen(final Parcelable map, final int pane) { + this.map = map; + this.pane = pane; + } + + @Override + public void flushVisibleTab() { + calls.add("flush"); + } + + @Override + public Parcelable serializeMap() { + calls.add("serialize"); + return map; + } + + @Override + public void unserializeMap(final Parcelable map) { + calls.add("unserialize"); + this.map = map; + } + + @Override + public int visiblePane() { + calls.add("visiblePane"); + return pane; + } + + @Override + public void openPane(final int index) { + calls.add("openPane"); + this.pane = index; + } + } + + /** The fresh screen SAVED's state lands in, restoring as build VERSION. */ + private static FakeScreen restore(final FakeScreen saved, final int version) { + final FakeStore store = new FakeStore(); + OptionsInstanceState.save(saved, store, VERSION); + final FakeScreen restored = new FakeScreen(null, + OptionsInstanceState.NO_PANE); + restored.restored = OptionsInstanceState.restore(restored, store, version); + return restored; } @Test - public void eachTabHasItsOwnPaneIndex() { + public void theMapAndTheOpenTabSurviveTheRoundTrip() { for (int i = 0; i < OptionsActivity.tabClasses.length; i++) { - // A tab listed twice would report the first occurrence for both. - assertEquals(i, OptionsActivity.paneIndexOf(OptionsActivity.tabClasses[i])); + final Parcelable map = new FakeMap(); + final FakeScreen restored = restore(new FakeScreen(map, i), VERSION); + assertTrue(restored.restored); + assertSame(map, restored.map); + assertEquals(i, restored.pane); } } @Test - public void theMenuIsNotAPane() { - // activityClass is null on the menu. - assertEquals(-1, OptionsActivity.paneIndexOf(null)); - assertFalse(OptionsActivity.isPaneIndex(-1)); - assertFalse(OptionsActivity.isPaneIndex(OptionsActivity.tabClasses.length)); + public void theMenuComesBackAsTheMenu() { + // No tab was open, so the map is restored but nothing is re-opened over the menu. + final Parcelable map = new FakeMap(); + final FakeScreen restored = restore( + new FakeScreen(map, OptionsInstanceState.NO_PANE), VERSION); + assertSame(map, restored.map); + assertFalse(restored.calls.contains("openPane")); } @Test - public void restoreReopensTheTabTheUserWasOn() { - for (int i = 0; i < OptionsActivity.tabClasses.length; i++) { - assertEquals(i, OptionsActivity.paneToRestore(true, i)); - } + public void aBundleFromAnotherBuildRestoresNothing() { + // R.id values are renumbered by any layout change, so that build's keys name other fields. + final FakeScreen restored = restore(new FakeScreen(new FakeMap(), 0), + VERSION + 1); + assertFalse(restored.restored); + assertNull(restored.map); + assertEquals(OptionsInstanceState.NO_PANE, restored.pane); } @Test public void aBundleWithoutAMapRestoresNothing() { - // The map is what the pane's fields are loaded from, so a pane alone must not reopen a tab. - assertEquals(-1, OptionsActivity.paneToRestore(false, 0)); - assertEquals(-1, OptionsActivity.paneToRestore(false, - OptionsActivity.tabClasses.length - 1)); + final FakeStore store = new FakeStore(); + store.putInt(HTTrackActivity.VERSION_CODE_NAME, VERSION); + store.putInt(HTTrackActivity.PANE_NAME, 0); + final FakeScreen restored = new FakeScreen(null, + OptionsInstanceState.NO_PANE); + assertFalse(OptionsInstanceState.restore(restored, store, VERSION)); + assertFalse(restored.calls.contains("openPane")); } @Test - public void aStalePaneIndexLeavesTheMenu() { - assertEquals(-1, OptionsActivity.paneToRestore(true, -1)); - assertEquals(-1, OptionsActivity.paneToRestore(true, - OptionsActivity.tabClasses.length)); - assertEquals(-1, OptionsActivity.paneToRestore(true, Integer.MAX_VALUE)); - assertEquals(-1, OptionsActivity.paneToRestore(true, Integer.MIN_VALUE)); + public void theVisibleTabIsFlushedBeforeTheMapIsRead() { + // A tab's widgets only reach the map when that tab is left. + final FakeScreen saved = new FakeScreen(new FakeMap(), 0); + OptionsInstanceState.save(saved, new FakeStore(), VERSION); + assertTrue(saved.calls.indexOf("flush") < saved.calls.indexOf("serialize")); } @Test - public void bothLifecycleHooksAreOverridden() throws IOException { - assertTrue("no onSaveInstanceState", - body("protected void onSaveInstanceState(").contains( - "saveInstanceState(outState)")); - assertTrue("no onRestoreInstanceState", - body("protected void onRestoreInstanceState(").contains( - "restoreInstanceState(savedInstanceState)")); + public void theMapIsRestoredBeforeTheTabIsReopened() { + // Opening a tab loads its fields from the map, so a reopen first would load the stale one. + final FakeScreen restored = restore(new FakeScreen(new FakeMap(), 2), + VERSION); + assertTrue(restored.calls.indexOf("unserialize") < restored.calls + .indexOf("openPane")); } @Test - public void theVisibleTabIsFlushedBeforeTheMapIsSerialized() throws IOException { - // A tab's widgets only reach the map when that tab is left. - final String saved = body("protected void saveInstanceState("); - assertTrue("no flush", saved.contains("saveIfNeeded()")); - assertTrue("flushed after serializing", - saved.indexOf("saveIfNeeded()") < saved.indexOf("mapper.serialize()")); + public void noTabIsListedTwice() { + // A duplicate would make two menu entries share one saved pane index. + for (int i = 0; i < OptionsActivity.tabClasses.length; i++) { + for (int j = i + 1; j < OptionsActivity.tabClasses.length; j++) { + assertNotSame(OptionsActivity.tabClasses[i].getName(), + OptionsActivity.tabClasses[i], OptionsActivity.tabClasses[j]); + } + } } @Test - public void theRestoredMapIsInPlaceBeforeTheTabIsReopened() throws IOException { - // setPane() loads the tab's fields from the map, so a reopen first would load the stale one. - final String restored = body("protected void restoreInstanceState("); - assertTrue("map not restored", restored.contains("mapper.unserialize(")); - assertTrue("tab reopened before the map was restored", - restored.indexOf("mapper.unserialize(") < restored.indexOf("setPane(")); + public void bothLifecycleHooksAreOverridden() throws IOException { + // The Activity overrides are the one thing above no seam reaches. + final String source = TestSources.javaSource("OptionsActivity"); + assertTrue("no onSaveInstanceState", + source.contains("OptionsInstanceState.save(this,")); + assertTrue("no onRestoreInstanceState", + source.contains("OptionsInstanceState.restore(this,")); } } From 6d107e0b02c3fb8e4bfe7e5328d3da31df617d9c Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Mon, 17 Aug 2026 19:44:35 +0200 Subject: [PATCH 4/5] Put the pane lookup back where a test can reach it visiblePane() had swallowed the tabClasses loop behind the Screen interface, where only the test's fake stands, so gutting the method to return NO_PANE left the whole suite green. The loop moves back into a static paneIndexOf(Class) the test calls directly. That move had also dropped the bounds check on the restored index, and setPane() indexes tabClasses unguarded, so an index naming no tab threw inside onRestoreInstanceState. The version stamp keeps such a bundle off Play, but not off a rebuild at the same versionCode: an "adb install -r" of a debug build with a reordered tab list. Screen now reports its tab count, and restore() leaves the menu when the saved index falls outside it. Also scope the lifecycle-hook assertions back to the two override bodies, where they no longer pass on a call moved elsewhere in the file, and drop OptionsInstanceState to package-private like its peer StoragePaths. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- .../com/httrack/android/OptionsActivity.java | 16 ++++- .../httrack/android/OptionsInstanceState.java | 28 ++++---- .../android/OptionsInstanceStateTest.java | 72 ++++++++++++++++--- 3 files changed, 91 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/com/httrack/android/OptionsActivity.java b/app/src/main/java/com/httrack/android/OptionsActivity.java index 033d3f05..b7ee2d61 100755 --- a/app/src/main/java/com/httrack/android/OptionsActivity.java +++ b/app/src/main/java/com/httrack/android/OptionsActivity.java @@ -425,16 +425,26 @@ public void unserializeMap(final Parcelable map) { mapper.unserialize(map); } - @Override - public int visiblePane() { + /** 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] == activityClass) { + if (tabClasses[i] == cls) { return i; } } return OptionsInstanceState.NO_PANE; } + @Override + public int paneCount() { + return tabClasses.length; + } + + @Override + public int visiblePane() { + return paneIndexOf(activityClass); + } + @Override public void openPane(final int index) { setPane(index); diff --git a/app/src/main/java/com/httrack/android/OptionsInstanceState.java b/app/src/main/java/com/httrack/android/OptionsInstanceState.java index 88924663..9fe00e0b 100644 --- a/app/src/main/java/com/httrack/android/OptionsInstanceState.java +++ b/app/src/main/java/com/httrack/android/OptionsInstanceState.java @@ -28,15 +28,15 @@ * 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. */ -public final class OptionsInstanceState { - /** No tab is open: the menu is showing. */ - public static final int NO_PANE = -1; +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. **/ - public interface Store { + interface Store { void putInt(final String key, final int value); int getInt(final String key, final int defaultValue); @@ -47,25 +47,28 @@ public interface Store { } /** The options screen the state is taken from and given back to. **/ - public interface Screen { - /** Flush the visible tab's widgets into the map, which is what gets saved. */ + 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); - /** Index in tabClasses of the open tab, NO_PANE on the menu. */ + /** 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. **/ - public static class BundleStore implements Store { + static final class BundleStore implements Store { private final Bundle bundle; - public BundleStore(final Bundle bundle) { + BundleStore(final Bundle bundle) { this.bundle = bundle; } @@ -91,7 +94,7 @@ public Parcelable getParcelable(final String key) { } /** Save what it takes to re-open SCREEN as the user left it. **/ - public static void save(final Screen screen, final Store store, + 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(); @@ -103,7 +106,7 @@ public static void save(final Screen screen, final Store store, } /** Restore SCREEN from STORE; false when the bundle held nothing usable. **/ - public static boolean restore(final Screen screen, final Store store, + 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) { @@ -118,8 +121,9 @@ public static boolean restore(final Screen screen, final Store store, // 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 != NO_PANE) { + if (pane >= 0 && pane < screen.paneCount()) { screen.openPane(pane); } return true; diff --git a/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java b/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java index fba88d77..500b48cf 100644 --- a/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java +++ b/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java @@ -2,7 +2,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotSame; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -95,6 +94,11 @@ public void unserializeMap(final Parcelable map) { this.map = map; } + @Override + public int paneCount() { + return OptionsActivity.tabClasses.length; + } + @Override public int visiblePane() { calls.add("visiblePane"); @@ -118,6 +122,18 @@ private static FakeScreen restore(final FakeScreen saved, final int version) { return restored; } + /** The screen a bundle naming PANE restores onto. */ + private static FakeScreen restorePane(final int pane) { + final FakeStore store = new FakeStore(); + store.putInt(HTTrackActivity.VERSION_CODE_NAME, VERSION); + store.putParcelable(HTTrackActivity.MAP_NAME, new FakeMap()); + store.putInt(HTTrackActivity.PANE_NAME, pane); + final FakeScreen restored = new FakeScreen(null, + OptionsInstanceState.NO_PANE); + restored.restored = OptionsInstanceState.restore(restored, store, VERSION); + return restored; + } + @Test public void theMapAndTheOpenTabSurviveTheRoundTrip() { for (int i = 0; i < OptionsActivity.tabClasses.length; i++) { @@ -178,23 +194,59 @@ public void theMapIsRestoredBeforeTheTabIsReopened() { } @Test - public void noTabIsListedTwice() { - // A duplicate would make two menu entries share one saved pane index. + public void everyTabHasItsOwnPaneIndex() { + // A duplicate would make two menu entries share one index, and reopen the wrong tab. for (int i = 0; i < OptionsActivity.tabClasses.length; i++) { - for (int j = i + 1; j < OptionsActivity.tabClasses.length; j++) { - assertNotSame(OptionsActivity.tabClasses[i].getName(), - OptionsActivity.tabClasses[i], OptionsActivity.tabClasses[j]); - } + assertEquals(OptionsActivity.tabClasses[i].getName(), i, + OptionsActivity.paneIndexOf(OptionsActivity.tabClasses[i])); + } + } + + @Test + public void whatIsNotATabIsNotAPane() { + // activityClass is null on the menu, and never a class outside the list. + assertEquals(OptionsInstanceState.NO_PANE, + OptionsActivity.paneIndexOf(null)); + assertEquals(OptionsInstanceState.NO_PANE, + OptionsActivity.paneIndexOf(OptionsActivity.class)); + } + + @Test + public void aPaneIndexNamingNoTabLeavesTheMenu() { + // A rebuild at the same versionCode passes the version stamp, so the index reaches setPane. + for (final int pane : new int[] { OptionsActivity.tabClasses.length, -2 }) { + final FakeScreen restored = restorePane(pane); + assertTrue("bundle refused for pane " + pane, restored.restored); + assertFalse("reopened pane " + pane, + restored.calls.contains("openPane")); } } + @Test + public void theSavedIndexIsTheVisibleTabs() throws IOException { + // The fake screen answers visiblePane() itself, so only the source pins the real one. + assertTrue("visiblePane() does not use paneIndexOf", + body("public int visiblePane(").contains("paneIndexOf(activityClass)")); + } + @Test public void bothLifecycleHooksAreOverridden() throws IOException { // The Activity overrides are the one thing above no seam reaches. - final String source = TestSources.javaSource("OptionsActivity"); assertTrue("no onSaveInstanceState", - source.contains("OptionsInstanceState.save(this,")); + body("protected void onSaveInstanceState(").contains( + "OptionsInstanceState.save(this,")); assertTrue("no onRestoreInstanceState", - source.contains("OptionsInstanceState.restore(this,")); + body("protected void onRestoreInstanceState(").contains( + "OptionsInstanceState.restore(this,")); + } + + /** Body of an OptionsActivity method, up to the closing brace at method indent. */ + private static String body(final String signature) throws IOException { + final String source = TestSources.javaSource("OptionsActivity"); + final int start = source.indexOf(signature); + assertTrue("not declared: " + signature, start >= 0); + final int end = source.indexOf("\n }", start); + assertTrue("unterminated: " + signature, end > start); + return source.substring(start, end); } } From 22e5ee89786795f5834f43552c6672d1f87f1b1d Mon Sep 17 00:00:00 2001 From: Xavier Roche Date: Mon, 17 Aug 2026 21:00:45 +0200 Subject: [PATCH 5/5] Give the tab count one definition the test can reach The bounds check in restore() leans on OptionsActivity.paneCount(), but the fake screen computed tabClasses.length itself, so a wrong count stayed green in the suite and crashed on rotate. Both sides now go through a static tabCount(), and a test pins it against paneIndexOf(). Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Xavier Roche --- .../com/httrack/android/OptionsActivity.java | 7 ++++++- .../android/OptionsInstanceStateTest.java | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/httrack/android/OptionsActivity.java b/app/src/main/java/com/httrack/android/OptionsActivity.java index b7ee2d61..9af1e663 100755 --- a/app/src/main/java/com/httrack/android/OptionsActivity.java +++ b/app/src/main/java/com/httrack/android/OptionsActivity.java @@ -435,9 +435,14 @@ protected static int paneIndexOf(final Class cls) { 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 tabClasses.length; + return tabCount(); } @Override diff --git a/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java b/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java index 500b48cf..aa016fd0 100644 --- a/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java +++ b/app/src/test/java/com/httrack/android/OptionsInstanceStateTest.java @@ -94,9 +94,10 @@ public void unserializeMap(final Parcelable map) { this.map = map; } + // The real one, so a wrong count fails here instead of only on a device. @Override public int paneCount() { - return OptionsActivity.tabClasses.length; + return OptionsActivity.tabCount(); } @Override @@ -213,7 +214,7 @@ public void whatIsNotATabIsNotAPane() { @Test public void aPaneIndexNamingNoTabLeavesTheMenu() { - // A rebuild at the same versionCode passes the version stamp, so the index reaches setPane. + // An out-of-range index still passes the version guard, but must not reach openPane. for (final int pane : new int[] { OptionsActivity.tabClasses.length, -2 }) { final FakeScreen restored = restorePane(pane); assertTrue("bundle refused for pane " + pane, restored.restored); @@ -222,11 +223,22 @@ public void aPaneIndexNamingNoTabLeavesTheMenu() { } } + @Test + public void theCountStopsAtTheLastPane() { + // restore() bounds the saved index with the count, and setPane throws one past the end. + final int last = OptionsActivity.tabCount() - 1; + assertTrue("the count runs past the last tab", + last < OptionsActivity.tabClasses.length); + assertEquals(last, + OptionsActivity.paneIndexOf(OptionsActivity.tabClasses[last])); + } + @Test public void theSavedIndexIsTheVisibleTabs() throws IOException { // The fake screen answers visiblePane() itself, so only the source pins the real one. assertTrue("visiblePane() does not use paneIndexOf", - body("public int visiblePane(").contains("paneIndexOf(activityClass)")); + body("public int visiblePane(").contains( + "return paneIndexOf(activityClass);")); } @Test