From 5ce0ceadd3a9da609171d7382a4ea7d8437a7e03 Mon Sep 17 00:00:00 2001 From: Eric Chang Date: Tue, 25 May 2021 14:54:20 -0700 Subject: [PATCH] Adds the dagger.hilt.android.useFragmentGetContextFix flag to guard the corrected Fragment.getContext() behavior. RELNOTES=Adds the dagger.hilt.android.useFragmentGetContextFix flag to guard the corrected Fragment.getContext() behavior PiperOrigin-RevId: 375801929 --- .../internal/androidentrypoint/BUILD | 1 + .../androidentrypoint/FragmentGenerator.java | 19 +++++++++++++++---- .../internal/HiltCompilerOptions.java | 14 +++++++++++++- .../android/FragmentContextOnAttachTest.java | 12 ++++++++++++ 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/java/dagger/hilt/android/processor/internal/androidentrypoint/BUILD b/java/dagger/hilt/android/processor/internal/androidentrypoint/BUILD index ee35b89829a..4159b7047cc 100644 --- a/java/dagger/hilt/android/processor/internal/androidentrypoint/BUILD +++ b/java/dagger/hilt/android/processor/internal/androidentrypoint/BUILD @@ -63,6 +63,7 @@ java_library( "//java/dagger/hilt/android/processor/internal:android_classnames", "//java/dagger/hilt/android/processor/internal:utils", "//java/dagger/hilt/processor/internal:classnames", + "//java/dagger/hilt/processor/internal:compiler_options", "//java/dagger/hilt/processor/internal:component_names", "//java/dagger/hilt/processor/internal:processor_errors", "//java/dagger/hilt/processor/internal:processors", diff --git a/java/dagger/hilt/android/processor/internal/androidentrypoint/FragmentGenerator.java b/java/dagger/hilt/android/processor/internal/androidentrypoint/FragmentGenerator.java index ed22fdd3c67..0a5641d2eba 100644 --- a/java/dagger/hilt/android/processor/internal/androidentrypoint/FragmentGenerator.java +++ b/java/dagger/hilt/android/processor/internal/androidentrypoint/FragmentGenerator.java @@ -16,6 +16,8 @@ package dagger.hilt.android.processor.internal.androidentrypoint; +import static dagger.hilt.processor.internal.HiltCompilerOptions.useFragmentGetContextFix; + import com.squareup.javapoet.ClassName; import com.squareup.javapoet.FieldSpec; import com.squareup.javapoet.JavaFile; @@ -176,12 +178,21 @@ private MethodSpec initializeComponentContextMethod() { // initializeComponentContext(); // return componentContext; // } - private static MethodSpec getContextMethod() { - return MethodSpec.methodBuilder("getContext") + private MethodSpec getContextMethod() { + MethodSpec.Builder builder = MethodSpec.methodBuilder("getContext") .returns(AndroidClassNames.CONTEXT) .addAnnotation(Override.class) - .addModifiers(Modifier.PUBLIC) - .beginControlFlow("if (super.getContext() == null)") + .addModifiers(Modifier.PUBLIC); + + if (useFragmentGetContextFix(env)) { + builder.beginControlFlow("if (super.getContext() == null)"); + } else { + builder.beginControlFlow( + "if (super.getContext() == null && $N == null)", + COMPONENT_CONTEXT_FIELD); + } + + return builder .addStatement("return null") .endControlFlow() .addStatement("initializeComponentContext()") diff --git a/java/dagger/hilt/processor/internal/HiltCompilerOptions.java b/java/dagger/hilt/processor/internal/HiltCompilerOptions.java index d8720053710..1ac15f72765 100644 --- a/java/dagger/hilt/processor/internal/HiltCompilerOptions.java +++ b/java/dagger/hilt/processor/internal/HiltCompilerOptions.java @@ -84,6 +84,16 @@ public static boolean useAggregatingRootProcessor(ProcessingEnvironment env) { return BooleanOption.USE_AGGREGATING_ROOT_PROCESSOR.get(env); } + /** + * Returns {@code true} if fragment code should use the fixed getContext() behavior where it + * correctly returns null after a fragment is removed. This fixed behavior matches the behavior + * of a regular fragment and can help catch issues where a removed or leaked fragment is + * incorrectly used. + */ + public static boolean useFragmentGetContextFix(ProcessingEnvironment env) { + return BooleanOption.USE_FRAGMENT_GET_CONTEXT_FIX.get(env); + } + /** Processor options which can have true or false values. */ private enum BooleanOption { /** Do not use! This is for internal use only. */ @@ -97,7 +107,9 @@ private enum BooleanOption { DISABLE_MODULES_HAVE_INSTALL_IN_CHECK("disableModulesHaveInstallInCheck", false), - SHARE_TEST_COMPONENTS("shareTestComponents", false); + SHARE_TEST_COMPONENTS("shareTestComponents", false), + + USE_FRAGMENT_GET_CONTEXT_FIX("android.useFragmentGetContextFix", false); private final String name; private final boolean defaultValue; diff --git a/javatests/dagger/hilt/android/FragmentContextOnAttachTest.java b/javatests/dagger/hilt/android/FragmentContextOnAttachTest.java index 48cc6f236eb..51d33b8e324 100644 --- a/javatests/dagger/hilt/android/FragmentContextOnAttachTest.java +++ b/javatests/dagger/hilt/android/FragmentContextOnAttachTest.java @@ -74,4 +74,16 @@ public void testGetContextAvailableBeforeSuperOnAttach() throws Exception { assertThat(fragment.onAttachContextContext).isNotNull(); assertThat(fragment.onAttachActivityContext).isNotNull(); } + + // Tests the default behavior of the useFragmentGetContextFix flag. Current default is false + // so this tests the broken behavior. + @Test + public void testGetContextDoesNotReturnNullAfterRemoval() throws Exception { + FragmentActivity activity = Robolectric.setupActivity(TestActivity.class); + TestFragment fragment = new TestFragment(); + activity.getSupportFragmentManager().beginTransaction().add(fragment, "").commitNow(); + assertThat(fragment.getContext()).isNotNull(); + activity.getSupportFragmentManager().beginTransaction().remove(fragment).commitNow(); + assertThat(fragment.getContext()).isNotNull(); + } }