From 39553a1b848ee573acb306e3297688df096f404f Mon Sep 17 00:00:00 2001 From: mrenouf Date: Sun, 3 Mar 2024 12:10:02 -0500 Subject: Fix CreationExtras.addDefaultArgs when args absent addDefaultArgs grabs the existing Bundle from CreationExtras and adds in the additional values. If default args aren't present (whenever the initial intent did not have extras), the updated bundle was never returned within the original instance since it was relying on modifying the Bundle in-place. This corrects the issue by explicitly returning a new instance containing the updated `DEFAULT_ARGS` Bundle. Test: atest com.android.intentresolver.v2.ext.CreationExtrasExtTest Change-Id: I5a7c61baaaeefa2878b4041d809c348bf5ac70c0 --- .../com/android/intentresolver/v2/ext/CreationExtrasExt.kt | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/v2/ext/CreationExtrasExt.kt b/java/src/com/android/intentresolver/v2/ext/CreationExtrasExt.kt index ebd613f1..6c36e6aa 100644 --- a/java/src/com/android/intentresolver/v2/ext/CreationExtrasExt.kt +++ b/java/src/com/android/intentresolver/v2/ext/CreationExtrasExt.kt @@ -18,14 +18,17 @@ package com.android.intentresolver.v2.ext import android.os.Bundle import android.os.Parcelable +import androidx.core.os.bundleOf import androidx.lifecycle.DEFAULT_ARGS_KEY import androidx.lifecycle.viewmodel.CreationExtras +import androidx.lifecycle.viewmodel.MutableCreationExtras -/** Adds one or more key-value pairs to the default Args bundle in this extras instance. */ +/** + * Returns a new instance with additional [values] added to the existing default args Bundle (if + * present), otherwise adds a new entry with a copy of this bundle. + */ fun CreationExtras.addDefaultArgs(vararg values: Pair): CreationExtras { val defaultArgs: Bundle = get(DEFAULT_ARGS_KEY) ?: Bundle() - for ((key, value) in values) { - defaultArgs.putParcelable(key, value) - } - return this + defaultArgs.putAll(bundleOf(*values)) + return MutableCreationExtras(this).apply { set(DEFAULT_ARGS_KEY, defaultArgs) } } -- cgit v1.2.3-59-g8ed1b From be63632a01b639a7a169da21f5996796c588fa5d Mon Sep 17 00:00:00 2001 From: mrenouf Date: Sun, 3 Mar 2024 14:52:03 -0500 Subject: Create ActivityModel directly instead of injecting There is an initialization order issue with the existing approach: Within the Hilt framework, the ViewModelComponent is initialized prior to injecting the Activity. The ViewModel cannot then depend on any injected values since they will not be available when required. Due to another bug, this was not causing a problem because the value of the injected field was never being checked when the ViewModel was requested in onCreate. The value was inserted on the second request for the ViewModel, after onCreate, and this would mutate the state of the default args Bundle in the already created instance's SavedStateHandle. (Fixed in ag/26434053) The @ActivityScoped Module providing the ActivityModel is replaced with a simple method which creates an instaance from the Activity. The ActivityModel is then available from the ViewModel. This method exists to allows for changing the launchedFromUid, launchedFromPackage, and referrer values without dependence on the system. Bug: 300157408 Test: atest IntentResolver-tests-activity:com.android.intentresolver.v2 Change-Id: I297cbd602c13462f0c0614a279655f2852658dc4 --- .../android/intentresolver/v2/ChooserActivity.java | 22 +++-- .../intentresolver/v2/ResolverActivity.java | 12 ++- .../intentresolver/v2/ui/model/ActivityModel.kt | 12 +++ .../v2/ui/model/ActivityModelModule.kt | 43 --------- .../v2/ui/viewmodel/ChooserViewModel.kt | 8 +- .../v2/ui/model/TestActivityModelModule.kt | 45 --------- .../v2/ui/model/ActivityLaunchTest.kt | 107 --------------------- .../v2/ui/model/ActivityModelTest.kt | 107 +++++++++++++++++++++ 8 files changed, 146 insertions(+), 210 deletions(-) delete mode 100644 java/src/com/android/intentresolver/v2/ui/model/ActivityModelModule.kt delete mode 100644 tests/activity/src/com/android/intentresolver/v2/ui/model/TestActivityModelModule.kt delete mode 100644 tests/unit/src/com/android/intentresolver/v2/ui/model/ActivityLaunchTest.kt create mode 100644 tests/unit/src/com/android/intentresolver/v2/ui/model/ActivityModelTest.kt (limited to 'java/src') diff --git a/java/src/com/android/intentresolver/v2/ChooserActivity.java b/java/src/com/android/intentresolver/v2/ChooserActivity.java index 765d7c2d..8387212a 100644 --- a/java/src/com/android/intentresolver/v2/ChooserActivity.java +++ b/java/src/com/android/intentresolver/v2/ChooserActivity.java @@ -31,6 +31,7 @@ import static androidx.lifecycle.LifecycleKt.getCoroutineScope; import static com.android.intentresolver.contentpreview.ContentPreviewType.CONTENT_PREVIEW_PAYLOAD_SELECTION; import static com.android.intentresolver.v2.ext.CreationExtrasExtKt.addDefaultArgs; +import static com.android.intentresolver.v2.ui.model.ActivityModel.ACTIVITY_MODEL_KEY; import static com.android.internal.annotations.VisibleForTesting.Visibility.PROTECTED; import static com.android.internal.util.LatencyTracker.ACTION_LOAD_SHARE_SHEET; @@ -274,7 +275,6 @@ public class ChooserActivity extends Hilt_ChooserActivity implements private static final int SCROLL_STATUS_SCROLLING_HORIZONTAL = 2; @Inject public ChooserHelper mChooserHelper; - @Inject public ActivityModel mActivityModel; @Inject public FeatureFlags mFeatureFlags; @Inject public android.service.chooser.FeatureFlags mChooserServiceFeatureFlags; @Inject public EventLog mEventLog; @@ -333,7 +333,13 @@ public class ChooserActivity extends Hilt_ChooserActivity implements private boolean mFinishWhenStopped = false; private final AtomicLong mIntentReceivedTime = new AtomicLong(-1); + + protected ActivityModel createActivityModel() { + return ActivityModel.createFrom(this); + } + private ChooserViewModel mViewModel; + private ActivityModel mActivityModel; @VisibleForTesting protected ChooserActivityLogic createActivityLogic() { @@ -348,30 +354,32 @@ public class ChooserActivity extends Hilt_ChooserActivity implements public CreationExtras getDefaultViewModelCreationExtras() { return addDefaultArgs( super.getDefaultViewModelCreationExtras(), - new Pair<>(ActivityModel.ACTIVITY_MODEL_KEY, mActivityModel)); + new Pair<>(ACTIVITY_MODEL_KEY, createActivityModel())); } @Override protected final void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); Log.i(TAG, "onCreate"); - Log.i(TAG, "mActivityModel=" + mActivityModel.toString()); - - // The postInit hook is invoked when this function returns, via Lifecycle. - mChooserHelper.setPostCreateCallback(this::init); + mViewModel = new ViewModelProvider(this).get(ChooserViewModel.class); + mActivityModel = mViewModel.getActivityModel(); int callerUid = mActivityModel.getLaunchedFromUid(); if (callerUid < 0 || UserHandle.isIsolated(callerUid)) { Log.e(TAG, "Can't start a resolver from uid " + callerUid); finish(); } + setTheme(R.style.Theme_DeviceDefault_Chooser); Tracer.INSTANCE.markLaunched(); - mViewModel = new ViewModelProvider(this).get(ChooserViewModel.class); if (!mViewModel.init()) { finish(); return; } + + // The post-create callback is invoked when this function returns, via Lifecycle. + mChooserHelper.setPostCreateCallback(this::init); + IntentSender chosenComponentSender = mViewModel.getChooserRequest().getChosenComponentSender(); if (chosenComponentSender != null) { diff --git a/java/src/com/android/intentresolver/v2/ResolverActivity.java b/java/src/com/android/intentresolver/v2/ResolverActivity.java index d985a161..a9d9f8b1 100644 --- a/java/src/com/android/intentresolver/v2/ResolverActivity.java +++ b/java/src/com/android/intentresolver/v2/ResolverActivity.java @@ -154,11 +154,10 @@ public class ResolverActivity extends Hilt_ResolverActivity implements ResolverListAdapter.ResolverListCommunicator { @Inject public PackageManager mPackageManager; - @Inject public ActivityModel mActivityModel; @Inject public DevicePolicyResources mDevicePolicyResources; @Inject public IntentForwarding mIntentForwarding; private ResolverRequest mResolverRequest; - + private ActivityModel mActivityModel; protected ActivityLogic mLogic; protected TargetDataLoader mTargetDataLoader; private boolean mResolvingHome; @@ -218,6 +217,9 @@ public class ResolverActivity extends Hilt_ResolverActivity implements } }; } + protected ActivityModel createActivityModel() { + return ActivityModel.createFrom(this); + } @VisibleForTesting protected ActivityLogic createActivityLogic() { @@ -232,15 +234,17 @@ public class ResolverActivity extends Hilt_ResolverActivity implements public CreationExtras getDefaultViewModelCreationExtras() { return addDefaultArgs( super.getDefaultViewModelCreationExtras(), - new Pair<>(ActivityModel.ACTIVITY_MODEL_KEY, mActivityModel)); + new Pair<>(ActivityModel.ACTIVITY_MODEL_KEY, ActivityModel.createFrom(this))); } @Override protected final void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); setTheme(R.style.Theme_DeviceDefault_Resolver); + mActivityModel = createActivityModel(); + Log.i(TAG, "onCreate"); - Log.i(TAG, "activityLaunch=" + mActivityModel.toString()); + Log.i(TAG, "activityModel=" + mActivityModel.toString()); int callerUid = mActivityModel.getLaunchedFromUid(); if (callerUid < 0 || UserHandle.isIsolated(callerUid)) { Log.e(TAG, "Can't start a resolver from uid " + callerUid); diff --git a/java/src/com/android/intentresolver/v2/ui/model/ActivityModel.kt b/java/src/com/android/intentresolver/v2/ui/model/ActivityModel.kt index 02bb6640..07b17435 100644 --- a/java/src/com/android/intentresolver/v2/ui/model/ActivityModel.kt +++ b/java/src/com/android/intentresolver/v2/ui/model/ActivityModel.kt @@ -15,12 +15,14 @@ */ package com.android.intentresolver.v2.ui.model +import android.app.Activity import android.content.Intent import android.net.Uri import android.os.Parcel import android.os.Parcelable import com.android.intentresolver.v2.ext.readParcelable import com.android.intentresolver.v2.ext.requireParcelable +import java.util.Objects /** Contains Activity-scope information about the state when started. */ data class ActivityModel( @@ -64,5 +66,15 @@ data class ActivityModel( override fun newArray(size: Int) = arrayOfNulls(size) override fun createFromParcel(source: Parcel) = ActivityModel(source) } + + @JvmStatic + fun createFrom(activity: Activity): ActivityModel { + return ActivityModel( + activity.intent, + activity.launchedFromUid, + Objects.requireNonNull(activity.launchedFromPackage), + activity.referrer + ) + } } } diff --git a/java/src/com/android/intentresolver/v2/ui/model/ActivityModelModule.kt b/java/src/com/android/intentresolver/v2/ui/model/ActivityModelModule.kt deleted file mode 100644 index d9fb1fa6..00000000 --- a/java/src/com/android/intentresolver/v2/ui/model/ActivityModelModule.kt +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.v2.ui.model - -import android.app.Activity -import dagger.Module -import dagger.Provides -import dagger.hilt.InstallIn -import dagger.hilt.android.components.ActivityComponent -import dagger.hilt.android.scopes.ActivityScoped - -@Module -@InstallIn(ActivityComponent::class) -object ActivityModelModule { - - @Provides - @ActivityScoped - fun activityModel(activity: Activity): ActivityModel { - return ActivityModel( - intent = activity.intent, - launchedFromUid = activity.launchedFromUid, - launchedFromPackage = requireNotNull(activity.launchedFromPackage) { - "activity.launchedFromPackage was null. This is expected to be non-null for " + - "any system-signed application!" - }, - referrer = activity.referrer - ) - } -} diff --git a/java/src/com/android/intentresolver/v2/ui/viewmodel/ChooserViewModel.kt b/java/src/com/android/intentresolver/v2/ui/viewmodel/ChooserViewModel.kt index 424f36cd..8ed2fa29 100644 --- a/java/src/com/android/intentresolver/v2/ui/viewmodel/ChooserViewModel.kt +++ b/java/src/com/android/intentresolver/v2/ui/viewmodel/ChooserViewModel.kt @@ -39,17 +39,17 @@ constructor( flags: ChooserServiceFlags, ) : ViewModel() { - private val mActivityModel: ActivityModel = + /** Parcelable-only references provided from the creating Activity */ + val activityModel: ActivityModel = requireNotNull(args[ACTIVITY_MODEL_KEY]) { "ActivityModel missing in SavedStateHandle! ($ACTIVITY_MODEL_KEY)" } /** The result of reading and validating the inputs provided in savedState. */ - private val status: ValidationResult = - readChooserRequest(mActivityModel, flags) + private val status: ValidationResult = readChooserRequest(activityModel, flags) val chooserRequest: ChooserRequest by lazy { - when(status) { + when (status) { is Valid -> status.value is Invalid -> error(status.errors) } diff --git a/tests/activity/src/com/android/intentresolver/v2/ui/model/TestActivityModelModule.kt b/tests/activity/src/com/android/intentresolver/v2/ui/model/TestActivityModelModule.kt deleted file mode 100644 index 7d05dc0f..00000000 --- a/tests/activity/src/com/android/intentresolver/v2/ui/model/TestActivityModelModule.kt +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.android.intentresolver.v2.ui.model - -import android.app.Activity -import android.net.Uri -import dagger.Module -import dagger.Provides -import dagger.hilt.android.components.ActivityComponent -import dagger.hilt.android.scopes.ActivityScoped -import dagger.hilt.testing.TestInstallIn - -@Module -@TestInstallIn(components = [ActivityComponent::class], replaces = [ActivityModelModule::class]) -class TestActivityModelModule { - - @Provides - @ActivityScoped - fun activityModel(activity: Activity): ActivityModel { - return ActivityModel( - intent = activity.intent, - launchedFromUid = LAUNCHED_FROM_UID, - launchedFromPackage = LAUNCHED_FROM_PACKAGE, - referrer = REFERRER) - } - - companion object { - const val LAUNCHED_FROM_PACKAGE = "example.com" - const val LAUNCHED_FROM_UID = 1234 - val REFERRER: Uri = Uri.fromParts(ANDROID_APP_SCHEME, LAUNCHED_FROM_PACKAGE, "") - } -} diff --git a/tests/unit/src/com/android/intentresolver/v2/ui/model/ActivityLaunchTest.kt b/tests/unit/src/com/android/intentresolver/v2/ui/model/ActivityLaunchTest.kt deleted file mode 100644 index e30cd81a..00000000 --- a/tests/unit/src/com/android/intentresolver/v2/ui/model/ActivityLaunchTest.kt +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright (C) 2024 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.intentresolver.v2.ui.model - -import android.content.Intent -import android.content.Intent.ACTION_CHOOSER -import android.content.Intent.EXTRA_TEXT -import android.net.Uri -import com.android.intentresolver.v2.ext.toParcelAndBack -import com.google.common.truth.Truth.assertThat -import com.google.common.truth.Truth.assertWithMessage -import org.junit.Test - -class ActivityLaunchTest { - - @Test - fun testDefaultValues() { - val input = ActivityModel(Intent(ACTION_CHOOSER), 0, "example.com", null) - - val output = input.toParcelAndBack() - - assertEquals(input, output) - } - - @Test - fun testCommonValues() { - val intent = Intent(ACTION_CHOOSER).apply { putExtra(EXTRA_TEXT, "Test") } - val input = - ActivityModel(intent, 1234, "com.example", Uri.parse("android-app://example.com")) - - val output = input.toParcelAndBack() - - assertEquals(input, output) - } - - @Test - fun testReferrerPackage_withAppReferrer_usesReferrer() { - val launch1 = - ActivityModel( - intent = Intent(), - launchedFromUid = 1000, - launchedFromPackage = "other.example.com", - referrer = Uri.parse("android-app://app.example.com") - ) - - assertThat(launch1.referrerPackage).isEqualTo("app.example.com") - } - - @Test - fun testReferrerPackage_httpReferrer_isNull() { - val launch = - ActivityModel( - intent = Intent(), - launchedFromUid = 1000, - launchedFromPackage = "example.com", - referrer = Uri.parse("http://some.other.value") - ) - - assertThat(launch.referrerPackage).isNull() - } - - @Test - fun testReferrerPackage_nullReferrer_isNull() { - val launch = - ActivityModel( - intent = Intent(), - launchedFromUid = 1000, - launchedFromPackage = "example.com", - referrer = null - ) - - assertThat(launch.referrerPackage).isNull() - } - - private fun assertEquals(expected: ActivityModel, actual: ActivityModel) { - // Test fields separately: Intent does not override equals() - assertWithMessage("%s.filterEquals(%s)", actual.intent, expected.intent) - .that(actual.intent.filterEquals(expected.intent)) - .isTrue() - - assertWithMessage("actual fromUid is equal to expected") - .that(actual.launchedFromUid) - .isEqualTo(expected.launchedFromUid) - - assertWithMessage("actual fromPackage is equal to expected") - .that(actual.launchedFromPackage) - .isEqualTo(expected.launchedFromPackage) - - assertWithMessage("actual referrer is equal to expected") - .that(actual.referrer) - .isEqualTo(expected.referrer) - } -} diff --git a/tests/unit/src/com/android/intentresolver/v2/ui/model/ActivityModelTest.kt b/tests/unit/src/com/android/intentresolver/v2/ui/model/ActivityModelTest.kt new file mode 100644 index 00000000..049fa001 --- /dev/null +++ b/tests/unit/src/com/android/intentresolver/v2/ui/model/ActivityModelTest.kt @@ -0,0 +1,107 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.intentresolver.v2.ui.model + +import android.content.Intent +import android.content.Intent.ACTION_CHOOSER +import android.content.Intent.EXTRA_TEXT +import android.net.Uri +import com.android.intentresolver.v2.ext.toParcelAndBack +import com.google.common.truth.Truth.assertThat +import com.google.common.truth.Truth.assertWithMessage +import org.junit.Test + +class ActivityModelTest { + + @Test + fun testDefaultValues() { + val input = ActivityModel(Intent(ACTION_CHOOSER), 0, "example.com", null) + + val output = input.toParcelAndBack() + + assertEquals(input, output) + } + + @Test + fun testCommonValues() { + val intent = Intent(ACTION_CHOOSER).apply { putExtra(EXTRA_TEXT, "Test") } + val input = + ActivityModel(intent, 1234, "com.example", Uri.parse("android-app://example.com")) + + val output = input.toParcelAndBack() + + assertEquals(input, output) + } + + @Test + fun testReferrerPackage_withAppReferrer_usesReferrer() { + val launch1 = + ActivityModel( + intent = Intent(), + launchedFromUid = 1000, + launchedFromPackage = "other.example.com", + referrer = Uri.parse("android-app://app.example.com") + ) + + assertThat(launch1.referrerPackage).isEqualTo("app.example.com") + } + + @Test + fun testReferrerPackage_httpReferrer_isNull() { + val launch = + ActivityModel( + intent = Intent(), + launchedFromUid = 1000, + launchedFromPackage = "example.com", + referrer = Uri.parse("http://some.other.value") + ) + + assertThat(launch.referrerPackage).isNull() + } + + @Test + fun testReferrerPackage_nullReferrer_isNull() { + val launch = + ActivityModel( + intent = Intent(), + launchedFromUid = 1000, + launchedFromPackage = "example.com", + referrer = null + ) + + assertThat(launch.referrerPackage).isNull() + } + + private fun assertEquals(expected: ActivityModel, actual: ActivityModel) { + // Test fields separately: Intent does not override equals() + assertWithMessage("%s.filterEquals(%s)", actual.intent, expected.intent) + .that(actual.intent.filterEquals(expected.intent)) + .isTrue() + + assertWithMessage("actual fromUid is equal to expected") + .that(actual.launchedFromUid) + .isEqualTo(expected.launchedFromUid) + + assertWithMessage("actual fromPackage is equal to expected") + .that(actual.launchedFromPackage) + .isEqualTo(expected.launchedFromPackage) + + assertWithMessage("actual referrer is equal to expected") + .that(actual.referrer) + .isEqualTo(expected.referrer) + } +} -- cgit v1.2.3-59-g8ed1b