From c7b937c7a5430ec3dc211d01e12158d95e430104 Mon Sep 17 00:00:00 2001 From: zhuinden Date: Fri, 11 Nov 2022 03:25:59 +0100 Subject: [PATCH] 2.6.5: reverse-iterate completion listeners to fix #263 --- CHANGELOG.md | 15 ++++- README.md | 4 +- simple-stack/build.gradle.kts | 2 +- .../zhuinden/simplestack/NavigationCore.java | 10 ++- .../simplestack/BackstackCoreTest.java | 62 +++++++++++++++++++ 5 files changed, 87 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ffa195e..f05484ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,13 +1,24 @@ # Change log +-Simple Stack 2.6.5 (2022-11-11) +-------------------------------- + +- FIX: `Backstack.CompletionListener` added to `Backstack` that unregistered themselves during dispatching notifications + would cause either a ConcurrentModificationException or invalid results, this is now fixed and no longer the case ( + #263, thanks @angusholder) + +- MINOR CHANGE: When `Backstack.CompletionListener`'s are being notified, the state changer is temporarily removed ( + similarly to dispatching `ScopedServices.Activated` events), so that navigation actions invoked on `Backstack` are + deferred until all `Backstack.CompletionListener`s are notified. + -Simple Stack 2.6.4 (2022-04-21) -------------------------------- -- FIX: Attempt at fixing a crash related to `LinkedHashMap.retainAll()` specifically on Android 6 and Android 6.1 devices (#256). +- FIX: Attempt at fixing a crash related to `LinkedHashMap.retainAll()` specifically on Android 6 and Android 6.1 + devices (#256). - 2.6.3 had an issue with `maven-publish` and transitive dependencies were missing, and is therefore skipped. - -Simple Stack 2.6.2 (2021-06-07) -------------------------------- diff --git a/README.md b/README.md index e5adda5e..829bee3e 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ and then, add the dependency to your module's `build.gradle.kts` (or `build.grad ``` kotlin // build.gradle.kts -implementation("com.github.Zhuinden:simple-stack:2.6.4") +implementation("com.github.Zhuinden:simple-stack:2.6.5") implementation("com.github.Zhuinden.simple-stack-extensions:core-ktx:2.2.4") implementation("com.github.Zhuinden.simple-stack-extensions:fragments:2.2.4") @@ -83,7 +83,7 @@ or ``` groovy // build.gradle -implementation 'com.github.Zhuinden:simple-stack:2.6.4' +implementation 'com.github.Zhuinden:simple-stack:2.6.5' implementation 'com.github.Zhuinden.simple-stack-extensions:core-ktx:2.2.4' implementation 'com.github.Zhuinden.simple-stack-extensions:fragments:2.2.4' diff --git a/simple-stack/build.gradle.kts b/simple-stack/build.gradle.kts index ba4e4cfe..5acc69d4 100644 --- a/simple-stack/build.gradle.kts +++ b/simple-stack/build.gradle.kts @@ -74,7 +74,7 @@ afterEvaluate { register("mavenJava", MavenPublication::class) { groupId = "com.github.Zhuinden" artifactId = "simple-stack" - version = "2.6.4" + version = "2.6.5" from(components["release"]) artifact(sourcesJar.get()) diff --git a/simple-stack/src/main/java/com/zhuinden/simplestack/NavigationCore.java b/simple-stack/src/main/java/com/zhuinden/simplestack/NavigationCore.java index 5e282b90..1066ccd0 100644 --- a/simple-stack/src/main/java/com/zhuinden/simplestack/NavigationCore.java +++ b/simple-stack/src/main/java/com/zhuinden/simplestack/NavigationCore.java @@ -705,9 +705,17 @@ public void removeCompletionListeners() { } private void notifyCompletionListeners(StateChange stateChange) { - for(Backstack.CompletionListener completionListener : completionListeners) { + final StateChanger currentStateChanger = stateChanger; + if(currentStateChanger != null) { + stateChanger = null; + } + for(int i = completionListeners.size() - 1; i >= 0; i--) { + Backstack.CompletionListener completionListener = completionListeners.get(i); completionListener.stateChangeCompleted(stateChange); } + if(stateChanger == null && currentStateChanger != null) { + this.stateChanger = currentStateChanger; // do not use `setStateChanger(REATTACH)` here, it would try to start state changes twice in succession + } } // force execute diff --git a/simple-stack/src/test/java/com/zhuinden/simplestack/BackstackCoreTest.java b/simple-stack/src/test/java/com/zhuinden/simplestack/BackstackCoreTest.java index 161d084e..cc2b9f09 100644 --- a/simple-stack/src/test/java/com/zhuinden/simplestack/BackstackCoreTest.java +++ b/simple-stack/src/test/java/com/zhuinden/simplestack/BackstackCoreTest.java @@ -26,6 +26,7 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nonnull; @@ -273,6 +274,67 @@ public void handleStateChange(@Nonnull StateChange _stateChange, @Nonnull Callba Mockito.verify(completionListener, Mockito.only()).stateChangeCompleted(stateChange); } + @Test + public void completionListenerCanRemoveItself() { + TestKey initial = new TestKey("hello"); + final TestKey second = new TestKey("world"); + final Backstack backstack = new Backstack(); + backstack.setup(History.of(initial)); + + final AtomicInteger listener1Called = new AtomicInteger(0); + final AtomicInteger listener2Called = new AtomicInteger(0); + final AtomicInteger listener3Called = new AtomicInteger(0); + + Backstack.CompletionListener completionListener1 = new Backstack.CompletionListener() { + @Override + public void stateChangeCompleted(@Nonnull StateChange stateChange) { + listener1Called.addAndGet(1); + } + }; + Backstack.CompletionListener completionListener2 = new Backstack.CompletionListener() { + @Override + public void stateChangeCompleted(@Nonnull StateChange stateChange) { + listener2Called.addAndGet(1); + backstack.removeCompletionListener(this); + } + }; + Backstack.CompletionListener completionListener3 = new Backstack.CompletionListener() { + @Override + public void stateChangeCompleted(@Nonnull StateChange stateChange) { + listener3Called.addAndGet(1); + } + }; + StateChanger stateChanger = new StateChanger() { + @Override + public void handleStateChange(@Nonnull StateChange _stateChange, @Nonnull Callback completionCallback) { + stateChange = _stateChange; + callback = completionCallback; + } + }; + backstack.addCompletionListener(completionListener1); + backstack.addCompletionListener(completionListener2); + backstack.addCompletionListener(completionListener2); // intentional duplicate line to reproduce #263 + backstack.addCompletionListener(completionListener3); + backstack.setStateChanger(stateChanger); + + callback.stateChangeComplete(); + + assertThat(backstack.isStateChangePending()).isFalse(); + + assertThat(listener1Called.get()).isEqualTo(1); + assertThat(listener2Called.get()).isEqualTo(2); + assertThat(listener3Called.get()).isEqualTo(1); + + backstack.goTo(second); + + callback.stateChangeComplete(); + + assertThat(listener1Called.get()).isEqualTo(2); + assertThat(listener2Called.get()).isEqualTo(2); + assertThat(listener3Called.get()).isEqualTo(2); + } + + @Test public void removedCompletionListenerShouldNotBeCalled() { TestKey initial = new TestKey("hello");