Skip to content

Commit

Permalink
Check if Activitys are FragmentActivities in RequestManagerRetriever
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sjudd committed May 14, 2020
1 parent 1caeff4 commit 8f354dc
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 3 deletions.
2 changes: 1 addition & 1 deletion instrumentation/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ android {

defaultConfig {
applicationId 'com.bumptech.glide.instrumentation'
minSdkVersion MIN_SDK_VERSION as int
minSdkVersion 16 as int
targetSdkVersion TARGET_SDK_VERSION as int
versionCode 1
versionName '1.0'
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,32 @@
package com.bumptech.glide;

import static com.google.common.truth.Truth.assertThat;

import android.content.Context;
import android.graphics.drawable.Drawable;
import android.os.Build;
import android.os.Bundle;
import android.widget.ImageView;
import androidx.annotation.NonNull;
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.runners.AndroidJUnit4;
import com.bumptech.glide.manager.Lifecycle;
import com.bumptech.glide.manager.LifecycleListener;
import com.bumptech.glide.manager.RequestManagerFragment;
import com.bumptech.glide.manager.RequestManagerTreeNode;
import com.bumptech.glide.manager.SupportRequestManagerFragment;
import com.bumptech.glide.request.target.Target;
import com.bumptech.glide.test.ConcurrencyHelper;
import com.bumptech.glide.test.GlideWithAsDifferentSupertypesActivity;
import com.bumptech.glide.test.ResourceIds;
import com.bumptech.glide.test.ResourceIds.raw;
import com.bumptech.glide.test.TearDownGlide;
import com.google.common.collect.Iterables;
import java.util.ArrayList;
import java.util.List;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -77,7 +90,7 @@ public void run() {

/** Tests b/69361054. */
@Test
public void clear_withNonOwningRequestManager_onBackgroundTHread_doesNotThrow() {
public void clear_withNonOwningRequestManager_onBackgroundThread_doesNotThrow() {
concurrency.runOnMainThread(
new Runnable() {
@Override
Expand All @@ -96,4 +109,57 @@ public void run() {
}
});
}

@Test
public void with_asDifferentSuperTypes_doesNotAddMultipleFragments() {
ActivityScenario<GlideWithAsDifferentSupertypesActivity> scenario =
ActivityScenario.launch(GlideWithAsDifferentSupertypesActivity.class);
scenario.moveToState(State.RESUMED);
scenario.onActivity(
new ActivityAction<GlideWithAsDifferentSupertypesActivity>() {
@Override
public void perform(GlideWithAsDifferentSupertypesActivity activity) {
Iterable<SupportRequestManagerFragment> glideSupportFragments =
Iterables.filter(
activity.getSupportFragmentManager().getFragments(),
SupportRequestManagerFragment.class);
Iterable<RequestManagerFragment> normalFragments =
Iterables.filter(
getAllFragments(activity.getFragmentManager()), RequestManagerFragment.class);
assertThat(normalFragments).hasSize(0);
assertThat(glideSupportFragments).hasSize(1);
}
});
}

private List<android.app.Fragment> getAllFragments(android.app.FragmentManager fragmentManager) {
return Build.VERSION.SDK_INT >= Build.VERSION_CODES.O
? fragmentManager.getFragments()
: getAllFragmentsPreO(fragmentManager);
}

// Hacks based on the implementation of FragmentManagerImpl in the non-support libraries that
// allow us to iterate over and retrieve all active Fragments in a FragmentManager.
private static final String FRAGMENT_INDEX_KEY = "key";

private List<android.app.Fragment> getAllFragmentsPreO(
android.app.FragmentManager fragmentManager) {
Bundle tempBundle = new Bundle();
int index = 0;
List<android.app.Fragment> result = new ArrayList<>();
while (true) {
tempBundle.putInt(FRAGMENT_INDEX_KEY, index++);
android.app.Fragment fragment = null;
try {
fragment = fragmentManager.getFragment(tempBundle, FRAGMENT_INDEX_KEY);
} catch (Exception e) {
// This generates log spam from FragmentManager anyway.
}
if (fragment == null) {
break;
}
result.add(fragment);
}
return result;
}
}
4 changes: 3 additions & 1 deletion instrumentation/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,7 @@
package="com.bumptech.glide.instrumentation">
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.SYSTEM_ALERT_WINDOW" />
<application />
<application>
<activity android:name="com.bumptech.glide.test.GlideWithAsDifferentSupertypesActivity" />
</application>
</manifest>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package com.bumptech.glide.test;

import android.app.Activity;
import android.content.Context;
import android.os.Bundle;
import androidx.annotation.Nullable;
import androidx.fragment.app.FragmentActivity;
import com.bumptech.glide.Glide;

public class GlideWithAsDifferentSupertypesActivity extends FragmentActivity {

@Override
protected void onCreate(@Nullable Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Glide.with(this);
Glide.with((Context) this);
Glide.with((Activity) this);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ public RequestManager get(@NonNull Fragment fragment) {
public RequestManager get(@NonNull Activity activity) {
if (Util.isOnBackgroundThread()) {
return get(activity.getApplicationContext());
} else if (activity instanceof FragmentActivity) {
return get((FragmentActivity) activity);
} else {
assertNotDestroyed(activity);
android.app.FragmentManager fm = activity.getFragmentManager();
Expand Down

0 comments on commit 8f354dc

Please sign in to comment.