Skip to content

Commit 8f354dc

Browse files
committedMay 14, 2020
Check if Activitys are FragmentActivities in RequestManagerRetriever
This fixes an issue where we can end up adding both a support and a non-support Fragment to a FragmentActivity if the FragmentActivity is passed to Glide as an Activity or Context instead of a FragmentContext at some point.
1 parent 1caeff4 commit 8f354dc

File tree

5 files changed

+92
-3
lines changed

5 files changed

+92
-3
lines changed
 

‎instrumentation/build.gradle

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ android {
2929

3030
defaultConfig {
3131
applicationId 'com.bumptech.glide.instrumentation'
32-
minSdkVersion MIN_SDK_VERSION as int
32+
minSdkVersion 16 as int
3333
targetSdkVersion TARGET_SDK_VERSION as int
3434
versionCode 1
3535
versionName '1.0'

‎instrumentation/src/androidTest/java/com/bumptech/glide/RequestManagerTest.java

+67-1
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,32 @@
11
package com.bumptech.glide;
22

3+
import static com.google.common.truth.Truth.assertThat;
4+
35
import android.content.Context;
46
import android.graphics.drawable.Drawable;
7+
import android.os.Build;
8+
import android.os.Bundle;
59
import android.widget.ImageView;
610
import androidx.annotation.NonNull;
11+
import androidx.lifecycle.Lifecycle.State;
12+
import androidx.test.core.app.ActivityScenario;
13+
import androidx.test.core.app.ActivityScenario.ActivityAction;
714
import androidx.test.core.app.ApplicationProvider;
815
import androidx.test.ext.junit.runners.AndroidJUnit4;
916
import com.bumptech.glide.manager.Lifecycle;
1017
import com.bumptech.glide.manager.LifecycleListener;
18+
import com.bumptech.glide.manager.RequestManagerFragment;
1119
import com.bumptech.glide.manager.RequestManagerTreeNode;
20+
import com.bumptech.glide.manager.SupportRequestManagerFragment;
1221
import com.bumptech.glide.request.target.Target;
1322
import com.bumptech.glide.test.ConcurrencyHelper;
23+
import com.bumptech.glide.test.GlideWithAsDifferentSupertypesActivity;
1424
import com.bumptech.glide.test.ResourceIds;
1525
import com.bumptech.glide.test.ResourceIds.raw;
1626
import com.bumptech.glide.test.TearDownGlide;
27+
import com.google.common.collect.Iterables;
28+
import java.util.ArrayList;
29+
import java.util.List;
1730
import org.junit.Before;
1831
import org.junit.Rule;
1932
import org.junit.Test;
@@ -77,7 +90,7 @@ public void run() {
7790

7891
/** Tests b/69361054. */
7992
@Test
80-
public void clear_withNonOwningRequestManager_onBackgroundTHread_doesNotThrow() {
93+
public void clear_withNonOwningRequestManager_onBackgroundThread_doesNotThrow() {
8194
concurrency.runOnMainThread(
8295
new Runnable() {
8396
@Override
@@ -96,4 +109,57 @@ public void run() {
96109
}
97110
});
98111
}
112+
113+
@Test
114+
public void with_asDifferentSuperTypes_doesNotAddMultipleFragments() {
115+
ActivityScenario<GlideWithAsDifferentSupertypesActivity> scenario =
116+
ActivityScenario.launch(GlideWithAsDifferentSupertypesActivity.class);
117+
scenario.moveToState(State.RESUMED);
118+
scenario.onActivity(
119+
new ActivityAction<GlideWithAsDifferentSupertypesActivity>() {
120+
@Override
121+
public void perform(GlideWithAsDifferentSupertypesActivity activity) {
122+
Iterable<SupportRequestManagerFragment> glideSupportFragments =
123+
Iterables.filter(
124+
activity.getSupportFragmentManager().getFragments(),
125+
SupportRequestManagerFragment.class);
126+
Iterable<RequestManagerFragment> normalFragments =
127+
Iterables.filter(
128+
getAllFragments(activity.getFragmentManager()), RequestManagerFragment.class);
129+
assertThat(normalFragments).hasSize(0);
130+
assertThat(glideSupportFragments).hasSize(1);
131+
}
132+
});
133+
}
134+
135+
private List<android.app.Fragment> getAllFragments(android.app.FragmentManager fragmentManager) {
136+
return Build.VERSION.SDK_INT >= Build.VERSION_CODES.O
137+
? fragmentManager.getFragments()
138+
: getAllFragmentsPreO(fragmentManager);
139+
}
140+
141+
// Hacks based on the implementation of FragmentManagerImpl in the non-support libraries that
142+
// allow us to iterate over and retrieve all active Fragments in a FragmentManager.
143+
private static final String FRAGMENT_INDEX_KEY = "key";
144+
145+
private List<android.app.Fragment> getAllFragmentsPreO(
146+
android.app.FragmentManager fragmentManager) {
147+
Bundle tempBundle = new Bundle();
148+
int index = 0;
149+
List<android.app.Fragment> result = new ArrayList<>();
150+
while (true) {
151+
tempBundle.putInt(FRAGMENT_INDEX_KEY, index++);
152+
android.app.Fragment fragment = null;
153+
try {
154+
fragment = fragmentManager.getFragment(tempBundle, FRAGMENT_INDEX_KEY);
155+
} catch (Exception e) {
156+
// This generates log spam from FragmentManager anyway.
157+
}
158+
if (fragment == null) {
159+
break;
160+
}
161+
result.add(fragment);
162+
}
163+
return result;
164+
}
99165
}

‎instrumentation/src/main/AndroidManifest.xml

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,7 @@
22
package="com.bumptech.glide.instrumentation">
33
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
44
<uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW" />
5-
<application />
5+
<application>
6+
<activity android:name="com.bumptech.glide.test.GlideWithAsDifferentSupertypesActivity" />
7+
</application>
68
</manifest>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package com.bumptech.glide.test;
2+
3+
import android.app.Activity;
4+
import android.content.Context;
5+
import android.os.Bundle;
6+
import androidx.annotation.Nullable;
7+
import androidx.fragment.app.FragmentActivity;
8+
import com.bumptech.glide.Glide;
9+
10+
public class GlideWithAsDifferentSupertypesActivity extends FragmentActivity {
11+
12+
@Override
13+
protected void onCreate(@Nullable Bundle savedInstanceState) {
14+
super.onCreate(savedInstanceState);
15+
Glide.with(this);
16+
Glide.with((Context) this);
17+
Glide.with((Activity) this);
18+
}
19+
}

‎library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java

+2
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ public RequestManager get(@NonNull Fragment fragment) {
149149
public RequestManager get(@NonNull Activity activity) {
150150
if (Util.isOnBackgroundThread()) {
151151
return get(activity.getApplicationContext());
152+
} else if (activity instanceof FragmentActivity) {
153+
return get((FragmentActivity) activity);
152154
} else {
153155
assertNotDestroyed(activity);
154156
android.app.FragmentManager fm = activity.getFragmentManager();

0 commit comments

Comments
 (0)
Please sign in to comment.