Clarify/simplify ContentPreviewCoordinator lifecycle
This is *not* an obviously-safe "pure" refactor; there are a few
notable logic changes(*), but they should cause no observable
behavior changes in practice.
The simplified lifecycle (deferred assignment -> pre-initialization)
shows that this component has essential responsibilities to
`ChooserActivity` in ensuring that asynchronous tasks are shut down
when the activity is destroyed. Minor refactoring in this CL shows
that the component is otherwise injectable as a delegate in our
preview-loading "factories," to be extracted in another upcoming
cleanup CL; a new (temporarily-homed) interface in this CL makes
that delegation API explicit. I extracted the implementation to
an outer class to chip away at the `ChooserActivity` monolith; to
draw attention to the coordinator's business-logic responsibilities
in defining success/failure conditions (in addition to the UI
responsibilities that ayepin@ suggests could be separated from the
rest of the coordinator component); and to provide a clearer line
to cut away if we (hopefully) eventually decide to move off of this
particular processing model altogether. For more discussion see
comments on ag/20390247, summarized below.
* [Logic changes]
1. We now guarantee at most one `ContentPreviewCoordinator` instance.
This is unlikely to differ from the earlier behavior, but we
would've had no checks before a potential re-assignment. If one
were to occur, we would've lost track of any pending tasks that
the previous instance(s) were responsible for cancelling. (By
comparison, after this CL, multiple instances would instead
queue their requests in a shared coordinator and influence each
other's "batch" timeout logic -- it's debatable whether that's
correct, but it's ultimately insignificant either way).
2. Even if we never re-assigned any extra coordinator instances,
the model prior to this CL was effectively "lazy initialization"
of the coordinator, but we now initialize a coordinator "eagerly"
to simplify the lifecycle. While the earlier model was technically
"lazy," it was still computed during the `onCreate()` flow, so
this doesn't make much difference -- except notably, we'll now
initialize a coordinator for every Sharesheet session, even if we
don't end up building a preview UI. The coordinator class is
lightweight if it's never used, so this doesn't seem like a
problem.
3. The `findViewById()` queries in `ContentPreviewCoordinator` now
have a broader root for their search so that they can work for
both kinds of previews ("sticky" and "list item"), and we can
share the one eagerly-initialized instance. We can always change
the API if we need more specificity later, but for now it seems
like we can make this change with no repercussions for our app
behavior. For more detail see ag/20390247, but essentially:
a. The IDs of the views we search for are explicitly named for
the purpose of content previews and won't plausibly be used
for anything else. Thus,
b. The broadened queries could only be ambiguous if we were to
display more than one content preview in our hierarchy. But:
c. We show at most one content preview: either the "sticky"
preview in the `ChooserActivity` root layout, or the "list
item" preview that's built into the list *when we have only
one profile to show*, and never both (gated on the result
of `shouldShowTabs()`).
Test: atest IntentResolverUnitTests
Bug: 202167050
Change-Id: I0dd6e48ee92845ce68d6dcf8e84e272b11caf496
3 files changed