commit 6f0eeb452ae640a1adfa87aacefb50c083441b46
author Joshua Trask <joshtrask@google.com> Tue Dec 05 19:55:41 2023 +0000
committer Joshua Trask <joshtrask@google.com> Wed Dec 06 19:28:45 2023 +0000
tree 2fa6c4d33ad6022f2a380ad3dd6d117c0373eb04
parent 702b0bcb8cde313c97cdd461e6c96b8075cf0b6d

Move per-profile operations to pager-adapter APIs

These are methods previously implemented in the activities, which
operate generically on all "profile tabs" (or the "active" or
"inactive" tabs) -- as opposed to special-case behaviors for
personal/work/clone/private/etc. The pager-adapter API exposed
low-level access to its data model, allowing clients to implement
their own algorithms that implicitly inherited our historic "2-tab"
assumption. By pulling these methods into the pager-adapter (which
was generally supposed to own this data model anyways), we limit
the scope of the remaining cleanup to relax the "2-tab" assumption.

These changes are as prototyped in ag/25335069 and described in
go/chooser-ntab-refactoring (starting with "snapshot #7"; note
according to go/chooser-ntab-refactoring, this "arc" begins with
"snapshot #6" of ag/25335069, but that particular change builds on
other changes reproduced in ag/25560839, so here we skip that one
change to get a "clean start" for parallel review).

Snapshot #1 (from ag/25335069 "snapshot #7"):
Generalize `ChooserActivity.shouldShowExtraRow()` to consider the
presence of empty-state screens in "any" inactive adapter, while
pushing the logic for that determination into the pager-adapter.
The logic isn't yet updated to evaluate multiple inactive profiles,
but it's at least consolidated into the `MultiProfilePagerAdapter`
component to be fixed later. Note the refactoring omits the old
`shouldShowTabs()` condition; this is implied since the new
"any-inactive" logic returns false if there are no inactive tabs.
There's seemingly no coverage for this "extra-row" functionality;
I'm open to suggestions but maybe we can wait for more requirements
from the private-space work. For now the equivalence should still
be clear from code-reading (but I'd reluctantly be willing to revert
the removal of the `shouldShowTabs()` in consideration of the scant
testing).

Snapshot #2 (from ag/25335069 "snapshot #8"):
Instead of letting `ResolverActivity` use low-level pager-adapter
methods to access the "active" and "inactive" tabs to rebuild,
have the activity request a "tab rebuild" where the pager-adapter
is responsible for deciding the applicable tabs according to its
internal data model. The equivalence of this change should be clear
from code-reading, but in the interest of completeness I confirmed
that we have some existing test coverage exercising this flow -- an
early return (true or false) from the new `rebuildTabs()` causes a
test failure, as does an inversion of the 'partial-rebuild' condtion
(i.e., starting with `includePartialRebuildOfInactiveTabs = !...").

Snapshot #3 (from ag/25335069 "snapshot #9"):
Move a concrete-`ResolverActivity`-specific operation into its
subclass pager-adapter since it generically manipulates "inactive
tabs." This test method is exercised by the legacy
`ResolverActivityTest` but we don't seem to make any assertions about
its behavior (do we need to add more test coverage now? It may be hard
to set up, and we expect significant architectural changes in the near
future anyways...)

Snapshot #4 (from ag/25335069 "snapshot #17"):
Move a `ChooserActivity`-specific operation into its subclass
pager-adapter since it generically manipulates *all* tabs. As in the
previous snapshot, this method is only nominally covered in tests, but
we don't make specific assertions about its behavior.

Bug: 310211468
Test: `IntentResolver-tests-{unit,activity,integration}`. See notes ^
Change-Id: Ie4a0e99a59872ec16f8491bfa6ffb548e37db1da