Fixes potential out-of-order attach calls for children if the framework itself calls them out of order. (Fixes #416)

conductor/src/main/java/com/bluelinelabs/conductor/Controller.java

@@ -76,6 +76,7 @@ public abstract class Controller {
     private String targetInstanceId;
     private boolean needsAttach;
     private boolean attachedToUnownedParent;
+    private boolean awaitingParentAttach;
     private boolean hasSavedViewState;
     boolean isDetachFrozen;
     private ControllerChangeHandler overriddenPushHandler;
@@ -886,6 +887,13 @@ public abstract class Controller {
             return;
         }
 
+        if (parentController != null && !parentController.attached) {
+            awaitingParentAttach = true;
+            return;
+        } else {
+            awaitingParentAttach = false;
+        }
+
         hasSavedViewState = false;
 
         List<LifecycleListener> listeners = new ArrayList<>(lifecycleListeners);
@@ -906,6 +914,14 @@ public abstract class Controller {
         for (LifecycleListener lifecycleListener : listeners) {
             lifecycleListener.postAttach(Controller.this, view);
         }
+
+        for (ControllerHostedRouter childRouter : childRouters) {
+            for (RouterTransaction childTransaction : childRouter.backstack) {
+                if (childTransaction.controller.awaitingParentAttach) {
+                    childTransaction.controller.attach(childTransaction.controller.view);
+                }
+            }
+        }
     }
 
     void detach(@NonNull View view, boolean forceViewRefRemoval, boolean blockViewRefRemoval) {
@@ -924,7 +940,10 @@ public abstract class Controller {
             }
 
             attached = false;
-            onDetach(view);
+
+            if (!awaitingParentAttach) {
+                onDetach(view);
+            }
 
             if (hasOptionsMenu && !optionsMenuHidden) {
                 router.invalidateOptionsMenu();

conductor/src/test/java/com/bluelinelabs/conductor/ControllerLifecycleCallbacksTests.java

@@ -7,6 +7,8 @@ import android.view.View;
 import android.view.ViewGroup;
 
 import com.bluelinelabs.conductor.Controller.LifecycleListener;
+import com.bluelinelabs.conductor.Controller.RetainViewMode;
+import com.bluelinelabs.conductor.changehandler.SimpleSwapChangeHandler;
 import com.bluelinelabs.conductor.util.ActivityProxy;
 import com.bluelinelabs.conductor.util.CallState;
 import com.bluelinelabs.conductor.util.MockChangeHandler;
@@ -21,8 +23,10 @@ import org.robolectric.RobolectricTestRunner;
 import org.robolectric.annotation.Config;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 @RunWith(RobolectricTestRunner.class)
 @Config(manifest = Config.NONE)
@@ -499,6 +503,38 @@ public class ControllerLifecycleCallbacksTests {
         assertCalls(expectedCallState, child);
     }
 
+    @Test
+    public void testChildLifecycleOrderingAfterUnexpectedAttach() {
+        Controller parent = new TestController();
+        parent.setRetainViewMode(RetainViewMode.RETAIN_DETACH);
+        router.pushController(RouterTransaction.with(parent)
+                .pushChangeHandler(MockChangeHandler.defaultHandler())
+                .popChangeHandler(MockChangeHandler.defaultHandler()));
+
+        TestController child = new TestController();
+        child.setRetainViewMode(RetainViewMode.RETAIN_DETACH);
+        Router childRouter = parent.getChildRouter((ViewGroup)parent.getView().findViewById(TestController.VIEW_ID));
+        childRouter
+                .setRoot(RouterTransaction.with(child)
+                        .pushChangeHandler(new SimpleSwapChangeHandler())
+                        .popChangeHandler(new SimpleSwapChangeHandler()));
+
+        assertTrue(parent.isAttached());
+        assertTrue(child.isAttached());
+
+        ViewUtils.reportAttached(parent.getView(), false, true);
+        assertFalse(parent.isAttached());
+        assertFalse(child.isAttached());
+
+        ViewUtils.reportAttached(child.getView(), true);
+        assertFalse(parent.isAttached());
+        assertFalse(child.isAttached());
+
+        ViewUtils.reportAttached(parent.getView(), true);
+        assertTrue(parent.isAttached());
+        assertTrue(child.isAttached());
+    }
+
     private MockChangeHandler getPushHandler(final CallState expectedCallState, final TestController controller) {
         return MockChangeHandler.listeningChangeHandler(new ChangeHandlerListener() {
             @Override

conductor/src/test/java/com/bluelinelabs/conductor/util/ViewUtils.java

@@ -16,7 +16,7 @@ public class ViewUtils {
 
     public static void reportAttached(View view, boolean attached, boolean propogateToChildren) {
         if (view instanceof AttachFakingFrameLayout) {
-            ((AttachFakingFrameLayout)view).setAttached(attached, false);
+            ((AttachFakingFrameLayout) view).setAttached(attached, false);
         }
 
         List<OnAttachStateChangeListener> listeners = getAttachStateListeners(view);
@@ -44,7 +44,7 @@ public class ViewUtils {
         }
 
         if (propogateToChildren && view instanceof ViewGroup) {
-            ViewGroup viewGroup = (ViewGroup)view;
+            ViewGroup viewGroup = (ViewGroup) view;
             int childCount = viewGroup.getChildCount();
             for (int i = 0; i < childCount; i++) {
                 reportAttached(viewGroup.getChildAt(i), attached, true);