Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make sure events are not assumed and always update to lifecycle obser… #1230

Closed
wants to merge 3 commits into from
+14 −32
Closed

make sure events are not assumed and always update to lifecycle obser… #1230

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9337e47
make sure events are not assumed and always update to lifecycle obser…
ekeitho Nov 11, 2024
324945a
revert a bit to please some tests
ekeitho Nov 11, 2024
2adf146
Apply changes from ktLintFormat
ekeitho Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 14 additions & 32 deletions ...i/core-android/src/main/java/com/squareup/workflow1/ui/androidx/WorkflowLifecycleOwner.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import androidx.annotation.VisibleForTesting
import androidx.annotation.VisibleForTesting.Companion.PRIVATE
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.Lifecycle.Event
import androidx.lifecycle.Lifecycle.State
import androidx.lifecycle.Lifecycle.State.DESTROYED
import androidx.lifecycle.Lifecycle.State.INITIALIZED
import androidx.lifecycle.Lifecycle.State.RESUMED
Expand Down Expand Up @@ -190,32 +191,38 @@ internal class RealWorkflowLifecycleOwner(
oldLifecycle?.removeObserver(this)
parentLifecycle?.addObserver(this)
}
updateLifecycle()

// if (parentLifecycle?.currentState == INITIALIZED) {
// // updateLifecycle(Event.ON_RESUME.targetState)
// } else {
//
// }
updateLifecycle(Event.ON_RESUME.targetState)
}

override fun onViewDetachedFromWindow(v: View) {
viewIsAttachedToWindow = false
updateLifecycle()
updateLifecycle(Event.ON_PAUSE.targetState)
}

/** Called when the [parentLifecycle] changes state. */
override fun onStateChanged(
source: LifecycleOwner,
event: Event
) {
updateLifecycle()
updateLifecycle(event.targetState)
}

override fun destroyOnDetach() {
if (!destroyOnDetach) {
destroyOnDetach = true
updateLifecycle()
updateLifecycle(Event.ON_DESTROY.targetState)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make sure the comment doesn't get lost across updates: we can't ship this way.

  • destroyOnDetach() has to mean "move to destroyed the next time onViewDetachedFromWindow() is called.

}
}

@VisibleForTesting(otherwise = PRIVATE)
internal fun updateLifecycle() {
val parentState = parentLifecycle?.currentState
internal fun updateLifecycle(parentTargetState: State) {
// val parentState = parentLifecycle?.currentState
val localState = localLifecycle.currentState

if (localState == DESTROYED || hasBeenDestroyed) {
Expand All @@ -224,32 +231,7 @@ internal class RealWorkflowLifecycleOwner(
return
}

localLifecycle.currentState = when {
destroyOnDetach && !viewIsAttachedToWindow -> {
// We've been enqueued for destruction.
// Stay attached to the parent's lifecycle until we re-attach, since the parent could be
// destroyed while we're detached.
DESTROYED
}
parentState != null -> {
// We may or may not be attached, but we have a parent lifecycle so we just blindly follow
// it.
parentState
}
localState == INITIALIZED -> {
// We have no parent and we're not destroyed, which means we have never been attached, so
// the only valid state we can be in is INITIALIZED.
INITIALIZED
}
else -> {
// We don't have a parent and we're neither in DESTROYED or INITIALIZED: this is an invalid
// state. Throw an AssertionError instead of IllegalStateException because there's no API to
// get into this state, so this means the library has a bug.
throw AssertionError(
"Must have a parent lifecycle after attaching and until being destroyed."
)
}
}.let { newState ->
localLifecycle.currentState = parentTargetState.let { newState ->
if (newState == DESTROYED) {
hasBeenDestroyed = true

Expand Down
Loading