[BUG] Strange scene lifecycle using an ActivityController

[manueldidonna]

Describe the bug A Scenewith ActivityControllerhas a strange lifecycle. When the activity launched with the ActivityController.createIntent(): Intent is closed, the scene lifecycle is attached -> destroy -> detach. The detach event happens between start & attach of the previous scene. I think it's right because I close the scene in a callback after onResult() and from DefaultActivityHandler I get that the order of events is attach -> onResult -> detach

v("ActivityHandler", "Attaching container to $scene.")
scene.forceAttach(activityController)

v("ActivityHandler", "Notifying ActivityController of result.") 
activityController.onResult(resultCode, data)

v("ActivityHandler", "Detaching container from $scene.")
scene.forceDetach(activityController)

The bug happens when the screen rotates and the activity is closed in a different orientation. In that case no scene lifecycle event will be invoked, nor ActivityController.onResult

To Reproduce

class ExportDatabaseController(
    private val context: Context
) : ActivityController, ExportDatabaseContainer {

    override var onExportResult: (() -> Unit)? = null

    override fun createIntent(): Intent {
        return Intent(Intent.ACTION_OPEN_DOCUMENT_TREE).apply {
            flags = Intent.FLAG_GRANT_READ_URI_PERMISSION or
                Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION
        }
    }

    override fun onResult(resultCode: Int, data: Intent?) {
        onExportResult?.invoke()
    }
}

interface ExportDatabaseContainer: Container {
    var onExportResult: (() -> Unit)?
}

class ExportDatabaseScene(
    private val listener: CloseSceneEvent
) : Scene<ExportDatabaseContainer> {

    override fun attach(v: ExportDatabaseContainer) {
        super.attach(v)
        Timber.d("attach")
        v.onExportResult = {
            Timber.d("callback")
            listener.closeCurrentScene()
        }
    }

    override fun detach(v: ExportDatabaseContainer) {
        super.detach(v)
        Timber.d("detach")
        v.onExportResult = null
    }

    override fun onDestroy() {
        super.onDestroy()
        Timber.d("destroy activity")
    }
}

Environmental info:

• Library version: 1.2.0
• Device: Oneplus 6T
• OS: Android 10

[manueldidonna]

From DefaultActivityHandler

private var lastSceneKey: SceneKey? = savedState.lastSceneKey
override fun withScene(scene: Scene<out Container>, activityController: ActivityController) {
        v("ActivityHandler", "Scene changed: $scene.")

        if (lastSceneKey == scene.key) {
            v(
                "ActivityHandler",
                "New external Scene has the same key as the previously dispatched Scene, not starting Activity."
            )
            return
        }

        lastScene = scene
        lastActivityController = activityController

        val intent = activityController.createIntent()

        v("ActivityHandler", "Starting Intent: $intent.")
        callback.startForResult(intent)
    }

When the rotated activity is closed, this method is invoked. At the time of invocation both lastScene & lastActivityController are null but lastSceneKey is restored from the savedState soif (lastSceneKey == scene.key) succeeds and those variables are never initialized. I think that we could solve the problem putting the assignations before the if block

The reason why the controller is never attached to the scene is that lastScene & lastActivityController are null when DefaultActivityHandler.onActivityResult is invoked

  override fun onActivityResult(resultCode: Int, data: Intent?) {
        d("ActivityHandler", "Activity result: resultCode=$resultCode, data=$data")

        val scene = lastScene
        val activityController = lastActivityController
        if (scene == null || activityController == null) {
            w(
                "ActivityHandler",
                "Activity result without active Scene, dropping result"
            )
            return
        }

        v("ActivityHandler", "Attaching container to $scene.")
        scene.forceAttach(activityController)

        v("ActivityHandler", "Notifying ActivityController of result.")
        activityController.onResult(resultCode, data)

        v("ActivityHandler", "Detaching container from $scene.")
        scene.forceDetach(activityController)
}

[manueldidonna]

I think that we could solve the problem putting the assignations before the if block

@nhaarman if you verify the correctness of what I said, I could also create a PR

[nhaarman]

I'll have a look soon!

[nhaarman]

'Soon' turned out to take a little longer, sorry about that ;-)

Thanks for the elaborate report! This really helps with the reproduction 👍

It turns out that unconditionally assigning the scene and activityController before the if statement does not work, since the if statement is actually dependent on the current assignment (lastSceneKey is replaced when lastScene is set).

In #166 I've implemented a solution that does the assignments inside the if block as well, which does solve the issue. It also turned out the existing hello-startactivity example showed this erratic behavior as well, so we've got a proper test case covering it now as well 👍

Thanks a lot!

[manueldidonna]

Don't worry for taking longer than expected. Thank you very much!

I am glad to have been helpful 😁

[manueldidonna]

Can you release an updated version of the library?

[nhaarman]

I have released 1.2.4, which should be synched to maven central soon!