From 0d88428f86f4ac7050911c34ea41eca3dee03bcc Mon Sep 17 00:00:00 2001 From: Sergey Petrov Date: Sat, 11 Apr 2020 10:36:32 +0300 Subject: [PATCH 1/2] Bug 592: fix needsAttach flag on host detach --- .../bluelinelabs/conductor/Controller.java | 4 +-- .../com/bluelinelabs/conductor/Router.java | 10 +++--- .../bluelinelabs/conductor/RouterTests.java | 36 +++++++++++++++++++ 3 files changed, 43 insertions(+), 7 deletions(-) diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/Controller.java b/conductor/src/main/java/com/bluelinelabs/conductor/Controller.java index 5ccfa9a9..9992f98b 100644 --- a/conductor/src/main/java/com/bluelinelabs/conductor/Controller.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/Controller.java @@ -761,8 +761,8 @@ final void setNeedsAttach(boolean needsAttach) { this.needsAttach = needsAttach; } - final void prepareForHostDetach() { - needsAttach = needsAttach || attached; + final void prepareForHostDetach(boolean wasAttached) { + needsAttach = wasAttached; for (ControllerHostedRouter router : childRouters) { router.prepareForHostDetach(); diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/Router.java b/conductor/src/main/java/com/bluelinelabs/conductor/Router.java index 024b3d70..8f169503 100644 --- a/conductor/src/main/java/com/bluelinelabs/conductor/Router.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/Router.java @@ -632,12 +632,12 @@ public void onActivityDestroyed(@NonNull Activity activity) { public void prepareForHostDetach() { pendingControllerChanges.clear(); // rely on backstack based restoration in rebindIfNeeded - for (RouterTransaction transaction : backstack) { - if (ControllerChangeHandler.completeHandlerImmediately(transaction.controller().getInstanceId())) { - transaction.controller().setNeedsAttach(true); + if (!backstack.isEmpty()) { + RouterTransaction top = backstack.peek(); + for (RouterTransaction transaction : backstack) { + ControllerChangeHandler.completeHandlerImmediately(transaction.controller().getInstanceId()); + transaction.controller().prepareForHostDetach(transaction == top); } - - transaction.controller().prepareForHostDetach(); } } diff --git a/conductor/src/test/java/com/bluelinelabs/conductor/RouterTests.java b/conductor/src/test/java/com/bluelinelabs/conductor/RouterTests.java index df4b062d..99ea9850 100644 --- a/conductor/src/test/java/com/bluelinelabs/conductor/RouterTests.java +++ b/conductor/src/test/java/com/bluelinelabs/conductor/RouterTests.java @@ -493,4 +493,40 @@ public void preDestroyView(@NonNull Controller controller, @NonNull View view) { assertTrue(controller3.isBeingDestroyed()); } + @Test + public void testReattachNestedBackstackCorrectly() { + Controller rootController = new TestController(); + Controller childController1 = new TestController(); + Controller childController2 = new TestController(); + Controller childController11 = new TestController(); + Controller childController12 = new TestController(); + Controller childController13 = new TestController(); + + + router.setRoot(RouterTransaction.with(rootController)); + Router router1 = rootController.getChildRouter(rootController.getView().findViewById(TestController.CHILD_VIEW_ID_1)); + router1.setRoot(RouterTransaction.with(childController1)); + Router router2 = childController1.getChildRouter(childController1.getView().findViewById(TestController.CHILD_VIEW_ID_1)); + router2.setRoot(RouterTransaction.with(childController11)); + router2.pushController(RouterTransaction.with(childController12).pushChangeHandler(new HorizontalChangeHandler()).popChangeHandler(new HorizontalChangeHandler())); + router2.pushController(RouterTransaction.with(childController13).pushChangeHandler(new HorizontalChangeHandler()).popChangeHandler(new HorizontalChangeHandler())); + router1.pushController(RouterTransaction.with(childController2)); + + assertTrue(rootController.isAttached()); + assertFalse(childController1.isAttached()); + assertFalse(childController11.isAttached()); + assertFalse(childController12.isAttached()); + assertFalse(childController13.isAttached()); + assertTrue(childController2.isAttached()); + + assertTrue(router.handleBack()); + + assertTrue(rootController.isAttached()); + assertTrue(childController1.isAttached()); + assertFalse(childController11.isAttached()); + assertFalse(childController12.isAttached()); + assertTrue(childController13.isAttached()); + assertFalse(childController2.isAttached()); + } + } From d883895f877fd6135188a7cea18af08cb80c50ab Mon Sep 17 00:00:00 2001 From: Sergey Petrov Date: Fri, 17 Apr 2020 09:44:06 +0300 Subject: [PATCH 2/2] Code review change. --- .../main/java/com/bluelinelabs/conductor/Router.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/conductor/src/main/java/com/bluelinelabs/conductor/Router.java b/conductor/src/main/java/com/bluelinelabs/conductor/Router.java index 8f169503..8a635d1b 100644 --- a/conductor/src/main/java/com/bluelinelabs/conductor/Router.java +++ b/conductor/src/main/java/com/bluelinelabs/conductor/Router.java @@ -632,12 +632,11 @@ public void onActivityDestroyed(@NonNull Activity activity) { public void prepareForHostDetach() { pendingControllerChanges.clear(); // rely on backstack based restoration in rebindIfNeeded - if (!backstack.isEmpty()) { - RouterTransaction top = backstack.peek(); - for (RouterTransaction transaction : backstack) { - ControllerChangeHandler.completeHandlerImmediately(transaction.controller().getInstanceId()); - transaction.controller().prepareForHostDetach(transaction == top); - } + boolean needsAttach = true; + for (RouterTransaction transaction : backstack) { + ControllerChangeHandler.completeHandlerImmediately(transaction.controller().getInstanceId()); + transaction.controller().prepareForHostDetach(needsAttach); + needsAttach = transaction.pushChangeHandler() != null && !transaction.pushChangeHandler().removesFromViewOnPush(); } }