Intent handling needs work

[loganj]

There are two big problems with the current Intent handling:

1. It's just broken. Attempting to unpack a history from an Intent will always fail because load() is looking for the wrong key.
2. It's too inflexible with respect to the Intent. Only Flow can properly pack a History into an Intent, which means it's tricky to offer an action-based API.

Probably the right solution is an SPI that allows pluggable Intent unpacking.

[meoyawn]

why store History in an Intent when one can always write a bunch of own Intent -> History functions?

Or maybe (History, Intent) -> (History, Direction) ones.

Flow provides void setHistory(History, Direction) and that should be enough to modify the stack with respect to the Intent's action and extras

[loganj]

That's more or less the thrust of this issue, yes.

[Zhuinden]

You're missing a line of code to make intent restoration work.

      // We always replace the dispatcher because it frequently references the Activity.
      fragment.dispatcher = dispatcher;
      if (newFragment) {

Should be

      // We always replace the dispatcher because it frequently references the Activity.
      fragment.dispatcher = dispatcher;
      fragment.intent = a.getIntent();
      if (newFragment) {

And also this line should add the extra for PERSISTENCE_KEY instead of "FLOW_STATE".

[Zhuinden]

Did I mention that if you use the EmptyState on intent restore, then any future view state on config changes will be lost?

[dcow]

How has this type of issue been around for 11 months? I mean I can make my own addHistoryToIntent function.. but..

[Zhuinden]

@dcow you can't (or at least you can't rely on Flow's "intent loading" and onNewIntent()), you don't have access to the load() function in InternalLifecycleIntegration.

Although a simple solution to this problem is to just send a bunch of keys into the intent manually and then call setHistory(newHistory, Direction.REPLACE).

[dcow]

@Zhuinden luckily I can do:

package flow

import android.content.Intent
import android.os.Bundle
import android.os.Parcelable
import java.util.*

object FlowFix
{
    fun addTo(intent: Intent, history: History)
    {
        val parcelables = ArrayList<Parcelable>()
        val histories = history.reverseIterator<Any>()
        while (histories.hasNext()) {
            parcelables.add(State.empty(histories.next()).toBundle(<my_parceler>))
        }
        val intent_key = "${InternalLifecycleIntegration::class.java.simpleName}_history"
        val bundle_key = "${InternalLifecycleIntegration::class.java.simpleName}_state"
        val bundle = Bundle()
        bundle.putParcelableArrayList(bundle_key, parcelables)
        intent.putExtra(intent_key, bundle)
    }
}

But that only works if I'm in control of the incoming intent (assuming that outgoing history is parceled correctly and the bug is only in the addHistory function--which appears to be the case). It's still annoying that I can't delay the re-loading of history until I know I have all the dependencies for my requested state (it happens at points during the fragment lifecycle regardless.

[Zhuinden]

@dcow I think the bug is in load(), and I think InternalLifecycleIntegration doesn't actually get the intent because it doesn't call a.getIntent() as I said above

You're better off with manually using setHistory()

[dcow]

@Zhuinden yeah this thing is fucked. That intent field is never set. I'd end up writing my own selectHistory. Does anyone even use it?

[Zhuinden]

@dcow probably not considering it's completely broken; I have my own fork of Flow where I did fix this because I did use this for adding History to a PendingIntent but that one no longer has Flow.Services and ServiceFactory.

[dcow]

@Zhuinden yeah your stuff is more and more compelling. I just wrote my own functions to pull history from an intent and select history in Flow's priority and then only set it based on whether history already exists or not. It works for now but if I have more time in the future I'll check your stuff out.

[Zhuinden]

@dcow my fork by the way is at https://github.com/Zhuinden/flowless/ .

The key difference in behavior is that I removed TreeKey and MultiKey because I honestly didn't figure out how to make them work, and putting the app in background and bringing forward made the reference counting break (although I think alpha2 fixed that)

[rjrjr]

Spun off the SPI redesign stuff into #237

@Zhuinden Can you file a new issue explaining in more detail what you meant by "if you use the EmptyState on intent restore, then any future view state on config changes will be lost"?