diff --git a/java/dagger/android/support/BUILD b/java/dagger/android/support/BUILD index 4b4f473c5a3..8afa703d4ad 100644 --- a/java/dagger/android/support/BUILD +++ b/java/dagger/android/support/BUILD @@ -45,6 +45,7 @@ android_library( "@maven//:androidx_annotation_annotation", "@maven//:androidx_appcompat_appcompat", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", ], diff --git a/java/dagger/hilt/android/BUILD b/java/dagger/hilt/android/BUILD index 5e82837e906..e9efe2cb9dd 100644 --- a/java/dagger/hilt/android/BUILD +++ b/java/dagger/hilt/android/BUILD @@ -44,6 +44,7 @@ android_library( "@maven//:androidx_activity_activity", "@maven//:androidx_annotation_annotation", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", ], @@ -82,6 +83,7 @@ android_library( "@maven//:androidx_activity_activity", "@maven//:androidx_annotation_annotation", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", ], @@ -100,6 +102,7 @@ android_library( "@google_bazel_common//third_party/java/jsr305_annotations", "@maven//:androidx_activity_activity", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", ], @@ -196,6 +199,7 @@ gen_maven_artifact( "androidx.activity:activity", "androidx.annotation:annotation", "androidx.fragment:fragment", + "androidx.lifecycle:lifecycle-common", "androidx.lifecycle:lifecycle-viewmodel", "androidx.lifecycle:lifecycle-viewmodel-savedstate", "androidx.savedstate:savedstate", diff --git a/java/dagger/hilt/android/internal/builders/BUILD b/java/dagger/hilt/android/internal/builders/BUILD index 1de877a3601..81d40bdc650 100644 --- a/java/dagger/hilt/android/internal/builders/BUILD +++ b/java/dagger/hilt/android/internal/builders/BUILD @@ -27,6 +27,7 @@ android_library( "//java/dagger/hilt/android/components:view_model_component", "@maven//:androidx_activity_activity", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", ], diff --git a/java/dagger/hilt/android/internal/lifecycle/BUILD b/java/dagger/hilt/android/internal/lifecycle/BUILD index b3ecfc22ee2..67f4e1f9c60 100644 --- a/java/dagger/hilt/android/internal/lifecycle/BUILD +++ b/java/dagger/hilt/android/internal/lifecycle/BUILD @@ -31,6 +31,7 @@ android_library( "@maven//:androidx_activity_activity", "@maven//:androidx_annotation_annotation", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", "@maven//:androidx_savedstate_savedstate", diff --git a/java/dagger/hilt/android/internal/managers/BUILD b/java/dagger/hilt/android/internal/managers/BUILD index f47701ddf7f..dee0f398e90 100644 --- a/java/dagger/hilt/android/internal/managers/BUILD +++ b/java/dagger/hilt/android/internal/managers/BUILD @@ -50,6 +50,7 @@ android_library( "@maven//:androidx_activity_activity", "@maven//:androidx_annotation_annotation", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", ], diff --git a/java/dagger/hilt/android/internal/managers/ViewComponentManager.java b/java/dagger/hilt/android/internal/managers/ViewComponentManager.java index cb2ece0c8a1..4ed79f7c203 100644 --- a/java/dagger/hilt/android/internal/managers/ViewComponentManager.java +++ b/java/dagger/hilt/android/internal/managers/ViewComponentManager.java @@ -16,6 +16,9 @@ package dagger.hilt.android.internal.managers; +import androidx.lifecycle.Lifecycle; +import androidx.lifecycle.LifecycleEventObserver; +import androidx.lifecycle.LifecycleOwner; import android.content.Context; import android.content.ContextWrapper; import androidx.fragment.app.Fragment; @@ -104,7 +107,7 @@ private GeneratedComponentManager getParentComponentManager(boolean allowMiss if (context instanceof FragmentContextWrapper) { FragmentContextWrapper fragmentContextWrapper = (FragmentContextWrapper) context; - return (GeneratedComponentManager) fragmentContextWrapper.fragment; + return (GeneratedComponentManager) fragmentContextWrapper.getFragment(); } else if (allowMissing) { // We didn't find anything, so return null if we're not supposed to fail. // The rest of the logic is just about getting a good error message. @@ -167,20 +170,38 @@ private static Context unwrap(Context context, Class target) { */ // This is only non-final for the account override public static final class FragmentContextWrapper extends ContextWrapper { + private Fragment fragment; private LayoutInflater baseInflater; private LayoutInflater inflater; - public final Fragment fragment; - - public FragmentContextWrapper(Context base, Fragment fragment) { + private final LifecycleEventObserver fragmentLifecycleObserver = + new LifecycleEventObserver() { + @Override + public void onStateChanged(LifecycleOwner source, Lifecycle.Event event) { + if (event == Lifecycle.Event.ON_DESTROY) { + // Prevent the fragment from leaking if the view outlives the fragment. + // See https://github.com/google/dagger/issues/2070 + FragmentContextWrapper.this.fragment = null; + } + } + }; + + FragmentContextWrapper(Context base, Fragment fragment) { super(Preconditions.checkNotNull(base)); this.baseInflater = null; this.fragment = Preconditions.checkNotNull(fragment); + this.fragment.getLifecycle().addObserver(fragmentLifecycleObserver); } - public FragmentContextWrapper(LayoutInflater baseInflater, Fragment fragment) { + FragmentContextWrapper(LayoutInflater baseInflater, Fragment fragment) { super(Preconditions.checkNotNull(Preconditions.checkNotNull(baseInflater).getContext())); this.baseInflater = baseInflater; this.fragment = Preconditions.checkNotNull(fragment); + this.fragment.getLifecycle().addObserver(fragmentLifecycleObserver); + } + + Fragment getFragment() { + Preconditions.checkNotNull(fragment, "The fragment has already been destroyed."); + return fragment; } @Override diff --git a/java/dagger/hilt/android/internal/modules/BUILD b/java/dagger/hilt/android/internal/modules/BUILD index 6bc4ddeed1e..c1b639dda7e 100644 --- a/java/dagger/hilt/android/internal/modules/BUILD +++ b/java/dagger/hilt/android/internal/modules/BUILD @@ -28,6 +28,7 @@ android_library( "@maven//:androidx_activity_activity", "@maven//:androidx_annotation_annotation", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", ], diff --git a/java/dagger/hilt/android/migration/BUILD b/java/dagger/hilt/android/migration/BUILD index 16de911b04c..a42f0ac65d1 100644 --- a/java/dagger/hilt/android/migration/BUILD +++ b/java/dagger/hilt/android/migration/BUILD @@ -33,6 +33,7 @@ android_library( "@maven//:androidx_activity_activity", "@maven//:androidx_annotation_annotation", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", ], diff --git a/java/dagger/hilt/android/testing/BUILD b/java/dagger/hilt/android/testing/BUILD index d63cd3aba9b..9a229088e95 100644 --- a/java/dagger/hilt/android/testing/BUILD +++ b/java/dagger/hilt/android/testing/BUILD @@ -212,6 +212,7 @@ gen_maven_artifact( "androidx.activity:activity", "androidx.annotation:annotation", "androidx.fragment:fragment", + "androidx.lifecycle:lifecycle-common", "androidx.lifecycle:lifecycle-viewmodel", "androidx.lifecycle:lifecycle-viewmodel-savedstate", "androidx.multidex:multidex", diff --git a/javatests/dagger/android/processor/BUILD b/javatests/dagger/android/processor/BUILD index 5e3b042d405..c639a419123 100644 --- a/javatests/dagger/android/processor/BUILD +++ b/javatests/dagger/android/processor/BUILD @@ -38,6 +38,7 @@ GenJavaTests( "@google_bazel_common//third_party/java/truth", "@maven//:androidx_activity_activity", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", ], diff --git a/javatests/dagger/android/support/BUILD b/javatests/dagger/android/support/BUILD index f10820be51f..2ef1ece6eda 100644 --- a/javatests/dagger/android/support/BUILD +++ b/javatests/dagger/android/support/BUILD @@ -40,6 +40,7 @@ GenRobolectricTests( "@maven//:androidx_activity_activity", "@maven//:androidx_appcompat_appcompat", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", ], diff --git a/javatests/dagger/android/support/functional/BUILD b/javatests/dagger/android/support/functional/BUILD index 1cf34fab8fe..459d55fee4f 100644 --- a/javatests/dagger/android/support/functional/BUILD +++ b/javatests/dagger/android/support/functional/BUILD @@ -31,6 +31,7 @@ android_library( deps = [ "@maven//:androidx_activity_activity", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", "@maven//:androidx_appcompat_appcompat", @@ -57,6 +58,7 @@ GenRobolectricTests( "@google_bazel_common//third_party/java/truth", "@maven//:androidx_activity_activity", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", "@maven//:androidx_test_core", diff --git a/javatests/dagger/hilt/android/BUILD b/javatests/dagger/hilt/android/BUILD index 7563f370bff..982862ff2ea 100644 --- a/javatests/dagger/hilt/android/BUILD +++ b/javatests/dagger/hilt/android/BUILD @@ -298,6 +298,7 @@ android_local_test( "@google_bazel_common//third_party/java/truth", "@maven//:androidx_activity_activity", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", "@maven//:junit_junit", @@ -322,6 +323,7 @@ android_local_test( "@google_bazel_common//third_party/java/truth", "@maven//:androidx_activity_activity", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", "@maven//:junit_junit", @@ -358,6 +360,7 @@ android_local_test( "@google_bazel_common//third_party/java/truth", "@maven//:androidx_activity_activity", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", "@maven//:junit_junit", @@ -383,6 +386,7 @@ android_local_test( "@google_bazel_common//third_party/java/truth", "@maven//:androidx_activity_activity", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", "@maven//:junit_junit", @@ -410,6 +414,7 @@ android_local_test( "@google_bazel_common//third_party/java/truth", "@maven//:androidx_activity_activity", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", "@maven//:junit_junit", @@ -436,6 +441,7 @@ android_local_test( "@google_bazel_common//third_party/java/truth", "@maven//:androidx_activity_activity", "@maven//:androidx_fragment_fragment", + "@maven//:androidx_lifecycle_lifecycle_common", "@maven//:androidx_lifecycle_lifecycle_viewmodel", "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate", "@maven//:junit_junit", diff --git a/javatests/dagger/hilt/android/internal/managers/AndroidManifest.xml b/javatests/dagger/hilt/android/internal/managers/AndroidManifest.xml new file mode 100644 index 00000000000..cc5128efd8b --- /dev/null +++ b/javatests/dagger/hilt/android/internal/managers/AndroidManifest.xml @@ -0,0 +1,11 @@ + + + + + + + + diff --git a/javatests/dagger/hilt/android/internal/managers/BUILD b/javatests/dagger/hilt/android/internal/managers/BUILD new file mode 100644 index 00000000000..ff4f432f7dd --- /dev/null +++ b/javatests/dagger/hilt/android/internal/managers/BUILD @@ -0,0 +1,39 @@ +# Copyright (C) 2021 The Dagger Authors. +# +# 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. +# Description: +# Tests for internal code for implementing Hilt processors. + +package(default_visibility = ["//:src"]) + +android_local_test( + name = "FragmentContextWrapperLeakTest", + size = "small", + srcs = ["FragmentContextWrapperLeakTest.java"], + manifest = "AndroidManifest.xml", + manifest_values = { + "minSdkVersion": "14", + }, + deps = [ + "//:android_local_test_exports", + "//:dagger_with_compiler", + "@google_bazel_common//third_party/java/jsr330_inject", + "@maven//:junit_junit", + "@google_bazel_common//third_party/java/truth", + "//java/dagger/hilt:entry_point", + "//java/dagger/hilt:install_in", + "//java/dagger/hilt/android:android_entry_point", + "//java/dagger/hilt/android/lifecycle", + "//java/dagger/hilt/android/testing:hilt_android_test", + ], +) diff --git a/javatests/dagger/hilt/android/internal/managers/FragmentContextWrapperLeakTest.java b/javatests/dagger/hilt/android/internal/managers/FragmentContextWrapperLeakTest.java new file mode 100644 index 00000000000..38086c6aaa6 --- /dev/null +++ b/javatests/dagger/hilt/android/internal/managers/FragmentContextWrapperLeakTest.java @@ -0,0 +1,74 @@ +/* + * Copyright (C) 2021 The Dagger Authors. + * + * 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 dagger.hilt.android.internal.managers; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import androidx.lifecycle.Lifecycle; +import android.os.Build; +import androidx.fragment.app.Fragment; +import androidx.fragment.app.FragmentActivity; +import androidx.test.core.app.ActivityScenario; +import androidx.test.ext.junit.runners.AndroidJUnit4; +import dagger.hilt.android.AndroidEntryPoint; +import dagger.hilt.android.internal.managers.ViewComponentManager.FragmentContextWrapper; +import dagger.hilt.android.testing.HiltAndroidRule; +import dagger.hilt.android.testing.HiltAndroidTest; +import dagger.hilt.android.testing.HiltTestApplication; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.annotation.Config; + +@HiltAndroidTest +@RunWith(AndroidJUnit4.class) +// Robolectric requires Java9 to run API 29 and above, so use API 28 instead +@Config(sdk = Build.VERSION_CODES.P, application = HiltTestApplication.class) +public final class FragmentContextWrapperLeakTest { + /** An activity to test injection. */ + @AndroidEntryPoint(FragmentActivity.class) + public static final class TestActivity extends Hilt_FragmentContextWrapperLeakTest_TestActivity {} + + /** A fragment to test injection. */ + @AndroidEntryPoint(Fragment.class) + public static final class TestFragment extends Hilt_FragmentContextWrapperLeakTest_TestFragment {} + + @Rule public HiltAndroidRule hiltRule = new HiltAndroidRule(this); + + @Test + public void testFragmentContextWrapperDoesNotLeakFragment() { + try (ActivityScenario scenario = ActivityScenario.launch(TestActivity.class)) { + TestFragment fragment = new TestFragment(); + scenario.onActivity( + activity -> + activity + .getSupportFragmentManager() + .beginTransaction() + .add(fragment, "TestFragment") + .commitNow()); + + FragmentContextWrapper fragmentContextWrapper = + (FragmentContextWrapper) fragment.getContext(); + assertThat(fragmentContextWrapper.getFragment()).isEqualTo(fragment); + scenario.moveToState(Lifecycle.State.DESTROYED); + NullPointerException exception = + assertThrows(NullPointerException.class, fragmentContextWrapper::getFragment); + assertThat(exception).hasMessageThat().contains("The fragment has already been destroyed"); + } + } +}