From e802964ddc838b4db776d45689e9dfd6f46a5b2c Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Sun, 24 Jul 2022 11:36:38 -0700 Subject: [PATCH] Deprecate Glide fragments in favor of androidx Lifecycle. --- instrumentation/build.gradle | 3 +- .../glide/RequestManagerLifecycleTest.java | 489 ++++++++++++++++++ instrumentation/src/main/AndroidManifest.xml | 8 +- .../glide/test/DefaultFragmentActivity.java | 15 + .../res/layout/default_fragment_activity.xml | 5 + .../main/java/com/bumptech/glide/Glide.java | 3 + .../java/com/bumptech/glide/GlideBuilder.java | 20 + .../glide/manager/LifecycleLifecycle.java | 63 +++ .../LifecycleRequestManagerRetriever.java | 96 ++++ .../glide/manager/RequestManagerFragment.java | 1 + .../manager/RequestManagerRetriever.java | 67 ++- 11 files changed, 751 insertions(+), 19 deletions(-) create mode 100644 instrumentation/src/androidTest/java/com/bumptech/glide/RequestManagerLifecycleTest.java create mode 100644 instrumentation/src/main/java/com/bumptech/glide/test/DefaultFragmentActivity.java create mode 100644 instrumentation/src/main/res/layout/default_fragment_activity.xml create mode 100644 library/src/main/java/com/bumptech/glide/manager/LifecycleLifecycle.java create mode 100644 library/src/main/java/com/bumptech/glide/manager/LifecycleRequestManagerRetriever.java diff --git a/instrumentation/build.gradle b/instrumentation/build.gradle index d80aa5742a..e3d8ff943d 100644 --- a/instrumentation/build.gradle +++ b/instrumentation/build.gradle @@ -6,6 +6,7 @@ tasks.whenTaskAdded { task -> apply plugin: 'com.android.application' dependencies { + debugImplementation 'androidx.fragment:fragment-testing:1.5.0' annotationProcessor project(":annotation:compiler") implementation project(":library") @@ -26,7 +27,7 @@ dependencies { } android { - compileSdk 30 as int + compileSdk COMPILE_SDK_VERSION as int defaultConfig { applicationId 'com.bumptech.glide.instrumentation' diff --git a/instrumentation/src/androidTest/java/com/bumptech/glide/RequestManagerLifecycleTest.java b/instrumentation/src/androidTest/java/com/bumptech/glide/RequestManagerLifecycleTest.java new file mode 100644 index 0000000000..f3216b0cd1 --- /dev/null +++ b/instrumentation/src/androidTest/java/com/bumptech/glide/RequestManagerLifecycleTest.java @@ -0,0 +1,489 @@ +package com.bumptech.glide; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import android.os.Bundle; +import android.view.LayoutInflater; +import android.view.View; +import android.view.ViewGroup; +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import androidx.fragment.app.Fragment; +import androidx.fragment.app.FragmentActivity; +import androidx.fragment.app.FragmentManager; +import androidx.fragment.app.testing.FragmentScenario; +import androidx.lifecycle.Lifecycle.State; +import androidx.test.core.app.ActivityScenario; +import androidx.test.core.app.ActivityScenario.ActivityAction; +import androidx.test.core.app.ApplicationProvider; +import androidx.test.ext.junit.rules.ActivityScenarioRule; +import com.bumptech.glide.instrumentation.R; +import com.bumptech.glide.test.DefaultFragmentActivity; +import com.bumptech.glide.testutil.TearDownGlide; +import com.google.common.collect.ImmutableList; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; + +// This test avoids using FragmentScenario because it doesn't seem to let us to get into the common +// created but not yet started state, only either before onCreateView or after onResume. +@RunWith(Parameterized.class) +public class RequestManagerLifecycleTest { + private static final String FRAGMENT_TAG = "fragment"; + private static final String FRAGMENT_SIBLING_TAG = "fragment_sibling"; + private static final String CHILD_FRAGMENT_TAG = "child"; + @Rule public final TearDownGlide tearDownGlide = new TearDownGlide(); + @Rule public final ActivityScenarioRule scenarioRule = + new ActivityScenarioRule<>(DefaultFragmentActivity.class); + private ActivityScenario scenario; + + @Parameter + public boolean useLifecycleInsteadOfInjectingFragments; + + @Parameters(name = "useLifecycleInsteadOfInjectingFragments = {0}") + public static ImmutableList parameters() { + return ImmutableList.of(true, false); + // return ImmutableList.of(false); + } + + @Before + public void setUp() { + Glide.init( + ApplicationProvider.getApplicationContext(), + new GlideBuilder() + .useLifecycleInsteadOfInjectingFragments(useLifecycleInsteadOfInjectingFragments)); + scenario = scenarioRule.getScenario(); + } + + @Test + public void get_twice_withSameActivity_returnsSameRequestManager() { + scenario.moveToState(State.CREATED); + scenario.onActivity( + activity -> assertThat(Glide.with(activity)).isEqualTo(Glide.with(activity))); + } + + @Test + public void get_withActivityBeforeCreate_startsRequestManager() { + scenario.moveToState(State.CREATED); + scenario.onActivity(activity -> assertThat(Glide.with(activity).isPaused()).isFalse()); + } + + @Test + public void get_withFragment_beforeFragmentIsAdded_throws() { + Fragment fragment = new Fragment(); + assertThrows(NullPointerException.class, () -> Glide.with(fragment)); + } + + @Test + public void get_withFragment_whenFragmentIsAddedAndVisible_beforeStart_startsRequestManager() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment fragment = getFragment(activity); + + assertThat(fragment.isVisible()).isTrue(); + assertThat(Glide.with(fragment).isPaused()).isFalse(); + }); + } + + @Test + public void requestManager_afterFragmentIsStopped_isPaused() { + // Avoid using FragmentScenario because it doesn't seem to let us to get into the common created + // but not yet started state, only either before onCreateView or after onResume. + final Fragment fragment = new EmptyContainerFragment() ; + scenario.moveToState(State.RESUMED); + scenario.onActivity( + activity -> { + activity.getSupportFragmentManager() + .beginTransaction() + .add(R.id.container, fragment) + .commitNowAllowingStateLoss(); + // If we call with() for the first time after the fragment is paused but while it's still + // visible, then we'll default the request manager to started. So we call with() once here + // to make sure the request manager is created before the stop event below. + Glide.with(fragment); + }); + + scenario.moveToState(State.CREATED); + scenario.onActivity( + activity -> { + assertThat(fragment.isVisible()).isTrue(); + assertThat(Glide.with(fragment).isPaused()).isTrue(); + }); + } + + @Test + public void get_twice_withSameFragment_returnsSameRequestManager() { + try (FragmentScenario fragmentScenario = + FragmentScenario.launchInContainer(EmptyContainerFragment.class)) { + fragmentScenario.onFragment( + fragment -> assertThat(Glide.with(fragment)).isEqualTo(Glide.with(fragment))); + } + } + + @Test + public void pauseRequestsRecursive_onActivity_pausesFragment() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment fragment = getFragment(activity); + + Glide.with(activity).pauseAllRequestsRecursive(); + assertThat(Glide.with(fragment).isPaused()).isTrue(); + }); + } + + @Test + public void resumeRequestsRecursive_onActivity_resumesFragment() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment fragment = getFragment(activity); + + Glide.with(activity).pauseAllRequestsRecursive(); + Glide.with(activity).resumeRequestsRecursive(); + + assertThat(Glide.with(fragment).isPaused()).isFalse(); + }); + } + + @Test + public void pauseRequestsRecursive_onActivity_pausesChildOfChildFragment() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment childFragment = getChildFragment(activity); + + Glide.with(activity).pauseAllRequestsRecursive(); + + assertThat(Glide.with(childFragment).isPaused()).isTrue(); + }); + } + + @Test + public void resumeRequestsRecursive_onActivity_resumesChildOfChildFragment() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment childFragment = getChildFragment(activity); + + Glide.with(activity).pauseAllRequestsRecursive(); + Glide.with(activity).resumeRequestsRecursive(); + + assertThat(Glide.with(childFragment).isPaused()).isFalse(); + } + ); + } + + @Test + public void pauseRequestsRecursive_onChildFragmentOfActivity_doesNotPauseActivity() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment fragment = getFragment(activity); + + Glide.with(fragment).pauseAllRequestsRecursive(); + + assertThat(Glide.with(fragment).isPaused()).isTrue(); + assertThat(Glide.with(activity).isPaused()).isFalse(); + }); + } + + @Test + public void pauseRequestsRecursive_onChildFragmentOfActivity_pausesChildOfChildFragment() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment parentFragment = getFragment(activity); + Fragment childFragment = getChildFragment(activity); + + Glide.with(parentFragment).pauseAllRequestsRecursive(); + + assertThat(Glide.with(childFragment).isPaused()).isTrue(); + }); + } + + @Test + public void resumeRequestsRecursive_onChildFragmentOfActivity_resumesChildOfChildFragment() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment parentFragment = getFragment(activity); + Fragment childFragment = getChildFragment(activity); + + Glide.with(parentFragment).pauseAllRequestsRecursive(); + Glide.with(parentFragment).resumeRequestsRecursive(); + + assertThat(Glide.with(childFragment).isPaused()).isFalse(); + }); + } + + @Test + public void pauseRequests_onActivity_pausesRequestManager() { + scenario.moveToState(State.RESUMED); + scenario.onActivity( + activity -> { + Glide.with(activity).pauseAllRequests(); + assertThat(Glide.with(activity).isPaused()).isTrue(); + }); + } + + @Test + public void resumeRequests_onActivity_pausesRequestManager() { + scenario.moveToState(State.RESUMED); + scenario.onActivity( + activity -> { + Glide.with(activity).pauseAllRequests(); + Glide.with(activity).resumeRequests(); + assertThat(Glide.with(activity).isPaused()).isFalse(); + }); + } + + @Test + public void pauseRequests_onActivity_doesNotPauseChildren() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment fragment = getFragment(activity); + initRequestManagers(activity, fragment); + + Glide.with(activity).pauseAllRequests(); + assertThat(Glide.with(fragment).isPaused()).isFalse(); + }); + } + + @Test + public void resumeRequests_onActivity_doesNotResumeChildren() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment fragment = getFragment(activity); + initRequestManagers(activity, fragment); + + Glide.with(activity).pauseAllRequests(); + Glide.with(fragment).pauseAllRequests(); + Glide.with(activity).resumeRequests(); + + assertThat(Glide.with(fragment).isPaused()).isTrue(); + }); + } + + @Test + public void pauseRequests_onFragment_pausesRequestManager() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment fragment = getFragment(activity); + Glide.with(fragment).pauseAllRequests(); + assertThat(Glide.with(fragment).isPaused()).isTrue(); + }); + } + + @Test + public void resumeRequests_onFragment_resumesRequestManager() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment fragment = getFragment(activity); + Glide.with(fragment).pauseAllRequests(); + Glide.with(fragment).resumeRequests(); + assertThat(Glide.with(fragment).isPaused()).isFalse(); + }); + } + + @Test + public void pauseRequests_onChildFragment_doesNotPauseParentFragment() { + withActivityFragmentAndChildFragment( + activity -> { + Glide.with(getChildFragment(activity)).pauseAllRequests(); + + assertThat(Glide.with(getFragment(activity)).isPaused()).isFalse(); + }); + } + + @Test + public void resumeRequests_onChildFragment_doesNotResumeParentFragment() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment parentFragment = getFragment(activity); + Fragment childFragment = getChildFragment(activity); + Glide.with(childFragment).pauseAllRequests(); + Glide.with(parentFragment).pauseAllRequests(); + Glide.with(childFragment).resumeRequests(); + + assertThat(Glide.with(parentFragment).isPaused()).isTrue(); + }); + } + + @Test + public void pauseRequests_onChildFragment_pausesChildFragment() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment childFragment = getChildFragment(activity); + Glide.with(childFragment).pauseAllRequests(); + + assertThat(Glide.with(childFragment).isPaused()).isTrue(); + }); + } + + @Test + public void resumeRequests_onChildFragment_resumesChildFragment() { + withActivityFragmentAndChildFragment( + activity -> { + Fragment childFragment = getChildFragment(activity); + Glide.with(childFragment).pauseAllRequests(); + Glide.with(childFragment).resumeRequests(); + + assertThat(Glide.with(childFragment).isPaused()).isFalse(); + }); + } + + @Test + public void pauseRequestsRecursive_onActivity_withTwoSiblingFragments_pausesBothSiblings() { + withActivityAndTwoFragmentSiblings( + activity -> { + Fragment fragment = getFragment(activity); + Fragment sibling = getSiblingFragment(activity); + + Glide.with(activity).pauseAllRequestsRecursive(); + + assertThat(Glide.with(fragment).isPaused()).isTrue(); + assertThat(Glide.with(sibling).isPaused()).isTrue(); + }); + } + + @Test + public void resumeRequestsRecursive_onActivity_withTwoSiblingFragments_resumesBothSiblings() { + withActivityAndTwoFragmentSiblings( + activity -> { + Fragment fragment = getFragment(activity); + Fragment sibling = getSiblingFragment(activity); + + Glide.with(activity).pauseAllRequestsRecursive(); + Glide.with(activity).resumeRequestsRecursive(); + + assertThat(Glide.with(fragment).isPaused()).isFalse(); + assertThat(Glide.with(sibling).isPaused()).isFalse(); + }); + } + + @Test + public void pauseRequestsRecursive_onFragment_withSibling_doesNotPauseSibling() { + withActivityAndTwoFragmentSiblings( + activity -> { + Fragment fragment = getFragment(activity); + Fragment sibling = getSiblingFragment(activity); + + Glide.with(fragment).pauseAllRequestsRecursive(); + + assertThat(Glide.with(sibling).isPaused()).isFalse(); + }); + } + + @Test + public void resumeRequestsRecursive_onFragment_withSibling_doesNotResumeSibling() { + withActivityAndTwoFragmentSiblings( + activity -> { + Fragment fragment = getFragment(activity); + Fragment sibling = getSiblingFragment(activity); + + Glide.with(fragment).pauseAllRequestsRecursive(); + Glide.with(sibling).pauseAllRequests(); + Glide.with(fragment).resumeRequestsRecursive(); + + assertThat(Glide.with(sibling).isPaused()).isTrue(); + }); + } + + // We need to create the RequestManager first, or else it will start in the paused state. + // TODO(judds): If the parent is explicitly paused, any children added after it's paused should + // probably default to paused when it's created? + private void initRequestManagers(FragmentActivity activity, Fragment... fragments) { + Glide.with(activity); + for (Fragment fragment : fragments) { + Glide.with(fragment); + } + } + + /** + * Creates the tree: Activity - Fragment + * - Fragment + */ + private void withActivityAndTwoFragmentSiblings( + ActivityAction assertion) { + setupAndRunActivityAction( + activity -> { + Fragment parentFragment = createAndAddFragment(activity, FRAGMENT_TAG); + Fragment siblingFragment = createAndAddFragment(activity, FRAGMENT_SIBLING_TAG); + initRequestManagers(activity, parentFragment, siblingFragment); + }, + assertion); + } + + /** + * Creates the tree: Activity - Fragment - Child Fragment + */ + private void withActivityFragmentAndChildFragment( + ActivityAction assertion) { + setupAndRunActivityAction( + activity -> { + Fragment parentFragment = createAndAddFragment(activity, FRAGMENT_TAG); + Fragment childFragment = createAndAddFragment(parentFragment, CHILD_FRAGMENT_TAG); + initRequestManagers(activity, parentFragment, childFragment); + }, + assertion); + } + + private void setupAndRunActivityAction( + ActivityAction setup, + ActivityAction assertion) { + scenario.moveToState(State.RESUMED); + // Using one onActivity call to do the test setup and another to assert gives the framework + // and Glide's fragment management code (onAttach in particular) the opportunity to run before our + // assertions take place. + scenario.onActivity(setup); + scenario.onActivity(assertion); + } + + private Fragment getFragment(FragmentActivity activity) { + return getFragment(activity, FRAGMENT_TAG); + } + + private Fragment getSiblingFragment(FragmentActivity activity) { + return getFragment(activity, FRAGMENT_SIBLING_TAG); + } + + private Fragment getChildFragment(FragmentActivity activity) { + return getFragment(getFragment(activity).getChildFragmentManager(), CHILD_FRAGMENT_TAG); + } + + private Fragment getFragment(FragmentActivity activity, String tag) { + return getFragment(activity.getSupportFragmentManager(), tag); + + } + + private Fragment getFragment(FragmentManager manager, String tag) { + return manager.findFragmentByTag(tag); + } + + private Fragment createAndAddFragment(FragmentActivity parent, String tag) { + return createAndAddFragment(parent.getSupportFragmentManager(), tag); + } + + private Fragment createAndAddFragment(Fragment fragment, String tag) { + return createAndAddFragment(fragment.getChildFragmentManager(), tag); + } + + private Fragment createAndAddFragment(FragmentManager manager, String tag) { + Fragment result = new EmptyContainerFragment(); + manager + .beginTransaction() + .add(R.id.container, result, tag) + .commitNowAllowingStateLoss(); + return result; + } + + public static final class EmptyContainerFragment extends Fragment { + @Override + public View onCreateView( + @NonNull LayoutInflater inflater, + @Nullable ViewGroup container, + @Nullable Bundle savedInstanceState) { + return inflater.inflate( + R.layout.default_fragment_activity, container, /* attachToRoot= */ false); + } + } +} diff --git a/instrumentation/src/main/AndroidManifest.xml b/instrumentation/src/main/AndroidManifest.xml index 074c59265c..67bf95db0f 100644 --- a/instrumentation/src/main/AndroidManifest.xml +++ b/instrumentation/src/main/AndroidManifest.xml @@ -6,10 +6,12 @@ + android:exported="false" /> + android:exported="false" /> + diff --git a/instrumentation/src/main/java/com/bumptech/glide/test/DefaultFragmentActivity.java b/instrumentation/src/main/java/com/bumptech/glide/test/DefaultFragmentActivity.java new file mode 100644 index 0000000000..8b63a69f2b --- /dev/null +++ b/instrumentation/src/main/java/com/bumptech/glide/test/DefaultFragmentActivity.java @@ -0,0 +1,15 @@ +package com.bumptech.glide.test; + +import android.os.Bundle; +import androidx.annotation.Nullable; +import androidx.fragment.app.FragmentActivity; +import com.bumptech.glide.instrumentation.R; + +public class DefaultFragmentActivity extends FragmentActivity { + + @Override + protected void onCreate(@Nullable Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + setContentView(R.layout.default_fragment_activity); + } +} diff --git a/instrumentation/src/main/res/layout/default_fragment_activity.xml b/instrumentation/src/main/res/layout/default_fragment_activity.xml new file mode 100644 index 0000000000..54b71a1b23 --- /dev/null +++ b/instrumentation/src/main/res/layout/default_fragment_activity.xml @@ -0,0 +1,5 @@ + + \ No newline at end of file diff --git a/library/src/main/java/com/bumptech/glide/Glide.java b/library/src/main/java/com/bumptech/glide/Glide.java index 736649e9d7..0efccd88d3 100644 --- a/library/src/main/java/com/bumptech/glide/Glide.java +++ b/library/src/main/java/com/bumptech/glide/Glide.java @@ -543,8 +543,11 @@ public static RequestManager with(@NonNull Context context) { * * @param activity The activity to use. * @return A RequestManager for the given activity that can be used to start a load. + * + * @deprecated TODO(judds): Figure out the end state and list it here. */ @NonNull + @Deprecated public static RequestManager with(@NonNull Activity activity) { return getRetriever(activity).get(activity); } diff --git a/library/src/main/java/com/bumptech/glide/GlideBuilder.java b/library/src/main/java/com/bumptech/glide/GlideBuilder.java index db18271180..7f01cb928e 100644 --- a/library/src/main/java/com/bumptech/glide/GlideBuilder.java +++ b/library/src/main/java/com/bumptech/glide/GlideBuilder.java @@ -487,6 +487,20 @@ public GlideBuilder setImageDecoderEnabledForBitmaps(boolean isEnabled) { return this; } + /** + * When given androidx Fragments and Activities, use {@link androidx.lifecycle.Lifecycle} to track + * the Activity or Fragment lifecycle instead of adding custom {@link + * com.bumptech.glide.manager.SupportRequestManagerFragment}s. + * + *

This flag is experimental and will be removed without notice in a future version. + */ + public GlideBuilder useLifecycleInsteadOfInjectingFragments(boolean isEnabled) { + glideExperimentsBuilder.update( + new UseLifecycleInsteadOfInjectingFragments(), + isEnabled); + return this; + } + void setRequestManagerFactory(@Nullable RequestManagerFactory factory) { this.requestManagerFactory = factory; } @@ -602,4 +616,10 @@ static final class EnableImageDecoderForBitmaps implements Experiment {} public static final class LogRequestOrigins implements Experiment {} static final class EnableLazyGlideRegistry implements Experiment {} + + /** + * Use the androidx lifecycle instead of injecting custom fragments when using androidx fragments + * and activities. + */ + public static final class UseLifecycleInsteadOfInjectingFragments implements Experiment {} } diff --git a/library/src/main/java/com/bumptech/glide/manager/LifecycleLifecycle.java b/library/src/main/java/com/bumptech/glide/manager/LifecycleLifecycle.java new file mode 100644 index 0000000000..da015fa1a7 --- /dev/null +++ b/library/src/main/java/com/bumptech/glide/manager/LifecycleLifecycle.java @@ -0,0 +1,63 @@ +package com.bumptech.glide.manager; + +import androidx.annotation.NonNull; +import androidx.lifecycle.Lifecycle.Event; +import androidx.lifecycle.Lifecycle.State; +import androidx.lifecycle.LifecycleObserver; +import androidx.lifecycle.LifecycleOwner; +import androidx.lifecycle.OnLifecycleEvent; +import com.bumptech.glide.util.Util; +import java.util.HashSet; +import java.util.Set; + +final class LifecycleLifecycle implements Lifecycle, LifecycleObserver { + @NonNull + private final Set lifecycleListeners = new HashSet(); + @NonNull + private final androidx.lifecycle.Lifecycle lifecycle; + + LifecycleLifecycle(androidx.lifecycle.Lifecycle lifecycle) { + this.lifecycle = lifecycle; + lifecycle.addObserver(this); + } + + @OnLifecycleEvent(Event.ON_START) + public void onStart(@NonNull LifecycleOwner owner) { + for (LifecycleListener lifecycleListener : Util.getSnapshot(lifecycleListeners)) { + lifecycleListener.onStart(); + } + } + + @OnLifecycleEvent(Event.ON_STOP) + public void onStop(@NonNull LifecycleOwner owner) { + for (LifecycleListener lifecycleListener : Util.getSnapshot(lifecycleListeners)) { + lifecycleListener.onStop(); + } + } + + @OnLifecycleEvent(Event.ON_DESTROY) + public void onDestroy(@NonNull LifecycleOwner owner) { + for (LifecycleListener lifecycleListener : Util.getSnapshot(lifecycleListeners)) { + lifecycleListener.onDestroy(); + } + owner.getLifecycle().removeObserver(this); + } + + @Override + public void addListener(@NonNull LifecycleListener listener) { + lifecycleListeners.add(listener); + + if (lifecycle.getCurrentState() == State.DESTROYED) { + listener.onDestroy(); + } else if (lifecycle.getCurrentState().isAtLeast(State.STARTED)) { + listener.onStart(); + } else { + listener.onStop(); + } + } + + @Override + public void removeListener(@NonNull LifecycleListener listener) { + lifecycleListeners.remove(listener); + } +} diff --git a/library/src/main/java/com/bumptech/glide/manager/LifecycleRequestManagerRetriever.java b/library/src/main/java/com/bumptech/glide/manager/LifecycleRequestManagerRetriever.java new file mode 100644 index 0000000000..023955cb43 --- /dev/null +++ b/library/src/main/java/com/bumptech/glide/manager/LifecycleRequestManagerRetriever.java @@ -0,0 +1,96 @@ +package com.bumptech.glide.manager; + +import android.content.Context; +import androidx.annotation.NonNull; +import androidx.fragment.app.Fragment; +import androidx.fragment.app.FragmentManager; +import androidx.lifecycle.Lifecycle; +import com.bumptech.glide.Glide; +import com.bumptech.glide.RequestManager; +import com.bumptech.glide.manager.RequestManagerRetriever.RequestManagerFactory; +import com.bumptech.glide.util.Synthetic; +import com.bumptech.glide.util.Util; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +final class LifecycleRequestManagerRetriever { + @Synthetic + final Map lifecycleToRequestManager = new HashMap<>(); + @NonNull + private final RequestManagerFactory factory; + + LifecycleRequestManagerRetriever(@NonNull RequestManagerFactory factory) { + this.factory = factory; + } + + RequestManager getOnly(Lifecycle lifecycle) { + Util.assertMainThread(); + return lifecycleToRequestManager.get(lifecycle); + } + + RequestManager getOrCreate( + Context context, + Glide glide, + final Lifecycle lifecycle, + FragmentManager childFragmentManager, + boolean isParentVisible) { + Util.assertMainThread(); + RequestManager result = getOnly(lifecycle); + if (result == null) { + LifecycleLifecycle glideLifecycle = new LifecycleLifecycle(lifecycle); + result = + factory.build( + glide, + glideLifecycle, + new SupportRequestManagerTreeNode(childFragmentManager), + context); + lifecycleToRequestManager.put(lifecycle, result); + glideLifecycle.addListener(new LifecycleListener() { + @Override public void onStart() {} + @Override public void onStop() {} + @Override public void onDestroy() { + lifecycleToRequestManager.remove(lifecycle); + } + }); + // This is a bit of hack, we're going to start the RequestManager, but not the + // corresponding Lifecycle. It's safe to start the RequestManager, but starting the + // Lifecycle might trigger memory leaks. See b/154405040 + if (isParentVisible) { + result.onStart(); + } + } + return result; + } + + private final class SupportRequestManagerTreeNode implements RequestManagerTreeNode { + private final FragmentManager childFragmentManager; + + SupportRequestManagerTreeNode(FragmentManager childFragmentManager) { + this.childFragmentManager = childFragmentManager; + } + + @NonNull + @Override + public Set getDescendants() { + Set result = new HashSet<>(); + getChildFragmentsRecursive(childFragmentManager, result); + return result; + } + + private void getChildFragmentsRecursive( + FragmentManager fragmentManager, Set requestManagers) { + List children = fragmentManager.getFragments(); + for (int i = 0, size = children.size(); i < size; i++) { + Fragment child = children.get(i); + getChildFragmentsRecursive(child.getChildFragmentManager(), requestManagers); + RequestManager fromChild = getOnly(child.getLifecycle()); + if (fromChild != null) { + requestManagers.add(fromChild); + } + } + } + } +} diff --git a/library/src/main/java/com/bumptech/glide/manager/RequestManagerFragment.java b/library/src/main/java/com/bumptech/glide/manager/RequestManagerFragment.java index b3d26fa189..61bbc89804 100644 --- a/library/src/main/java/com/bumptech/glide/manager/RequestManagerFragment.java +++ b/library/src/main/java/com/bumptech/glide/manager/RequestManagerFragment.java @@ -241,4 +241,5 @@ public String toString() { return super.toString() + "{fragment=" + RequestManagerFragment.this + "}"; } } + } diff --git a/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java b/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java index 11eed32f08..50c94ec2f6 100644 --- a/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java +++ b/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java @@ -23,6 +23,7 @@ import androidx.fragment.app.FragmentManager; import androidx.fragment.app.FragmentTransaction; import com.bumptech.glide.Glide; +import com.bumptech.glide.GlideBuilder; import com.bumptech.glide.GlideBuilder.WaitForFramesAfterTrimMemory; import com.bumptech.glide.GlideExperiments; import com.bumptech.glide.RequestManager; @@ -70,6 +71,7 @@ public class RequestManagerRetriever implements Handler.Callback { private final Handler handler; private final RequestManagerFactory factory; + private final GlideExperiments experiments; // Objects used to find Fragments and Activities containing views. private final ArrayMap tempViewToSupportFragment = new ArrayMap<>(); @@ -79,12 +81,14 @@ public class RequestManagerRetriever implements Handler.Callback { // Fragment/Activity extraction logic that already exists here. It's gross, but less likely to // break. private final FrameWaiter frameWaiter; + private final LifecycleRequestManagerRetriever lifecycleRequestManagerRetriever; public RequestManagerRetriever( @Nullable RequestManagerFactory factory, GlideExperiments experiments) { this.factory = factory != null ? factory : DEFAULT_FACTORY; + this.experiments = experiments; handler = new Handler(Looper.getMainLooper(), this /* Callback */); - + lifecycleRequestManagerRetriever = new LifecycleRequestManagerRetriever(this.factory); frameWaiter = buildFrameWaiter(experiments); } @@ -149,14 +153,29 @@ public RequestManager get(@NonNull Context context) { public RequestManager get(@NonNull FragmentActivity activity) { if (Util.isOnBackgroundThread()) { return get(activity.getApplicationContext()); + } + assertNotDestroyed(activity); + frameWaiter.registerSelf(activity); + FragmentManager fm = activity.getSupportFragmentManager(); + boolean isActivityVisible = isActivityVisible(activity); + if (useLifecycleInsteadOfInjectingFragments()) { + Context context = activity.getApplicationContext(); + Glide glide = Glide.get(context); + return lifecycleRequestManagerRetriever.getOrCreate( + context, + glide, + activity.getLifecycle(), + activity.getSupportFragmentManager(), + isActivityVisible); } else { - assertNotDestroyed(activity); - frameWaiter.registerSelf(activity); - FragmentManager fm = activity.getSupportFragmentManager(); - return supportFragmentGet(activity, fm, /*parentHint=*/ null, isActivityVisible(activity)); + return supportFragmentGet(activity, fm, /*parentHint=*/ null, isActivityVisible); } } + private boolean useLifecycleInsteadOfInjectingFragments() { + return experiments.isEnabled(GlideBuilder.UseLifecycleInsteadOfInjectingFragments.class); + } + @NonNull public RequestManager get(@NonNull Fragment fragment) { Preconditions.checkNotNull( @@ -164,19 +183,30 @@ public RequestManager get(@NonNull Fragment fragment) { "You cannot start a load on a fragment before it is attached or after it is destroyed"); if (Util.isOnBackgroundThread()) { return get(fragment.getContext().getApplicationContext()); + } + // In some unusual cases, it's possible to have a Fragment not hosted by an activity. There's + // not all that much we can do here. Most apps will be started with a standard activity. If + // we manage not to register the first frame waiter for a while, the consequences are not + // catastrophic, we'll just use some extra memory. + if (fragment.getActivity() != null) { + frameWaiter.registerSelf(fragment.getActivity()); + } + FragmentManager fm = fragment.getChildFragmentManager(); + Context context = fragment.getContext(); + if (useLifecycleInsteadOfInjectingFragments()) { + Glide glide = Glide.get(context.getApplicationContext()); + return lifecycleRequestManagerRetriever.getOrCreate( + context, glide, fragment.getLifecycle(), fm, fragment.isVisible()); } else { - // In some unusual cases, it's possible to have a Fragment not hosted by an activity. There's - // not all that much we can do here. Most apps will be started with a standard activity. If - // we manage not to register the first frame waiter for a while, the consequences are not - // catastrophic, we'll just use some extra memory. - if (fragment.getActivity() != null) { - frameWaiter.registerSelf(fragment.getActivity()); - } - FragmentManager fm = fragment.getChildFragmentManager(); - return supportFragmentGet(fragment.getContext(), fm, fragment, fragment.isVisible()); + return supportFragmentGet(context, fm, fragment, fragment.isVisible()); } } + /** + * @deprecated Use androidx Activities instead (ie {@link FragmentActivity}, or + * {@link androidx.appcompat.app.AppCompatActivity}). + */ + @Deprecated @SuppressWarnings("deprecation") @NonNull public RequestManager get(@NonNull Activity activity) { @@ -354,6 +384,9 @@ private static void assertNotDestroyed(@NonNull Activity activity) { } } + /** + * @deprecated Use androidx fragments instead: {@link Fragment}. + */ @SuppressWarnings("deprecation") @Deprecated @NonNull @@ -378,6 +411,10 @@ public RequestManager get(@NonNull android.app.Fragment fragment) { } } + /** + * @deprecated Use androidx activities like {@link FragmentActivity} or + * {@link androidx.appcompat.app.AppCompatActivity} instead. + */ @SuppressWarnings("deprecation") @Deprecated @NonNull @@ -407,7 +444,7 @@ private RequestManagerFragment getRequestManagerFragment( return current; } - @SuppressWarnings({"deprecation", "DeprecatedIsStillUsed"}) + @SuppressWarnings("deprecation") @Deprecated @NonNull private RequestManager fragmentGet(