aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJoshua Trask <joshtrask@google.com>2023-12-05 19:55:41 +0000
committerJoshua Trask <joshtrask@google.com>2023-12-06 19:28:45 +0000
commit6f0eeb452ae640a1adfa87aacefb50c083441b46 (patch)
tree2fa6c4d33ad6022f2a380ad3dd6d117c0373eb04
parent702b0bcb8cde313c97cdd461e6c96b8075cf0b6d (diff)
downloadIntentResolver-6f0eeb452ae640a1adfa87aacefb50c083441b46.tar.gz
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
-rw-r--r--java/src/com/android/intentresolver/v2/ChooserActivity.java15
-rw-r--r--java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java7
-rw-r--r--java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java36
-rw-r--r--java/src/com/android/intentresolver/v2/ResolverActivity.java14
-rw-r--r--java/src/com/android/intentresolver/v2/ResolverMultiProfilePagerAdapter.java9
5 files changed, 59 insertions, 22 deletions
diff --git a/java/src/com/android/intentresolver/v2/ChooserActivity.java b/java/src/com/android/intentresolver/v2/ChooserActivity.java
index 95eedf4..19032b3 100644
--- a/java/src/com/android/intentresolver/v2/ChooserActivity.java
+++ b/java/src/com/android/intentresolver/v2/ChooserActivity.java
@@ -992,11 +992,7 @@ public class ChooserActivity extends Hilt_ChooserActivity implements
@Override
protected void applyFooterView(int height) {
- int count = mChooserMultiProfilePagerAdapter.getItemCount();
-
- for (int i = 0; i < count; i++) {
- mChooserMultiProfilePagerAdapter.getAdapterForIndex(i).setFooterHeight(height);
- }
+ mChooserMultiProfilePagerAdapter.setFooterHeightInEveryAdapter(height);
}
private void logDirectShareTargetReceived(UserHandle forUser) {
@@ -1499,14 +1495,13 @@ public class ChooserActivity extends Hilt_ChooserActivity implements
/**
* If we have a tabbed view and are showing 1 row in the current profile and an empty
- * state screen in the other profile, to prevent cropping of the empty state screen we show
+ * state screen in another profile, to prevent cropping of the empty state screen we show
* a second row in the current profile.
*/
private boolean shouldShowExtraRow(int rowsToShow) {
- return shouldShowTabs()
- && rowsToShow == 1
- && mChooserMultiProfilePagerAdapter.shouldShowEmptyStateScreen(
- mChooserMultiProfilePagerAdapter.getInactiveListAdapter());
+ return rowsToShow == 1
+ && mChooserMultiProfilePagerAdapter
+ .shouldShowEmptyStateScreenInAnyInactiveAdapter();
}
/**
diff --git a/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java b/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java
index 87b3b20..7ea78d1 100644
--- a/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java
+++ b/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java
@@ -161,6 +161,13 @@ public class ChooserMultiProfilePagerAdapter extends MultiProfilePagerAdapter<
return super.rebuildTab(listAdapter, doPostProcessing);
}
+ /** Apply the specified {@code height} as the footer in each tab's adapter. */
+ public void setFooterHeightInEveryAdapter(int height) {
+ for (int i = 0; i < getItemCount(); ++i) {
+ getAdapterForIndex(i).setFooterHeight(height);
+ }
+ }
+
private static class BottomPaddingOverrideSupplier implements Supplier<Optional<Integer>> {
private final Context mContext;
private int mBottomOffset;
diff --git a/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java b/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java
index a600d4a..212bf3b 100644
--- a/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java
+++ b/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java
@@ -354,6 +354,28 @@ public class MultiProfilePagerAdapter<
}
/**
+ * Fully-rebuild the active tab and, if specified, partially-rebuild any other inactive tabs.
+ */
+ public boolean rebuildTabs(boolean includePartialRebuildOfInactiveTabs) {
+ // TODO: we may be able to determine `includePartialRebuildOfInactiveTabs` ourselves as
+ // a function of our own instance state. OTOH the purpose of this "partial rebuild" is to
+ // be able to evaluate the intermediate state of one particular profile tab (i.e. work
+ // profile) that may not generalize well when we have other "inactive tabs." I.e., either we
+ // rebuild *all* the inactive tabs just to evaluate some auto-launch conditions that only
+ // depend on personal and/or work tabs, or we have to explicitly specify the ones we care
+ // about. It's not the pager-adapter's business to know "which ones we care about," so maybe
+ // they should be rebuilt lazily when-and-if it comes up (e.g. during the evaluation of
+ // autolaunch conditions).
+ boolean rebuildCompleted = rebuildActiveTab(true) || getActiveListAdapter().isTabLoaded();
+ if (includePartialRebuildOfInactiveTabs) {
+ boolean rebuildInactiveCompleted =
+ rebuildInactiveTab(false) || getInactiveListAdapter().isTabLoaded();
+ rebuildCompleted = rebuildCompleted && rebuildInactiveCompleted;
+ }
+ return rebuildCompleted;
+ }
+
+ /**
* Rebuilds the tab that is currently visible to the user.
* <p>Returns {@code true} if rebuild has completed.
*/
@@ -368,7 +390,7 @@ public class MultiProfilePagerAdapter<
* Rebuilds the tab that is not currently visible to the user, if such one exists.
* <p>Returns {@code true} if rebuild has completed.
*/
- public final boolean rebuildInactiveTab(boolean doPostProcessing) {
+ private boolean rebuildInactiveTab(boolean doPostProcessing) {
Trace.beginSection("MultiProfilePagerAdapter#rebuildInactiveTab");
if (getItemCount() == 1) {
Trace.endSection();
@@ -477,6 +499,18 @@ public class MultiProfilePagerAdapter<
descriptor.mEmptyStateUi.hide();
}
+ /**
+ * @return whether any "inactive" tab's adapter would show an empty-state screen in our current
+ * application state.
+ */
+ public final boolean shouldShowEmptyStateScreenInAnyInactiveAdapter() {
+ if (getCount() < 2) {
+ return false;
+ }
+ // TODO: check against *any* inactive adapter; for now we only have one.
+ return shouldShowEmptyStateScreen(getInactiveListAdapter());
+ }
+
public boolean shouldShowEmptyStateScreen(ListAdapterT listAdapter) {
int count = listAdapter.getUnfilteredCount();
return (count == 0 && listAdapter.getPlaceholderCount() == 0)
diff --git a/java/src/com/android/intentresolver/v2/ResolverActivity.java b/java/src/com/android/intentresolver/v2/ResolverActivity.java
index a7f2047..2c1497f 100644
--- a/java/src/com/android/intentresolver/v2/ResolverActivity.java
+++ b/java/src/com/android/intentresolver/v2/ResolverActivity.java
@@ -1582,13 +1582,7 @@ public class ResolverActivity extends FragmentActivity implements
Trace.beginSection("configureContentView");
// We partially rebuild the inactive adapter to determine if we should auto launch
// isTabLoaded will be true here if the empty state screen is shown instead of the list.
- boolean rebuildCompleted = mMultiProfilePagerAdapter.rebuildActiveTab(true)
- || mMultiProfilePagerAdapter.getActiveListAdapter().isTabLoaded();
- if (shouldShowTabs()) {
- boolean rebuildInactiveCompleted = mMultiProfilePagerAdapter.rebuildInactiveTab(false)
- || mMultiProfilePagerAdapter.getInactiveListAdapter().isTabLoaded();
- rebuildCompleted = rebuildCompleted && rebuildInactiveCompleted;
- }
+ boolean rebuildCompleted = mMultiProfilePagerAdapter.rebuildTabs(shouldShowTabs());
if (shouldUseMiniResolver()) {
configureMiniResolverContent(targetDataLoader);
@@ -1962,10 +1956,8 @@ public class ResolverActivity extends FragmentActivity implements
return;
}
mLastSelected = ListView.INVALID_POSITION;
- ListView inactiveListView = (ListView) mMultiProfilePagerAdapter.getInactiveAdapterView();
- if (inactiveListView.getCheckedItemCount() > 0) {
- inactiveListView.setItemChecked(inactiveListView.getCheckedItemPosition(), false);
- }
+ ((ResolverMultiProfilePagerAdapter) mMultiProfilePagerAdapter)
+ .clearCheckedItemsInInactiveProfiles();
}
private String getPersonalTabAccessibilityLabel() {
diff --git a/java/src/com/android/intentresolver/v2/ResolverMultiProfilePagerAdapter.java b/java/src/com/android/intentresolver/v2/ResolverMultiProfilePagerAdapter.java
index dadc9c0..d96fd15 100644
--- a/java/src/com/android/intentresolver/v2/ResolverMultiProfilePagerAdapter.java
+++ b/java/src/com/android/intentresolver/v2/ResolverMultiProfilePagerAdapter.java
@@ -107,6 +107,15 @@ public class ResolverMultiProfilePagerAdapter extends
mBottomPaddingOverrideSupplier.setUseLayoutWithDefault(useLayoutWithDefault);
}
+ /** Un-check any item(s) that may be checked in any of our inactive adapter(s). */
+ public void clearCheckedItemsInInactiveProfiles() {
+ // TODO: apply to all inactive adapters; for now we just have the one.
+ ListView inactiveListView = getInactiveAdapterView();
+ if (inactiveListView.getCheckedItemCount() > 0) {
+ inactiveListView.setItemChecked(inactiveListView.getCheckedItemPosition(), false);
+ }
+ }
+
private static class BottomPaddingOverrideSupplier implements Supplier<Optional<Integer>> {
private boolean mUseLayoutWithDefault;