From 7d1898e92600a6a2698d72e12ec26c6715fb8fc5 Mon Sep 17 00:00:00 2001 From: judds Date: Tue, 13 Feb 2018 14:42:37 -0800 Subject: [PATCH] Set RequestManagers to started if their parent Fragments/Activitys are started. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=185590877 --- .../glide/manager/RequestManagerFragment.java | 4 +- .../manager/RequestManagerRetriever.java | 60 +++++++++++++++---- .../glide/manager/RequestTracker.java | 5 ++ .../SupportRequestManagerFragment.java | 4 +- .../glide/manager/RequestTrackerTest.java | 6 +- 5 files changed, 60 insertions(+), 19 deletions(-) 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 259cb7a760..14215377ab 100644 --- a/library/src/main/java/com/bumptech/glide/manager/RequestManagerFragment.java +++ b/library/src/main/java/com/bumptech/glide/manager/RequestManagerFragment.java @@ -161,8 +161,8 @@ private boolean isDescendant(@NonNull Fragment fragment) { @SuppressWarnings("deprecation") private void registerFragmentWithRoot(@NonNull Activity activity) { unregisterFragmentWithRoot(); - rootRequestManagerFragment = Glide.get(activity).getRequestManagerRetriever() - .getRequestManagerFragment(activity.getFragmentManager(), null); + rootRequestManagerFragment = + Glide.get(activity).getRequestManagerRetriever().getRequestManagerFragment(activity); if (!equals(rootRequestManagerFragment)) { rootRequestManagerFragment.addChildRequestManagerFragment(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 c6ddb6839a..a8260fa81c 100644 --- a/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java +++ b/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java @@ -131,7 +131,8 @@ public RequestManager get(@NonNull FragmentActivity activity) { } else { assertNotDestroyed(activity); FragmentManager fm = activity.getSupportFragmentManager(); - return supportFragmentGet(activity, fm, null /*parentHint*/); + return supportFragmentGet( + activity, fm, /*parentHint=*/ null, isActivityVisible(activity)); } } @@ -143,7 +144,7 @@ public RequestManager get(@NonNull Fragment fragment) { return get(fragment.getActivity().getApplicationContext()); } else { FragmentManager fm = fragment.getChildFragmentManager(); - return supportFragmentGet(fragment.getActivity(), fm, fragment); + return supportFragmentGet(fragment.getActivity(), fm, fragment, fragment.isVisible()); } } @@ -155,7 +156,8 @@ public RequestManager get(@NonNull Activity activity) { } else { assertNotDestroyed(activity); android.app.FragmentManager fm = activity.getFragmentManager(); - return fragmentGet(activity, fm, null /*parentHint*/); + return fragmentGet( + activity, fm, /*parentHint=*/ null, isActivityVisible(activity)); } } @@ -335,21 +337,33 @@ public RequestManager get(@NonNull android.app.Fragment fragment) { return get(fragment.getActivity().getApplicationContext()); } else { android.app.FragmentManager fm = fragment.getChildFragmentManager(); - return fragmentGet(fragment.getActivity(), fm, fragment); + return fragmentGet(fragment.getActivity(), fm, fragment, fragment.isVisible()); } } @SuppressWarnings("deprecation") @Deprecated @NonNull - RequestManagerFragment getRequestManagerFragment( - @NonNull final android.app.FragmentManager fm, @Nullable android.app.Fragment parentHint) { + RequestManagerFragment getRequestManagerFragment(Activity activity) { + return getRequestManagerFragment( + activity.getFragmentManager(), /*parentHint=*/ null, isActivityVisible(activity)); + } + + @SuppressWarnings("deprecation") + @NonNull + private RequestManagerFragment getRequestManagerFragment( + @NonNull final android.app.FragmentManager fm, + @Nullable android.app.Fragment parentHint, + boolean isParentVisible) { RequestManagerFragment current = (RequestManagerFragment) fm.findFragmentByTag(FRAGMENT_TAG); if (current == null) { current = pendingRequestManagerFragments.get(fm); if (current == null) { current = new RequestManagerFragment(); current.setParentFragmentHint(parentHint); + if (isParentVisible) { + current.getGlideLifecycle().onStart(); + } pendingRequestManagerFragments.put(fm, current); fm.beginTransaction().add(current, FRAGMENT_TAG).commitAllowingStateLoss(); handler.obtainMessage(ID_REMOVE_FRAGMENT_MANAGER, fm).sendToTarget(); @@ -363,8 +377,9 @@ RequestManagerFragment getRequestManagerFragment( @NonNull private RequestManager fragmentGet(@NonNull Context context, @NonNull android.app.FragmentManager fm, - @Nullable android.app.Fragment parentHint) { - RequestManagerFragment current = getRequestManagerFragment(fm, parentHint); + @Nullable android.app.Fragment parentHint, + boolean isParentVisible) { + RequestManagerFragment current = getRequestManagerFragment(fm, parentHint, isParentVisible); RequestManager requestManager = current.getRequestManager(); if (requestManager == null) { // TODO(b/27524013): Factor out this Glide.get() call. @@ -378,8 +393,20 @@ private RequestManager fragmentGet(@NonNull Context context, } @NonNull - SupportRequestManagerFragment getSupportRequestManagerFragment( - @NonNull final FragmentManager fm, @Nullable Fragment parentHint) { + SupportRequestManagerFragment getSupportRequestManagerFragment(FragmentActivity activity) { + return getSupportRequestManagerFragment( + activity.getSupportFragmentManager(), /*parentHint=*/ null, isActivityVisible(activity)); + } + + private static boolean isActivityVisible(Activity activity) { + // This is a poor heuristic, but it's about all we have. We'd rather err on the side of visible + // and start requests than on the side of invisible and ignore valid requests. + return !activity.isFinishing(); + } + + @NonNull + private SupportRequestManagerFragment getSupportRequestManagerFragment( + @NonNull final FragmentManager fm, @Nullable Fragment parentHint, boolean isParentVisible) { SupportRequestManagerFragment current = (SupportRequestManagerFragment) fm.findFragmentByTag(FRAGMENT_TAG); if (current == null) { @@ -387,6 +414,9 @@ SupportRequestManagerFragment getSupportRequestManagerFragment( if (current == null) { current = new SupportRequestManagerFragment(); current.setParentFragmentHint(parentHint); + if (isParentVisible) { + current.getGlideLifecycle().onStart(); + } pendingSupportRequestManagerFragments.put(fm, current); fm.beginTransaction().add(current, FRAGMENT_TAG).commitAllowingStateLoss(); handler.obtainMessage(ID_REMOVE_SUPPORT_FRAGMENT_MANAGER, fm).sendToTarget(); @@ -396,9 +426,13 @@ SupportRequestManagerFragment getSupportRequestManagerFragment( } @NonNull - private RequestManager supportFragmentGet(@NonNull Context context, @NonNull FragmentManager fm, - @Nullable Fragment parentHint) { - SupportRequestManagerFragment current = getSupportRequestManagerFragment(fm, parentHint); + private RequestManager supportFragmentGet( + @NonNull Context context, + @NonNull FragmentManager fm, + @Nullable Fragment parentHint, + boolean isParentVisible) { + SupportRequestManagerFragment current = + getSupportRequestManagerFragment(fm, parentHint, isParentVisible); RequestManager requestManager = current.getRequestManager(); if (requestManager == null) { // TODO(b/27524013): Factor out this Glide.get() call. diff --git a/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java b/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java index 45fa25c175..d654f2a1c4 100644 --- a/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java +++ b/library/src/main/java/com/bumptech/glide/manager/RequestTracker.java @@ -3,6 +3,7 @@ import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.annotation.VisibleForTesting; +import android.util.Log; import com.bumptech.glide.request.Request; import com.bumptech.glide.util.Util; import java.util.ArrayList; @@ -17,6 +18,7 @@ *

This class is not thread safe and must be accessed on the main thread. */ public class RequestTracker { + private static final String TAG = "RequestTracker"; // Most requests will be for views and will therefore be held strongly (and safely) by the view // via the tag. However, a user can always pass in a different type of target which may end up not // being strongly referenced even though the user still would like the request to finish. Weak @@ -41,6 +43,9 @@ public void runRequest(@NonNull Request request) { if (!isPaused) { request.begin(); } else { + if (Log.isLoggable(TAG, Log.VERBOSE)) { + Log.v(TAG, "Paused, delaying request"); + } pendingRequests.add(request); } } diff --git a/library/src/main/java/com/bumptech/glide/manager/SupportRequestManagerFragment.java b/library/src/main/java/com/bumptech/glide/manager/SupportRequestManagerFragment.java index b6b2f28dc3..7c764cbbc1 100644 --- a/library/src/main/java/com/bumptech/glide/manager/SupportRequestManagerFragment.java +++ b/library/src/main/java/com/bumptech/glide/manager/SupportRequestManagerFragment.java @@ -142,8 +142,8 @@ private boolean isDescendant(@NonNull Fragment fragment) { private void registerFragmentWithRoot(@NonNull FragmentActivity activity) { unregisterFragmentWithRoot(); - rootRequestManagerFragment = Glide.get(activity).getRequestManagerRetriever() - .getSupportRequestManagerFragment(activity.getSupportFragmentManager(), null); + rootRequestManagerFragment = + Glide.get(activity).getRequestManagerRetriever().getSupportRequestManagerFragment(activity); if (!equals(rootRequestManagerFragment)) { rootRequestManagerFragment.addChildRequestManagerFragment(this); } diff --git a/library/test/src/test/java/com/bumptech/glide/manager/RequestTrackerTest.java b/library/test/src/test/java/com/bumptech/glide/manager/RequestTrackerTest.java index afff35212f..825ac1bce0 100644 --- a/library/test/src/test/java/com/bumptech/glide/manager/RequestTrackerTest.java +++ b/library/test/src/test/java/com/bumptech/glide/manager/RequestTrackerTest.java @@ -14,12 +14,14 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; import org.mockito.MockitoAnnotations; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; -@RunWith(JUnit4.class) +@RunWith(RobolectricTestRunner.class) +@Config(manifest = Config.NONE) public class RequestTrackerTest { private RequestTracker tracker;