Simplify ContentPreviewCoordinator (pure refactor)
This component effectively serves as the bridge between our (mostly
standalone) content preview logic and the ChooserActivity application
where that preview is displayed. A subsequent refactoring CL will
migrate the content preview logic out of ChooserActivity, so this is
a preliminary step to decouple dependencies on unrelated
ChooserActivity concerns (after this CL, we could even choose to
migrate the definition of the ContentPreviewCoordinator class out of
ChooserActivity alongside the rest of the preview logic).
Changes are small to aid reviewers in understanding that this CL
should cause no behavioral changes. Any more-significant cleanup is
out-of-scope for now and should wait until we resolve a clearer
picture of where various responsibilities will land in the new
application architecture.
Main changes:
1. Make the inner `ContentPreviewCoordinator` class `static`, then
inject a reference to the `ChooserActivity` in the constructor.
That back-reference technically means that this is *barely* less
coupled than before (i.e., it's just setting up a "code move" to
organize our source code, without fundamentally improving the
design). The reference isn't used for much -- access to package
resources, lifecycle state, and one preview-related method that I
expect to move in the next CL -- so it can probably be cleaned up
for better decoupling once the dust settles. Either way, the
`static` declaration explicates any dependencies to the outer
`ChooserActivity` so we can more easily reason about the inner
class component.
2. Inline `hideParentOnFail = false`. This was hard-coded at the
three call sites where we construct the coordinator objects, so
I removed the argument and deleted the extra complexity in
handling it (including some knock-on concerns in the outer class).
3. Inject success/failure callbacks to delegate the parts of the
logic that relate to `ChooserActivity` concerns which are
otherwise tangential to content previews. These are just specified
as `Consumer` and `Runnable` for now (respectively) for simplicity
but could be replaced by a more purpose-built interface (IMO
probably worthwhile only if the responsibilities expand?)
4. Add visibility specifiers (and re-order methods) to show which
parts of the `ContentPreviewCoordinator` API are actually used by
"clients." They're now specified as they'd need to be if the class
ends up being migrated out of `ChooserActivity`. (EDIT: presubmit
hooks kicked back one of the changes since it technically can't
matter in the current location, but I just converted it to a
comment for now, because IMO it's still useful info, and there's
no guarantee the class is staying "in the current location.")
5. Document TODOs and make other minor style changes I felt would
improve readability.
Test: atest IntentResolverUnitTests
Bug: 202167050
Change-Id: I098ccb8e419dee1b301a85da07bda573cc94f5f8
1 file changed