From e366d8cd25624e35bb0eeb5e1d46cbfdf12a4ae5 Mon Sep 17 00:00:00 2001 From: Joshua Trask Date: Mon, 14 Nov 2022 12:11:00 -0500 Subject: Fail startup via ResolverActivity.super_onCreate() `ResolverActivity` provides this mechanism so that we can bail out of activity creation if we determine that the request is invalid. That elides a lot of the internal (~"template method") calls that the base class dispatches down to `ChooserActivity` during setup, where we've had to deal with annoying null-check/lazy-initialization requirements even when we intended to fail out. This generally should not affect our behavior for valid requests. Interestingly, there's a test changed in this CL because it appears to have been set up incorrectly, and in the original version of that test we can see that certain Chooser Intents that `ChooserActivity` attempts to reject as invalid, that nevertheless would end up getting handled by the base `ResolverActivity` during the failure flow. Because `ChooserActivity` immediately calls `finish()` as part of this flow, that behavior should only be observable for autolaunch, and otherwise the intent will be rejected as-planned; I conclude that this probably isn't intended as a mechanism to support autolaunch, and so it's OK to "regress" as long as the test is fixed. [EDIT: since this CL was originally uploaded, an intervening change by ayepin@ already fixed this same test.] The CL also cleans up one case I know we had null-checking specifically for this reason; there are probably others left to clean up later. Test: atest IntentResolverUnitTests Bug: 202167050 Change-Id: I28c7b1eff50e516d6bf0043aa141d13f1afce076 --- .../com/android/intentresolver/ChooserActivity.java | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/ChooserActivity.java b/java/src/com/android/intentresolver/ChooserActivity.java index ad106bba..afc3b4bd 100644 --- a/java/src/com/android/intentresolver/ChooserActivity.java +++ b/java/src/com/android/intentresolver/ChooserActivity.java @@ -239,17 +239,12 @@ public class ChooserActivity extends ResolverActivity implements | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION | Intent.FLAG_GRANT_PREFIX_URI_PERMISSION; - /* TODO: this is `nullable` *primarily* because we have to defer the assignment til onCreate(). - * We make the only assignment there, and *expect* it to be ready by the time we ever use it -- + /* TODO: this is `nullable` because we have to defer the assignment til onCreate(). We make the + * only assignment there, and expect it to be ready by the time we ever use it -- * someday if we move all the usage to a component with a narrower lifecycle (something that * matches our Activity's create/destroy lifecycle, not its Java object lifecycle) then we - * should be able to make this assignment as "final." Unfortunately, for now we also have - * a vestigial design where ChooserActivity.onCreate() can invalidate a request, but it still - * has to call up to ResolverActivity.onCreate() before closing, and the base method delegates - * back down to other methods in ChooserActivity that aren't really relevant if we're closing - * (and so they'd normally want to assume it was a valid "creation," with non-null parameters). - * Any client null checks are workarounds for this condition that can be removed once that - * design is cleaned up. */ + * should be able to make this assignment as "final." + */ @Nullable private ChooserRequestParameters mChooserRequest; @@ -333,7 +328,7 @@ public class ChooserActivity extends ResolverActivity implements } catch (IllegalArgumentException e) { Log.e(TAG, "Caller provided invalid Chooser request parameters", e); finish(); - super.onCreate(null); + super_onCreate(null); return; } @@ -1154,10 +1149,6 @@ public class ChooserActivity extends ResolverActivity implements @Override public void addUseDifferentAppLabelIfNecessary(ResolverListAdapter adapter) { - if (mChooserRequest == null) { - return; - } - if (mChooserRequest.getCallerChooserTargets().size() > 0) { mChooserMultiProfilePagerAdapter.getActiveListAdapter().addServiceResults( /* origTarget */ null, -- cgit v1.2.3-59-g8ed1b