Skip to content

Commit

Permalink
Load placeholders with the RequestManager Context.
Browse files Browse the repository at this point in the history
Previously we'd use the application Context, which would not use the
Activity theme. That would in turn mean we would not customize Drawables
for light / dark mode.

I've also added a bunch of emulator tests. It turns out these changes
aren't quite sufficient, even with #4842, to get asynchronous loading
of resources passed to load() working. I've left a pretty extensive
comment in the new test along with ignored tests about what additional
changes we'd need.

Progress towards #3751.
  • Loading branch information
sjudd committed Oct 21, 2022
1 parent e1366e5 commit eab4c37
Show file tree
Hide file tree
Showing 12 changed files with 493 additions and 19 deletions.
1 change: 1 addition & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ ANDROID_X_FRAGMENT_VERSION=1.3.6
ANDROID_X_RECYCLERVIEW_VERSION=1.2.1
ANDROID_X_TEST_CORE_VERSION=1.4.0
ANDROID_X_TEST_ESPRESSO_VERSION=3.4.0
ANDROID_X_FRAGMENT_TESTING_VERSION=1.5.0
ANDROID_X_TEST_JUNIT_VERSION=1.1.3
ANDROID_X_TEST_RULES_VERSION=1.4.0
ANDROID_X_TEST_RUNNER_VERSION=1.4.0
Expand Down
27 changes: 18 additions & 9 deletions instrumentation/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,27 @@ tasks.whenTaskAdded { task ->
apply plugin: 'com.android.application'

dependencies {
debugImplementation 'androidx.fragment:fragment-testing:1.5.0'
annotationProcessor project(":annotation:compiler")
implementation project(":library")
implementation "androidx.appcompat:appcompat:$ANDROID_X_APPCOMPAT_VERSION"

androidTestImplementation project(':library')
androidTestImplementation project(':mocks')
androidTestImplementation project(':testutil')
androidTestImplementation "org.mockito:mockito-android:${MOCKITO_ANDROID_VERSION}"
androidTestImplementation "androidx.test.ext:junit:${ANDROID_X_TEST_JUNIT_VERSION}"
androidTestImplementation "androidx.test:rules:${ANDROID_X_TEST_RULES_VERSION}"
androidTestImplementation "androidx.test:core:${ANDROID_X_TEST_CORE_VERSION}"
androidTestImplementation "com.google.truth:truth:${TRUTH_VERSION}"
androidTestImplementation "junit:junit:${JUNIT_VERSION}"
androidTestImplementation "androidx.exifinterface:exifinterface:${ANDROID_X_EXIF_INTERFACE_VERSION}"
androidTestImplementation "org.mockito:mockito-android:$MOCKITO_ANDROID_VERSION"
androidTestImplementation "androidx.test.ext:junit:$ANDROID_X_TEST_JUNIT_VERSION"
androidTestImplementation "androidx.test:rules:$ANDROID_X_TEST_RULES_VERSION"
androidTestImplementation "androidx.test:core:$ANDROID_X_TEST_CORE_VERSION"
androidTestImplementation "androidx.test.espresso.idling:idling-concurrent:$ANDROID_X_TEST_ESPRESSO_VERSION"
androidTestImplementation "androidx.test.espresso:espresso-core:$ANDROID_X_TEST_ESPRESSO_VERSION"
androidTestImplementation "androidx.fragment:fragment-testing:$ANDROID_X_FRAGMENT_TESTING_VERSION"
androidTestImplementation "com.google.truth:truth:$TRUTH_VERSION"
androidTestImplementation "junit:junit:$JUNIT_VERSION"
androidTestImplementation "androidx.exifinterface:exifinterface:$ANDROID_X_EXIF_INTERFACE_VERSION"

// Not totally clear why this is required, but it seems to be missing when tests are run on
// 4.1.2 and 4.2.0 emulators.
androidTestImplementation "com.google.code.findbugs:jsr305:${JSR_305_VERSION}"
androidTestImplementation "com.google.code.findbugs:jsr305:$JSR_305_VERSION"
}

android {
Expand All @@ -42,5 +45,11 @@ android {
sourceCompatibility JavaVersion.VERSION_11
targetCompatibility JavaVersion.VERSION_11
}

buildTypes {
debug {
isDefault = true
}
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,339 @@
package com.bumptech.glide;

import static androidx.test.espresso.Espresso.onIdle;
import static com.bumptech.glide.testutil.BitmapSubject.assertThat;

import android.content.Context;
import android.graphics.Bitmap;
import android.graphics.drawable.BitmapDrawable;
import android.graphics.drawable.Drawable;
import android.os.Bundle;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.view.ViewGroup.LayoutParams;
import android.widget.ImageView;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.fragment.app.Fragment;
import androidx.fragment.app.FragmentActivity;
import androidx.test.core.app.ActivityScenario;
import androidx.test.core.app.ApplicationProvider;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import com.bumptech.glide.instrumentation.R;
import com.bumptech.glide.load.engine.executor.IdlingGlideRule;
import com.bumptech.glide.request.target.Target;
import com.bumptech.glide.test.ForceDarkOrLightModeActivity;
import com.google.common.base.Function;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;

@RunWith(AndroidJUnit4.class)
public class DarkModeTest {
private final Context context = ApplicationProvider.getApplicationContext();
@Rule public final IdlingGlideRule idlingGlideRule =
IdlingGlideRule.newGlideRule(glideBuilder -> glideBuilder);

// TODO(judds): The way we handle data loads in the background for resoures is not Theme
// compatible. In particular, the theme gets lost when we convert the resource id to a Uri and
// we don't use the user provided theme. While ResourceBitmapDecoder and ResourceDrawableDecoder
// will use the theme, they're not called for most resource ids because those instead go through
// UriLoader, which just calls contentResolver.openStream. This isn't sufficient to use to theme.
// We could:
// 1. Avoid using contentResolver for android resource Uris and use ResourceBitmapDecoder instead.
// 2. #1 but only for non-raw resources which won't be themed
// 3. Always use Theme.getResources().openRawResource, which, despite the name, works find on
// Drawables and takes into account the theme.
// In addition we'd also need to consider just passing through the theme always, rather than only
// when it's specified by the user. Otherwise whether or not we'd obey dark mode would depend on
// the user also providing the theme from the activity. We'd want to try to make sure that doesn't
// leak the Activity.
// TODO(judds): Add tests for Fragments for load().
@Test
public void load_withDarkModeActivity_usesLightModeDrawable() {
runActivityTest(
darkModeActivity(),
R.raw.dog_light,
activity -> Glide.with(activity).load(R.drawable.dog).override(Target.SIZE_ORIGINAL));
}

@Test
public void load_withDarkModeFragment_usesLightModeDrawable() {
runFragmentTest(
darkModeActivity(),
R.raw.dog_light,
fragment -> Glide.with(fragment).load(R.drawable.dog).override(Target.SIZE_ORIGINAL));
}

@Test
public void load_withLightModeActivity_usesLightModeDrawable() {
runActivityTest(
lightModeActivity(),
R.raw.dog_light,
activity -> Glide.with(activity).load(R.drawable.dog).override(Target.SIZE_ORIGINAL));
}

@Test
public void load_withLightModeFragment_usesLightModeDrawable() {
runFragmentTest(
lightModeActivity(),
R.raw.dog_light,
fragment -> Glide.with(fragment).load(R.drawable.dog).override(Target.SIZE_ORIGINAL));
}

@Ignore("We do not asynchronously load resources correctly")
@Test
public void load_withDarkModeActivity_darkModeTheme_usesDarkModeDrawable() {
runActivityTest(
darkModeActivity(),
R.raw.dog_dark,
activity ->
Glide.with(activity)
.load(R.drawable.dog)
.override(Target.SIZE_ORIGINAL)
.theme(activity.getTheme()));
}

@Ignore("We do not asynchronously load resources correctly")
@Test
public void load_withDarkModeFragment_darkModeTheme_usesDarkModeDrawable() {
runFragmentTest(
darkModeActivity(),
R.raw.dog_dark,
fragment ->
Glide.with(fragment)
.load(R.drawable.dog)
.override(Target.SIZE_ORIGINAL)
.theme(fragment.requireActivity().getTheme()));
}

@Test
public void load_withLightModeActivity_lightModeTheme_usesLightModeDrawable() {
runActivityTest(
lightModeActivity(),
R.raw.dog_light,
activity ->
Glide.with(activity)
.load(R.drawable.dog)
.override(Target.SIZE_ORIGINAL)
.theme(activity.getTheme()));
}

@Test
public void placeholder_withDarkModeActivity_usesDarkModeDrawable() {
runActivityTest(
darkModeActivity(),
R.raw.dog_dark,
input ->
Glide.with(input)
.load((Object) null)
.placeholder(R.drawable.dog));
}

@Test
public void placeholder_withDarkModeFragment_usesDarkModeDrawable() {
runFragmentTest(
darkModeActivity(),
R.raw.dog_dark,
input ->
Glide.with(input)
.load((Object) null)
.placeholder(R.drawable.dog));
}

@Test
public void error_withDarkModeActivity_usesDarkModeDrawable() {
runActivityTest(
darkModeActivity(),
R.raw.dog_dark,
input ->
Glide.with(input)
.load((Object) null)
.error(R.drawable.dog));
}

@Test
public void error_withDarkModeFragment_usesDarkModeDrawable() {
runFragmentTest(
darkModeActivity(),
R.raw.dog_dark,
input ->
Glide.with(input)
.load((Object) null)
.error(R.drawable.dog));
}

@Test
public void fallback_withDarkModeActivity_usesDarkModeDrawable() {
runActivityTest(
darkModeActivity(),
R.raw.dog_dark,
input ->
Glide.with(input)
.load((Object) null)
.fallback(R.drawable.dog));
}

@Test
public void fallback_withDarkModeFragment_usesDarkModeDrawable() {
runFragmentTest(
darkModeActivity(),
R.raw.dog_dark,
input ->
Glide.with(input)
.load((Object) null)
.fallback(R.drawable.dog));
}


@Test
public void placeholder_withLightModeActivity_usesLightModeDrawable() {
runActivityTest(
lightModeActivity(),
R.raw.dog_light,
input ->
Glide.with(input)
.load((Object) null)
.placeholder(R.drawable.dog));
}

@Test
public void placeholder_withLightModeFragment_usesLightModeDrawable() {
runFragmentTest(
lightModeActivity(),
R.raw.dog_light,
input ->
Glide.with(input)
.load((Object) null)
.placeholder(R.drawable.dog));
}

@Test
public void placeholder_withDarkModeActivityAndTheme_usesDarkModeDrawable() {
runActivityTest(
darkModeActivity(),
R.raw.dog_dark,
input ->
Glide.with(input)
.load((Object) null)
.theme(input.getTheme())
.placeholder(R.drawable.dog));
}

@Test
public void placeholder_withLightModeActivityAndTheme_usesLightModeDrawable() {
runActivityTest(
lightModeActivity(),
R.raw.dog_light,
input ->
Glide.with(input)
.load((Object) null)
.theme(input.getTheme())
.placeholder(R.drawable.dog));
}

@Test
public void placeholder_withApplicationContext_darkTheme_usesDarkModeDrawable() {
runActivityTest(
darkModeActivity(),
R.raw.dog_dark,
input ->
Glide.with(input.getApplicationContext())
.load((Object) null)
.theme(input.getTheme())
.placeholder(R.drawable.dog));
}

@Test
public void placeholder_withApplicationContext_lightTheme_usesLightModeDrawable() {
runActivityTest(
lightModeActivity(),
R.raw.dog_light,
input ->
Glide.with(input.getApplicationContext())
.load((Object) null)
.theme(input.getTheme())
.placeholder(R.drawable.dog));
}

private ActivityScenario<FragmentActivity> darkModeActivity() {
return ActivityScenario.launch(ForceDarkOrLightModeActivity.forceDarkMode(context));
}

private ActivityScenario<FragmentActivity> lightModeActivity() {
return ActivityScenario.launch(ForceDarkOrLightModeActivity.forceLightMode(context));
}

private static void runFragmentTest(
ActivityScenario<? extends FragmentActivity> scenario,
int expectedResource,
Function<Fragment, RequestBuilder<Drawable>> requestBuilder) {
try (scenario) {
scenario.onActivity(
activity -> {
ImageViewFragment fragment = new ImageViewFragment();
activity
.getSupportFragmentManager()
.beginTransaction()
.add(R.id.container, fragment)
.commitNowAllowingStateLoss();
ViewGroup container = findContainer(activity);
ImageView imageView = (ImageView) container.getChildAt(0);

requestBuilder.apply(fragment).into(imageView);
});

assertImageViewContainerChildHasContent(scenario, expectedResource);
}
}

public static final class ImageViewFragment extends Fragment {
@Override
public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup container,
@Nullable Bundle savedInstanceState) {
return newFixedSizeImageView(getContext());
}
}

private static ImageView newFixedSizeImageView(Context context) {
ImageView imageView = new ImageView(context);
imageView.setLayoutParams(new LayoutParams(200, 200));
return imageView;
}

private static void runActivityTest(
ActivityScenario<? extends FragmentActivity> scenario,
int expectedResource,
Function<FragmentActivity, RequestBuilder<Drawable>> glideBuilder) {
try (scenario) {
scenario.onActivity(
activity -> {
ViewGroup container = findContainer(activity);
ImageView imageView = newFixedSizeImageView(activity);
container.addView(imageView);

glideBuilder.apply(activity).into(imageView);
});

assertImageViewContainerChildHasContent(scenario, expectedResource);
}
}

private static void assertImageViewContainerChildHasContent(
ActivityScenario<? extends FragmentActivity> scenario, int expectedResource) {
onIdle();
scenario.onActivity(
activity -> {
ViewGroup container = findContainer(activity);
ImageView imageView = (ImageView) container.getChildAt(0);
Bitmap bitmap = ((BitmapDrawable) imageView.getDrawable()).getBitmap();
assertThat(bitmap).sameAs(expectedResource);
});
}

private static ViewGroup findContainer(FragmentActivity activity) {
return activity.findViewById(R.id.container);
}
}
Loading

0 comments on commit eab4c37

Please sign in to comment.