From 702b0bcb8cde313c97cdd461e6c96b8075cf0b6d Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Mon, 4 Dec 2023 17:56:09 +0000 Subject: Chooser pager-adapter: override generic rebuildTab (This is the "miscellaneous" change from go/chooser-ntab-refactoring / "snapshot 19" from ag/25335069). TLDR: we want to burn down external references to having an "inactive" tab, and this is one "low-hanging fruit" case where we really didn't need to distinguish active/inactive status in the first place. As originally noted in ag/25335069, this is essentially a no-op change; there may appear to be a discrepancy compared to the legacy `rebuildInactiveTab()` with regards to the old `getItemCount() != 1` condition, but the distinction is vacuous since the only caller of `rebuildInactiveTab()` (`ResolverActivity.configureContentView()`) already verifies `shouldShowTabs()` as a precondition to the rebuild, which implies `getItemCount() > 1`. FWIW this CL *does* invert the "nesting" of the two different trace mechanisms. In the original implementation, the chooser override's "app target loading section" would begin prior to the call up to the super `MultiProfilePagerAdapter.rebuild{Active,Inactive}Tab()`, "outside" of the tracing that starts in those methods; now the subclass trace section is moved "inside," to `rebuildTab()`, where the superclass tracing has already started. IMO this distinction should be negligible. It's hard to test this kind of "structure-only" change in isolation, especially when we expect more upcoming changes to redefine the APIs that we're taking care to preserve here (namely via other anticipated changes from ag/25335069, if nothing else in the "v2 restructuring"). For the sake of being thorough I did confirm that we have *some* meaningful test coverage that exercises this rebuild flow, as I was able to induce test failures (in `IntentResolver-tests-activity`) by by deliberately omitting the superclass call in the override `ChooserMultiProfilePagerAdapter.rebuildTab()`. Otherwise, I believe the equivalence should be clear from code-reading. Bug: 310211468 Test: `IntentResolver-tests-{unit,activity,integration}`. See note ^ Change-Id: I938984e5f2fade24d1f4c58cf0d99e60e4932e3d --- .../intentresolver/v2/ChooserMultiProfilePagerAdapter.java | 14 +++----------- .../intentresolver/v2/MultiProfilePagerAdapter.java | 6 +++--- 2 files changed, 6 insertions(+), 14 deletions(-) (limited to 'java') diff --git a/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java b/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java index 81797e9a..87b3b201 100644 --- a/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java +++ b/java/src/com/android/intentresolver/v2/ChooserMultiProfilePagerAdapter.java @@ -154,19 +154,11 @@ public class ChooserMultiProfilePagerAdapter extends MultiProfilePagerAdapter< } @Override - public boolean rebuildActiveTab(boolean doPostProcessing) { + protected final boolean rebuildTab(ChooserListAdapter listAdapter, boolean doPostProcessing) { if (doPostProcessing) { - Tracer.INSTANCE.beginAppTargetLoadingSection(getActiveListAdapter().getUserHandle()); + Tracer.INSTANCE.beginAppTargetLoadingSection(listAdapter.getUserHandle()); } - return super.rebuildActiveTab(doPostProcessing); - } - - @Override - public boolean rebuildInactiveTab(boolean doPostProcessing) { - if (getItemCount() != 1 && doPostProcessing) { - Tracer.INSTANCE.beginAppTargetLoadingSection(getInactiveListAdapter().getUserHandle()); - } - return super.rebuildInactiveTab(doPostProcessing); + return super.rebuildTab(listAdapter, doPostProcessing); } private static class BottomPaddingOverrideSupplier implements Supplier> { diff --git a/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java b/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java index 0b64a0ee..a600d4ad 100644 --- a/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java +++ b/java/src/com/android/intentresolver/v2/MultiProfilePagerAdapter.java @@ -357,7 +357,7 @@ public class MultiProfilePagerAdapter< * Rebuilds the tab that is currently visible to the user. *

Returns {@code true} if rebuild has completed. */ - public boolean rebuildActiveTab(boolean doPostProcessing) { + public final boolean rebuildActiveTab(boolean doPostProcessing) { Trace.beginSection("MultiProfilePagerAdapter#rebuildActiveTab"); boolean result = rebuildTab(getActiveListAdapter(), doPostProcessing); Trace.endSection(); @@ -368,7 +368,7 @@ public class MultiProfilePagerAdapter< * Rebuilds the tab that is not currently visible to the user, if such one exists. *

Returns {@code true} if rebuild has completed. */ - public boolean rebuildInactiveTab(boolean doPostProcessing) { + public final boolean rebuildInactiveTab(boolean doPostProcessing) { Trace.beginSection("MultiProfilePagerAdapter#rebuildInactiveTab"); if (getItemCount() == 1) { Trace.endSection(); @@ -387,7 +387,7 @@ public class MultiProfilePagerAdapter< } } - private boolean rebuildTab(ListAdapterT activeListAdapter, boolean doPostProcessing) { + protected boolean rebuildTab(ListAdapterT activeListAdapter, boolean doPostProcessing) { if (shouldSkipRebuild(activeListAdapter)) { activeListAdapter.postListReadyRunnable(doPostProcessing, /* rebuildCompleted */ true); return false; -- cgit v1.2.3-59-g8ed1b