Skip to content

Commit

Permalink
Add a runtime flag to control the Fragment.getContext() fix. This run…
Browse files Browse the repository at this point in the history
…time flag is used only if the compiler flag is also on.

RELNOTES=Add a runtime flag for the Fragment.getContext() fix.
PiperOrigin-RevId: 397816397
  • Loading branch information
Chang-Eric authored and Dagger Team committed Sep 20, 2021
1 parent f6a8e0e commit c3f613f
Show file tree
Hide file tree
Showing 11 changed files with 234 additions and 13 deletions.
1 change: 1 addition & 0 deletions java/dagger/hilt/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ filegroup(
srcs = [
"//java/dagger/hilt/android:srcs_filegroup",
"//java/dagger/hilt/android/components:srcs_filegroup",
"//java/dagger/hilt/android/flags:srcs_filegroup",
"//java/dagger/hilt/android/internal:srcs_filegroup",
"//java/dagger/hilt/android/internal/builders:srcs_filegroup",
"//java/dagger/hilt/android/internal/lifecycle:srcs_filegroup",
Expand Down
2 changes: 2 additions & 0 deletions java/dagger/hilt/android/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ android_library(
exports = [
"//java/dagger/hilt:install_in",
"//java/dagger/hilt/android/components",
"//java/dagger/hilt/android/flags:fragment_get_context_fix",
"//java/dagger/hilt/android/internal",
"//java/dagger/hilt/android/internal/builders",
"//java/dagger/hilt/android/internal/managers",
Expand Down Expand Up @@ -184,6 +185,7 @@ gen_maven_artifact(
"//java/dagger/hilt/android:package_info",
"//java/dagger/hilt/android/components",
"//java/dagger/hilt/android/components:package_info",
"//java/dagger/hilt/android/flags:fragment_get_context_fix",
"//java/dagger/hilt/android/internal",
"//java/dagger/hilt/android/internal/builders",
"//java/dagger/hilt/android/internal/earlyentrypoint",
Expand Down
40 changes: 40 additions & 0 deletions java/dagger/hilt/android/flags/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
# 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:
# Runtime flags to control Hilt behavior for rollout of changes. These flags are usually
# meant to be temporary and so defaults may change with releases and then these flags
# may eventually be removed, just like compiler options with similar purposes.

package(default_visibility = ["//:src"])

android_library(
name = "fragment_get_context_fix",
srcs = [
"FragmentGetContextFix.java",
],
deps = [
"//:dagger_with_compiler",
"//java/dagger/hilt:entry_point",
"//java/dagger/hilt:install_in",
"//java/dagger/hilt/android:entry_point_accessors",
"//java/dagger/hilt/android/components",
"//java/dagger/hilt/internal:preconditions",
],
)

filegroup(
name = "srcs_filegroup",
srcs = glob(["*"]),
)
103 changes: 103 additions & 0 deletions java/dagger/hilt/android/flags/FragmentGetContextFix.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* 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.flags;

import android.content.Context;
import dagger.Module;
import dagger.hilt.EntryPoint;
import dagger.hilt.InstallIn;
import dagger.hilt.android.EntryPointAccessors;
import dagger.hilt.components.SingletonComponent;
import dagger.hilt.internal.Preconditions;
import dagger.multibindings.Multibinds;
import java.lang.annotation.ElementType;
import java.lang.annotation.Target;
import java.util.Set;
import javax.inject.Qualifier;

/**
* Runtime flag for the Fragment.getContext() fix. See https://github.com/google/dagger/pull/2620
* for this change. Controls 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, non-Hilt fragment and can help catch issues where a removed or leaked fragment is
* incorrectly used.
*
* <p>This flag is paired with the compiler option flag
* dagger.hilt.android.useFragmentGetContextFix. When that flag is false, this runtime flag has no
* effect on behavior (e.g. the compiler flag being off takes precedence). When the compiler flag is
* on, then the runtime flag may be used to disable the behavior at runtime.
*
* <p>In order to set the flag, bind a boolean value qualified with
* {@link DisableFragmentGetContextFix} into a set in the {@code SingletonComponent}. A set is used
* instead of an optional binding to avoid a dependency on Guava. Only one value may be bound into
* the set within a given app. Example for binding the value:
*
* <pre><code>
* {@literal @}Module
* {@literal @}InstallIn(SingletonComponent.class)
* public final class DisableFragmentGetContextFixModule {
* {@literal @}Provides
* {@literal @}IntoSet
* {@literal @}FragmentGetContextFix.DisableFragmentGetContextFix
* static Boolean provideDisableFragmentGetContextFix() {
* return // true or false depending on some rollout logic for your app
* }
* }
* </code></pre>
*/
public final class FragmentGetContextFix {

/** Qualifier annotation to bind disable the Fragment.getContext() fix at runtime. */
@Target({ElementType.METHOD, ElementType.PARAMETER, ElementType.FIELD})
@Qualifier
public @interface DisableFragmentGetContextFix {}

public static boolean isFragmentGetContextFixDisabled(Context context) {
// Use a set here instead of an optional to avoid the Guava dependency
Set<Boolean> flagSet = EntryPointAccessors.fromApplication(
context, FragmentGetContextFixEntryPoint.class).getDisableFragmentGetContextFix();

// TODO(b/199927963): Consider adding a plugin to check this at compile time
Preconditions.checkState(flagSet.size() <= 1,
"Cannot bind the flag @DisableFragmentGetContextFix more than once.");

if (flagSet.isEmpty()) {
return false;
} else {
return flagSet.iterator().next();
}
}

/** Entry point for getting the flag. */
@EntryPoint
@InstallIn(SingletonComponent.class)
public interface FragmentGetContextFixEntryPoint {
@DisableFragmentGetContextFix Set<Boolean> getDisableFragmentGetContextFix();
}

/** Declare the empty flag set. */
@Module
@InstallIn(SingletonComponent.class)
abstract static class FragmentGetContextFixModule {
@Multibinds
@DisableFragmentGetContextFix
abstract Set<Boolean> disableFragmentGetContextFix();
}

private FragmentGetContextFix() {
}
}
27 changes: 27 additions & 0 deletions java/dagger/hilt/android/flags/package-info.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* 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.
*/

/**
* Runtime flags to control Hilt behavior for rollout of changes. These flags are usually meant to
* be temporary and so defaults may change with releases and then these flags may eventually be
* removed, just like compiler options with similar purposes.
*
* @see <a href="https://dagger.dev/hilt">Hilt Developer Docs</a>
*/
@ParametersAreNonnullByDefault
package dagger.hilt.android.flags;

import javax.annotation.ParametersAreNonnullByDefault;
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ interface HiltExtension {
/**
* If set to `true`, Hilt will disable cross compilation root validation.
*
* See [documentation](https://dagger.dev/hilt/compiler-options#disable-cross-compilation-root-validation)
* See [documentation](https://dagger.dev/hilt/flags#disable-cross-compilation-root-validation)
* for more information.
*/
var disableCrossCompilationRootValidation: Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ public final class AndroidClassNames {
public static final ClassName VIEW_MODEL_COMPONENT =
get("dagger.hilt.android.components", "ViewModelComponent");

public static final ClassName FRAGMENT_GET_CONTEXT_FIX =
get("dagger.hilt.android.flags", "FragmentGetContextFix");

public static final ClassName ACTIVITY_COMPONENT_MANAGER =
get("dagger.hilt.android.internal.managers", "ActivityComponentManager");
public static final ClassName APPLICATION_COMPONENT_MANAGER =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.squareup.javapoet.FieldSpec;
import com.squareup.javapoet.JavaFile;
import com.squareup.javapoet.MethodSpec;
import com.squareup.javapoet.TypeName;
import com.squareup.javapoet.TypeSpec;
import com.squareup.javapoet.TypeVariableName;
import dagger.hilt.android.processor.internal.AndroidClassNames;
Expand All @@ -39,6 +40,11 @@ public final class FragmentGenerator {
.addModifiers(Modifier.PRIVATE)
.build();

private static final FieldSpec DISABLE_GET_CONTEXT_FIX_FIELD =
FieldSpec.builder(TypeName.BOOLEAN, "disableGetContextFix")
.addModifiers(Modifier.PRIVATE)
.build();

private final ProcessingEnvironment env;
private final AndroidEntryPointMetadata metadata;
private final ClassName generatedClassName;
Expand Down Expand Up @@ -74,6 +80,10 @@ TypeSpec createTypeSpec() {
.addMethod(getContextMethod())
.addMethod(inflatorMethod());

if (useFragmentGetContextFix(env)) {
builder.addField(DISABLE_GET_CONTEXT_FIX_FIELD);
}

Generators.addGeneratedBaseClassJavadoc(builder, AndroidClassNames.ANDROID_ENTRY_POINT);
Processors.addGeneratedAnnotation(builder, env, getClass());
Generators.copyLintAnnotations(metadata.element(), builder);
Expand Down Expand Up @@ -158,10 +168,12 @@ private static MethodSpec onAttachActivityMethod() {
// // Fragment's because we are getting it from base context instead of cloning from super
// // Fragment's LayoutInflater.
// componentContext = FragmentComponentManager.createContextWrapper(super.getContext(), this);
// disableGetContextFix = FragmentGetContextFix.isFragmentGetContextFixDisabled(
// super.getContext());
// }
// }
private MethodSpec initializeComponentContextMethod() {
return MethodSpec.methodBuilder("initializeComponentContext")
MethodSpec.Builder builder = MethodSpec.methodBuilder("initializeComponentContext")
.addModifiers(Modifier.PRIVATE)
.beginControlFlow("if ($N == null)", COMPONENT_CONTEXT_FIELD)
.addComment(
Expand All @@ -171,14 +183,22 @@ private MethodSpec initializeComponentContextMethod() {
.addStatement(
"$N = $T.createContextWrapper(super.getContext(), this)",
COMPONENT_CONTEXT_FIELD,
metadata.componentManager())
metadata.componentManager());

if (useFragmentGetContextFix(env)) {
builder.addStatement("$N = $T.isFragmentGetContextFixDisabled(super.getContext())",
DISABLE_GET_CONTEXT_FIX_FIELD,
AndroidClassNames.FRAGMENT_GET_CONTEXT_FIX);
}

return builder
.endControlFlow()
.build();
}

// @Override
// public Context getContext() {
// if (super.getContext() == null) {
// if (super.getContext() == null && !disableGetContextFix) {
// return null;
// }
// initializeComponentContext();
Expand All @@ -191,11 +211,17 @@ private MethodSpec getContextMethod() {
.addModifiers(Modifier.PUBLIC);

if (useFragmentGetContextFix(env)) {
builder.beginControlFlow("if (super.getContext() == null)");
builder
// Note that disableGetContext can only be true if componentContext is set, so if it is
// true we don't need to check whether componentContext is set or not.
.beginControlFlow(
"if (super.getContext() == null && !$N)",
DISABLE_GET_CONTEXT_FIX_FIELD);
} else {
builder.beginControlFlow(
"if (super.getContext() == null && $N == null)",
COMPONENT_CONTEXT_FIELD);
builder
.beginControlFlow(
"if (super.getContext() == null && $N == null)",
COMPONENT_CONTEXT_FIELD);
}

return builder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ private void processModule(
|| installInCheckDisabled(element),
element,
"%s is missing an @InstallIn annotation. If this was intentional, see"
+ " https://dagger.dev/hilt/compiler-options#disable-install-in-check for how to disable this"
+ " https://dagger.dev/hilt/flags#disable-install-in-check for how to disable this"
+ " check.",
element);

Expand Down
4 changes: 4 additions & 0 deletions javatests/dagger/hilt/android/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ android_local_test(
name = "FragmentContextOnAttachTest",
size = "small",
srcs = ["FragmentContextOnAttachTest.java"],
javacopts = [
"-Adagger.hilt.android.useFragmentGetContextFix=true",
],
manifest_values = {
"minSdkVersion": "14",
},
Expand All @@ -198,6 +201,7 @@ android_local_test(
"//java/dagger/hilt:install_in",
"//java/dagger/hilt/android:android_entry_point",
"//java/dagger/hilt/android:package_info",
"//java/dagger/hilt/android/flags:fragment_get_context_fix",
"//java/dagger/hilt/android/testing:bind_value",
"//java/dagger/hilt/android/testing:hilt_android_test",
"@google_bazel_common//third_party/java/jsr330_inject",
Expand Down
23 changes: 19 additions & 4 deletions javatests/dagger/hilt/android/FragmentContextOnAttachTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import androidx.fragment.app.Fragment;
import androidx.fragment.app.FragmentActivity;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import dagger.hilt.android.flags.FragmentGetContextFix;
import dagger.hilt.android.testing.BindValueIntoSet;
import dagger.hilt.android.testing.HiltAndroidRule;
import dagger.hilt.android.testing.HiltAndroidTest;
import dagger.hilt.android.testing.HiltTestApplication;
Expand All @@ -41,6 +43,10 @@ public final class FragmentContextOnAttachTest {

@Rule public final HiltAndroidRule rule = new HiltAndroidRule(this);

@BindValueIntoSet
@FragmentGetContextFix.DisableFragmentGetContextFix
boolean disableGetContextFix = false;

/** Hilt Activity */
@AndroidEntryPoint(FragmentActivity.class)
public static final class TestActivity extends Hilt_FragmentContextOnAttachTest_TestActivity {}
Expand Down Expand Up @@ -75,15 +81,24 @@ public void testGetContextAvailableBeforeSuperOnAttach() throws Exception {
assertThat(fragment.onAttachActivityContext).isNotNull();
}

// Tests the default behavior of the useFragmentGetContextFix flag. Current default is false
// so this tests the broken behavior.
// Tests the behavior when using the useFragmentGetContextFix flag.
@Test
public void testGetContextDoesNotReturnNullAfterRemoval() throws Exception {
public void testGetContextReturnsNullAfterRemoval() 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();
// This should be null since the fix was enabled by the compiler flag and runtime flag
assertThat(fragment.getContext()).isNull();

// Flip the flag so that we now disable the fix
disableGetContextFix = true;
TestFragment fragment2 = new TestFragment();
activity.getSupportFragmentManager().beginTransaction().add(fragment2, "").commitNow();
assertThat(fragment2.getContext()).isNotNull();
activity.getSupportFragmentManager().beginTransaction().remove(fragment2).commitNow();
// This should not be null since the fix was disabled by the runtime flag
assertThat(fragment2.getContext()).isNotNull();
}
}

0 comments on commit c3f613f

Please sign in to comment.