Fix/clarify refinement termination conditions
This primarily closes some gaps where our old implementation was
willing to leave refinement flows hanging in assorted edge cases.
As of this CL, every refinement flow ends in exactly one of:
A) the flow is cancelled by `ChooserRefinementManager.destroy()` (e.g.
we do this when `ChooserActivity` is closing);
B) `ChooserRefinementManager` fires its configured success callback;
C) `ChooserRefinementManager` fires its configured failure callback.
After any of those outcomes, the refinement manager cleans up its own
state (i.e., clients are no longer responsible for calling `destroy()`
during routine handling of their callbacks), and I've noted in an
implementation comment that we can use the nullability of
`ChooserRefinementManager.mRefinementResultReceiver` as an indication
of whether there's an active refinement session. These receivers are
always destroyed at the end of the session, and can never receive more
than one result callback in a single session. I've added a `@UiThread`
annotation to the entire class to clarify the concurrency model, so
it's easy to reason about the synchronized computation in this class.
A subsequent CL will expose this "active session" condition in the
refinement manager API so that `ChooserActivity` can detect the case
when the refinement activity closes with no result. `ChooserActivity`
also needs to stop suppressing its own termination when a refined
launch fails due to the target being suspended. Then we'll be able to
assert that chooser always finishes after *any* attempted refinement.
Bug: 273864843
Test: manually tested with a custom refinement activity to trigger
all the failure cases under `ChooserRefinementManager` responsibility.
As described above, `ChooserActivity` needs to handle some others
external to the refinement manager; those are punted from this CL.
Change-Id: Iee399e34dec27b9a846ea040831e23fafda46fd5
2 files changed