Skip to content

Commit

Permalink
Open source a Cronet integration for Glide.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 289467603
  • Loading branch information
sjudd authored and glide-copybara-robot committed Jan 13, 2020
1 parent ff29849 commit 114251e
Show file tree
Hide file tree
Showing 7 changed files with 121 additions and 44 deletions.
11 changes: 8 additions & 3 deletions integration/cronet/build.gradle
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
package com.bumptech.glide.integration.cronet

apply plugin: 'com.android.library'

dependencies {
implementation project(':library')
annotationProcessor project(':annotation:compiler')
implementation 'com.google.android.gms:play-services-cronet:17.0.0'
implementation "com.google.guava:guava:${GUAVA_VERSION}"
implementation project(':annotation')

api "androidx.annotation:annotation:${ANDROID_X_VERSION}"

testImplementation "com.google.truth:truth:${TRUTH_VERSION}"
testImplementation "junit:junit:${JUNIT_VERSION}"
testImplementation "org.robolectric:robolectric:${ROBOLECTRIC_VERSION}"
testImplementation "org.mockito:mockito-core:${MOCKITO_VERSION}"
}

android {
Expand Down
2 changes: 0 additions & 2 deletions integration/cronet/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="com.bumptech.glide.integration.cronet">

<uses-sdk android:minSdkVersion="14" />
<application>
<meta-data
android:name="com.bumptech.glide.integration.cronet.CronetGlideModule"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,17 @@
* <ul>
* A new request is made to cronet if a url is requested and no cronet request for that url is
* in progress
* <ul>
* Subsequent requests for in progress urls do not make new requests to cronet, but are
* notified when the existing cronet request completes.
* <ul>
* Cancelling a single request does not cancel the cronet request if multiple requests for
* the url have been made, but cancelling all requests for a url does cancel the cronet
* request.
* </ul>
*
* <ul>
* Subsequent requests for in progress urls do not make new requests to cronet, but are notified
* when the existing cronet request completes.
* </ul>
*
* <ul>
* Cancelling a single request does not cancel the cronet request if multiple requests for the url
* have been made, but cancelling all requests for a url does cancel the cronet request.
* </ul>
*/
final class ChromiumRequestSerializer {
private static final String TAG = "ChromiumSerializer";
Expand All @@ -50,7 +54,7 @@ final class ChromiumRequestSerializer {
new EnumMap<>(Priority.class);
// Memoized so that all callers can share an instance.
// Suppliers.memoize() is thread safe. See google3/java/com/google/common/base/Suppliers.java
private static final Supplier<Executor> glideExecutorSupplier =
private static final Supplier<Executor> GLIDE_EXECUTOR_SUPPLIER =
Suppliers.memoize(
new Supplier<Executor>() {
@Override
Expand Down Expand Up @@ -229,7 +233,7 @@ public void onReadCompleted(

@Override
public void onSucceeded(UrlRequest request, final UrlResponseInfo info) {
glideExecutorSupplier
GLIDE_EXECUTOR_SUPPLIER
.get()
.execute(
new PriorityRunnable(priority) {
Expand All @@ -247,7 +251,7 @@ public void run() {
@Override
public void onFailed(
UrlRequest urlRequest, final UrlResponseInfo urlResponseInfo, final CronetException e) {
glideExecutorSupplier
GLIDE_EXECUTOR_SUPPLIER
.get()
.execute(
new PriorityRunnable(priority) {
Expand All @@ -260,7 +264,7 @@ public void run() {

@Override
public void onCanceled(UrlRequest urlRequest, @Nullable final UrlResponseInfo urlResponseInfo) {
glideExecutorSupplier
GLIDE_EXECUTOR_SUPPLIER
.get()
.execute(
new PriorityRunnable(priority) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,30 @@
import com.bumptech.glide.Registry;
import com.bumptech.glide.load.model.GlideUrl;
import com.bumptech.glide.module.GlideModule;
import com.google.android.apps.common.proguard.UsedByReflection;
import com.google.common.base.Supplier;
import java.io.InputStream;
import java.nio.ByteBuffer;
import org.chromium.net.CronetEngine;

/**
* A {@link GlideModule} that registers components allowing remote image fetching to be done using
* Cronet.
*/
@UsedByReflection("meta-data")
public final class CronetGlideModule implements GlideModule {

@Override
public void applyOptions(Context context, GlideBuilder builder) {}

@Override
public void registerComponents(Context context, Glide glide, Registry registry) {
public void registerComponents(final Context context, Glide glide, Registry registry) {
CronetRequestFactory factory =
new CronetRequestFactoryImpl(() -> CronetEngineSingleton.getSingleton(context));
new CronetRequestFactoryImpl(
new Supplier<CronetEngine>() {
@Override
public CronetEngine get() {
return CronetEngineSingleton.getSingleton(context);
}
});
registry.replace(
GlideUrl.class, InputStream.class, new ChromiumUrlLoader.StreamFactory(factory, null));
registry.prepend(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
import com.bumptech.glide.integration.cronet.ChromiumUrlLoader.StreamFactory;
import com.bumptech.glide.load.model.GlideUrl;
import com.bumptech.glide.module.LibraryGlideModule;
import com.google.common.base.Supplier;
import java.io.InputStream;
import java.nio.ByteBuffer;
import org.chromium.net.CronetEngine;

/**
* A {@link LibraryGlideModule} that registers components allowing remote image fetching to be done
Expand All @@ -21,9 +23,15 @@ public final class CronetLibraryGlideModule extends LibraryGlideModule {

@Override
public void registerComponents(
@NonNull Context context, @NonNull Glide glide, @NonNull Registry registry) {
final @NonNull Context context, @NonNull Glide glide, @NonNull Registry registry) {
CronetRequestFactory factory =
new CronetRequestFactoryImpl(() -> CronetEngineSingleton.getSingleton(context));
new CronetRequestFactoryImpl(
new Supplier<CronetEngine>() {
@Override
public CronetEngine get() {
return CronetEngineSingleton.getSingleton(context);
}
});
registry.replace(
GlideUrl.class, InputStream.class, new StreamFactory(factory, null /* dataLogger */));
registry.prepend(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,39 +23,46 @@
import com.bumptech.glide.load.model.LazyHeaders;
import com.bumptech.glide.load.model.LazyHeaders.Builder;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.util.concurrent.MoreExecutors;
import java.net.HttpURLConnection;
import java.nio.ByteBuffer;
import java.util.AbstractMap.SimpleImmutableEntry;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.concurrent.Executor;
import org.chromium.net.CronetEngine;
import org.chromium.net.CronetException;
import org.chromium.net.UrlRequest;
import org.chromium.net.UrlRequest.Callback;
import org.chromium.net.UrlResponseInfo;
import org.chromium.net.impl.UrlResponseInfoImpl;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.ArgumentCaptor;
import org.mockito.ArgumentMatchers;
import org.mockito.Matchers;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
import org.mockito.stubbing.Answer;
import org.robolectric.RobolectricTestRunner;

/** Tests for {@link ChromiumUrlFetcher}. */
@RunWith(RobolectricTestRunner.class)
public class ChromiumUrlFetcherTest {

@Rule public final MockitoRule mocks = MockitoJUnit.rule();
@Mock private DataCallback<ByteBuffer> callback;
@Mock private CronetEngine cronetEngine;
@Mock private UrlRequest request;
@Mock private UrlRequest.Builder mockUrlRequestBuilder;
@Mock private ByteBufferParser<ByteBuffer> parser;
@Mock private CronetRequestFactory cronetRequestFactory;
@Mock private DataCallback<ByteBuffer> firstCallback;
@Mock private DataCallback<ByteBuffer> secondCallback;

private UrlRequest.Builder builder;
private GlideUrl glideUrl;
Expand All @@ -65,7 +72,6 @@ public class ChromiumUrlFetcherTest {

@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
when(parser.getDataClass()).thenReturn(ByteBuffer.class);
when(parser.parse(any(ByteBuffer.class)))
.thenAnswer(
Expand Down Expand Up @@ -144,7 +150,7 @@ public void testLoadData_withInProgressRequest_doesNotStartNewRequest() {
.newRequest(
Matchers.eq(glideUrl.toStringUrl()),
anyInt(),
anyMap(),
ArgumentMatchers.<String, String>anyMap(),
any(UrlRequest.Callback.class));
}

Expand All @@ -155,28 +161,74 @@ public void testLoadData_withInProgressRequest_isNotifiedWhenRequestCompletes()
ChromiumUrlFetcher<ByteBuffer> secondFetcher =
new ChromiumUrlFetcher<>(serializer, parser, glideUrl);

DataCallback<ByteBuffer> firstCb = mock(DataCallback.class);
DataCallback<ByteBuffer> secondCb = mock(DataCallback.class);
firstFetcher.loadData(Priority.LOW, firstCb);
secondFetcher.loadData(Priority.HIGH, secondCb);
firstFetcher.loadData(Priority.LOW, firstCallback);
secondFetcher.loadData(Priority.HIGH, secondCallback);

succeed(getInfo(10, 200), urlRequestListenerCaptor.getValue(), ByteBuffer.allocateDirect(10));

verify(firstCb, timeout(1000)).onDataReady(isA(ByteBuffer.class));
verify(secondCb, timeout(1000)).onDataReady(isA(ByteBuffer.class));
verify(firstCallback, timeout(1000)).onDataReady(isA(ByteBuffer.class));
verify(secondCallback, timeout(1000)).onDataReady(isA(ByteBuffer.class));
}

@NonNull
private UrlResponseInfo getInfo(int contentLength, int statusCode) {
return new UrlResponseInfoImpl(
ImmutableList.of(glideUrl.toStringUrl()),
statusCode,
"OK",
ImmutableList.<Entry<String, String>>of(
new SimpleImmutableEntry<>("Content-Length", Integer.toString(contentLength))),
false,
"",
"");
private UrlResponseInfo getInfo(final int contentLength, final int statusCode) {
return new UrlResponseInfo() {

@Override
public String getUrl() {
return glideUrl.toStringUrl();
}

@Override
public List<String> getUrlChain() {
return ImmutableList.of(getUrl());
}

@Override
public int getHttpStatusCode() {
return statusCode;
}

@Override
public String getHttpStatusText() {
return "OK";
}

@Override
public List<Map.Entry<String, String>> getAllHeadersAsList() {
return ImmutableList.<Map.Entry<String, String>>of(
new SimpleImmutableEntry<>("Content-Length", Integer.toString(contentLength)));
}

@Override
public Map<String, List<String>> getAllHeaders() {
ImmutableMap.Builder<String, List<String>> builder = ImmutableMap.builder();
for (Map.Entry<String, String> entry : getAllHeadersAsList()) {
builder.put(entry.getKey(), ImmutableList.copyOf(entry.getValue().split(",")));
}
return builder.build();
}

@Override
public boolean wasCached() {
return false;
}

@Override
public String getNegotiatedProtocol() {
return "";
}

@Override
public String getProxyServer() {
return "";
}

@Override
public long getReceivedByteCount() {
return 0;
}
};
}

@Test
Expand Down Expand Up @@ -226,7 +278,10 @@ public void testCancel_withStartedRequest_cancelsRequest() {

@Test
public void testRequestComplete_withNonNullException_callsCallbackWithException() {
CronetException expected = new CronetException("test", /*cause=*/ null) {};
CronetException expected =
new CronetException("test", /*cause=*/ null) {
static final long serialVersionUID = 1;
};
fetcher.loadData(Priority.LOW, callback);
urlRequestListenerCaptor.getValue().onFailed(request, null, expected);

Expand Down
1 change: 1 addition & 0 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ include ':samples:contacturi'
include ':samples:imgur'
include ':integration'
include ':integration:concurrent'
include ':integration:cronet'
include ':integration:gifencoder'
include ':integration:okhttp'
include ':integration:okhttp3'
Expand Down

0 comments on commit 114251e

Please sign in to comment.