From 14a0e1acd07d76bc161de6d211f9cd7484918f61 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Mon, 1 Jun 2020 15:35:44 -0700 Subject: [PATCH] Only start RequestManagers, not lifecycles when creating with visible parents --- .../manager/RequestManagerRetriever.java | 38 +++++++++---------- .../SupportRequestManagerFragment.java | 2 +- 2 files changed, 20 insertions(+), 20 deletions(-) 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 0dfb1d7228..0a4c02f108 100644 --- a/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java +++ b/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java @@ -357,25 +357,19 @@ public RequestManager get(@NonNull android.app.Fragment fragment) { @Deprecated @NonNull RequestManagerFragment getRequestManagerFragment(Activity activity) { - return getRequestManagerFragment( - activity.getFragmentManager(), /*parentHint=*/ null, isActivityVisible(activity)); + return getRequestManagerFragment(activity.getFragmentManager(), /*parentHint=*/ null); } @SuppressWarnings("deprecation") @NonNull private RequestManagerFragment getRequestManagerFragment( - @NonNull final android.app.FragmentManager fm, - @Nullable android.app.Fragment parentHint, - boolean isParentVisible) { + @NonNull final android.app.FragmentManager fm, @Nullable android.app.Fragment parentHint) { 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(); @@ -392,7 +386,7 @@ private RequestManager fragmentGet( @NonNull android.app.FragmentManager fm, @Nullable android.app.Fragment parentHint, boolean isParentVisible) { - RequestManagerFragment current = getRequestManagerFragment(fm, parentHint, isParentVisible); + RequestManagerFragment current = getRequestManagerFragment(fm, parentHint); RequestManager requestManager = current.getRequestManager(); if (requestManager == null) { // TODO(b/27524013): Factor out this Glide.get() call. @@ -400,16 +394,20 @@ private RequestManager fragmentGet( requestManager = factory.build( glide, current.getGlideLifecycle(), current.getRequestManagerTreeNode(), context); + // 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) { + requestManager.onStart(); + } current.setRequestManager(requestManager); } return requestManager; } @NonNull - SupportRequestManagerFragment getSupportRequestManagerFragment( - Context context, FragmentManager fragmentManager) { - return getSupportRequestManagerFragment( - fragmentManager, /*parentHint=*/ null, isActivityVisible(context)); + SupportRequestManagerFragment getSupportRequestManagerFragment(FragmentManager fragmentManager) { + return getSupportRequestManagerFragment(fragmentManager, /*parentHint=*/ null); } private static boolean isActivityVisible(Context context) { @@ -421,7 +419,7 @@ private static boolean isActivityVisible(Context context) { @NonNull private SupportRequestManagerFragment getSupportRequestManagerFragment( - @NonNull final FragmentManager fm, @Nullable Fragment parentHint, boolean isParentVisible) { + @NonNull final FragmentManager fm, @Nullable Fragment parentHint) { SupportRequestManagerFragment current = (SupportRequestManagerFragment) fm.findFragmentByTag(FRAGMENT_TAG); if (current == null) { @@ -429,9 +427,6 @@ private 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(); @@ -446,8 +441,7 @@ private RequestManager supportFragmentGet( @NonNull FragmentManager fm, @Nullable Fragment parentHint, boolean isParentVisible) { - SupportRequestManagerFragment current = - getSupportRequestManagerFragment(fm, parentHint, isParentVisible); + SupportRequestManagerFragment current = getSupportRequestManagerFragment(fm, parentHint); RequestManager requestManager = current.getRequestManager(); if (requestManager == null) { // TODO(b/27524013): Factor out this Glide.get() call. @@ -455,6 +449,12 @@ private RequestManager supportFragmentGet( requestManager = factory.build( glide, current.getGlideLifecycle(), current.getRequestManagerTreeNode(), context); + // 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) { + requestManager.onStart(); + } current.setRequestManager(requestManager); } return requestManager; 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 f702355b3d..fe558d4d02 100644 --- a/library/src/main/java/com/bumptech/glide/manager/SupportRequestManagerFragment.java +++ b/library/src/main/java/com/bumptech/glide/manager/SupportRequestManagerFragment.java @@ -154,7 +154,7 @@ private void registerFragmentWithRoot( rootRequestManagerFragment = Glide.get(context) .getRequestManagerRetriever() - .getSupportRequestManagerFragment(context, fragmentManager); + .getSupportRequestManagerFragment(fragmentManager); if (!equals(rootRequestManagerFragment)) { rootRequestManagerFragment.addChildRequestManagerFragment(this); }