Skip to content

Commit

Permalink
cronet: Update Cronet to latest release + Move to Stable Cronet APIs.
Browse files Browse the repository at this point in the history
  • Loading branch information
@Ashok-Varma @temawi
Ashok-Varma authored and temawi committed Apr 22, 2024
1 parent 9de8e44 commit 5a8da19
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 130 deletions.
78 changes: 4 additions & 74 deletions cronet/src/main/java/io/grpc/cronet/CronetChannelBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static io.grpc.internal.GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE;

import android.util.Log;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.util.concurrent.MoreExecutors;
Expand All @@ -38,8 +37,6 @@
import io.grpc.internal.ManagedChannelImplBuilder.ClientTransportFactoryBuilder;
import io.grpc.internal.SharedResourceHolder;
import io.grpc.internal.TransportTracer;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.Collection;
Expand All @@ -49,15 +46,11 @@
import javax.annotation.Nullable;
import org.chromium.net.BidirectionalStream;
import org.chromium.net.CronetEngine;
import org.chromium.net.ExperimentalBidirectionalStream;
import org.chromium.net.ExperimentalCronetEngine;

/** Convenience class for building channels with the cronet transport. */
@ExperimentalApi("There is no plan to make this API stable, given transport API instability")
public final class CronetChannelBuilder extends ForwardingChannelBuilder2<CronetChannelBuilder> {

private static final String LOG_TAG = "CronetChannelBuilder";

/** BidirectionalStream.Builder factory used for getting the gRPC BidirectionalStream. */
public static abstract class StreamBuilderFactory {
public abstract BidirectionalStream.Builder newBidirectionalStreamBuilder(
Expand Down Expand Up @@ -296,11 +289,6 @@ public Collection<Class<? extends SocketAddress>> getSupportedSocketAddressTypes
* StreamBuilderFactory impl that applies TrafficStats tags to stream builders that are produced.
*/
private static class TaggingStreamFactory extends StreamBuilderFactory {
private static volatile boolean loadSetTrafficStatsTagAttempted;
private static volatile boolean loadSetTrafficStatsUidAttempted;
private static volatile Method setTrafficStatsTagMethod;
private static volatile Method setTrafficStatsUidMethod;

private final CronetEngine cronetEngine;
private final boolean trafficStatsTagSet;
private final int trafficStatsTag;
Expand All @@ -323,74 +311,16 @@ private static class TaggingStreamFactory extends StreamBuilderFactory {
@Override
public BidirectionalStream.Builder newBidirectionalStreamBuilder(
String url, BidirectionalStream.Callback callback, Executor executor) {
ExperimentalBidirectionalStream.Builder builder =
((ExperimentalCronetEngine) cronetEngine)
BidirectionalStream.Builder builder =
cronetEngine
.newBidirectionalStreamBuilder(url, callback, executor);
if (trafficStatsTagSet) {
setTrafficStatsTag(builder, trafficStatsTag);
builder.setTrafficStatsTag(trafficStatsTag);
}
if (trafficStatsUidSet) {
setTrafficStatsUid(builder, trafficStatsUid);
builder.setTrafficStatsUid(trafficStatsUid);
}
return builder;
}

private static void setTrafficStatsTag(ExperimentalBidirectionalStream.Builder builder,
int tag) {
if (!loadSetTrafficStatsTagAttempted) {
synchronized (TaggingStreamFactory.class) {
if (!loadSetTrafficStatsTagAttempted) {
try {
setTrafficStatsTagMethod = ExperimentalBidirectionalStream.Builder.class
.getMethod("setTrafficStatsTag", int.class);
} catch (NoSuchMethodException e) {
Log.w(LOG_TAG,
"Failed to load method ExperimentalBidirectionalStream.Builder.setTrafficStatsTag",
e);
} finally {
loadSetTrafficStatsTagAttempted = true;
}
}
}
}
if (setTrafficStatsTagMethod != null) {
try {
setTrafficStatsTagMethod.invoke(builder, tag);
} catch (InvocationTargetException e) {
throw new RuntimeException(e.getCause() == null ? e.getTargetException() : e.getCause());
} catch (IllegalAccessException e) {
Log.w(LOG_TAG, "Failed to set traffic stats tag: " + tag, e);
}
}
}

private static void setTrafficStatsUid(ExperimentalBidirectionalStream.Builder builder,
int uid) {
if (!loadSetTrafficStatsUidAttempted) {
synchronized (TaggingStreamFactory.class) {
if (!loadSetTrafficStatsUidAttempted) {
try {
setTrafficStatsUidMethod = ExperimentalBidirectionalStream.Builder.class
.getMethod("setTrafficStatsUid", int.class);
} catch (NoSuchMethodException e) {
Log.w(LOG_TAG,
"Failed to load method ExperimentalBidirectionalStream.Builder.setTrafficStatsUid",
e);
} finally {
loadSetTrafficStatsUidAttempted = true;
}
}
}
}
if (setTrafficStatsUidMethod != null) {
try {
setTrafficStatsUidMethod.invoke(builder, uid);
} catch (InvocationTargetException e) {
throw new RuntimeException(e.getCause() == null ? e.getTargetException() : e.getCause());
} catch (IllegalAccessException e) {
Log.w(LOG_TAG, "Failed to set traffic stats uid: " + uid, e);
}
}
}
}
}
41 changes: 2 additions & 39 deletions cronet/src/main/java/io/grpc/cronet/CronetClientStream.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@
import io.grpc.internal.TransportFrameUtil;
import io.grpc.internal.TransportTracer;
import io.grpc.internal.WritableBuffer;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.nio.Buffer;
import java.nio.ByteBuffer;
import java.nio.charset.Charset;
Expand All @@ -55,7 +53,6 @@
import javax.annotation.concurrent.GuardedBy;
import org.chromium.net.BidirectionalStream;
import org.chromium.net.CronetException;
import org.chromium.net.ExperimentalBidirectionalStream;
import org.chromium.net.UrlResponseInfo;

/**
Expand All @@ -66,9 +63,6 @@ class CronetClientStream extends AbstractClientStream {
private static final ByteBuffer EMPTY_BUFFER = ByteBuffer.allocateDirect(0);
private static final String LOG_TAG = "grpc-java-cronet";

private static volatile boolean loadAddRequestAnnotationAttempted;
private static volatile Method addRequestAnnotationMethod;

@Deprecated
static final CallOptions.Key<Object> CRONET_ANNOTATION_KEY =
CallOptions.Key.create("cronet-annotation");
Expand Down Expand Up @@ -194,14 +188,12 @@ public void writeHeaders(Metadata metadata, byte[] payload) {
builder.delayRequestHeadersUntilFirstFlush(true);
}
if (annotation != null || annotations != null) {
ExperimentalBidirectionalStream.Builder expBidiStreamBuilder =
(ExperimentalBidirectionalStream.Builder) builder;
if (annotation != null) {
addRequestAnnotation(expBidiStreamBuilder, annotation);
builder.addRequestAnnotation(annotation);
}
if (annotations != null) {
for (Object o : annotations) {
addRequestAnnotation(expBidiStreamBuilder, o);
builder.addRequestAnnotation(o);
}
}
}
Expand Down Expand Up @@ -367,35 +359,6 @@ private static boolean isApplicationHeader(String key) {
&& !TE_HEADER.name().equalsIgnoreCase(key);
}

private static void addRequestAnnotation(ExperimentalBidirectionalStream.Builder builder,
Object annotation) {
if (!loadAddRequestAnnotationAttempted) {
synchronized (CronetClientStream.class) {
if (!loadAddRequestAnnotationAttempted) {
try {
addRequestAnnotationMethod = ExperimentalBidirectionalStream.Builder.class
.getMethod("addRequestAnnotation", Object.class);
} catch (NoSuchMethodException e) {
Log.w(LOG_TAG,
"Failed to load method ExperimentalBidirectionalStream.Builder.addRequestAnnotation",
e);
} finally {
loadAddRequestAnnotationAttempted = true;
}
}
}
}
if (addRequestAnnotationMethod != null) {
try {
addRequestAnnotationMethod.invoke(builder, annotation);
} catch (InvocationTargetException e) {
throw new RuntimeException(e.getCause() == null ? e.getTargetException() : e.getCause());
} catch (IllegalAccessException e) {
Log.w(LOG_TAG, "Failed to add request annotation: " + annotation, e);
}
}
}

private void setGrpcHeaders(BidirectionalStream.Builder builder) {
// Psuedo-headers are set by cronet.
// All non-pseudo headers must come after pseudo headers.
Expand Down
4 changes: 2 additions & 2 deletions cronet/src/test/java/io/grpc/cronet/CronetChannelBuilderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import io.grpc.testing.TestMethodDescriptors;
import java.net.InetSocketAddress;
import java.util.concurrent.ScheduledExecutorService;
import org.chromium.net.ExperimentalCronetEngine;
import org.chromium.net.CronetEngine;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -50,7 +50,7 @@
public final class CronetChannelBuilderTest {
@Rule public final MockitoRule mocks = MockitoJUnit.rule();

@Mock private ExperimentalCronetEngine mockEngine;
@Mock private CronetEngine mockEngine;
@Mock private ChannelLogger channelLogger;

private final ClientStreamTracer[] tracers =
Expand Down
21 changes: 10 additions & 11 deletions cronet/src/test/java/io/grpc/cronet/CronetClientStreamTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import java.util.concurrent.Executor;
import org.chromium.net.BidirectionalStream;
import org.chromium.net.CronetException;
import org.chromium.net.ExperimentalBidirectionalStream;
import org.chromium.net.UrlResponseInfo;
import org.chromium.net.impl.UrlResponseInfoImpl;
import org.junit.Before;
Expand All @@ -76,9 +75,9 @@ public final class CronetClientStreamTest {
@Mock private CronetClientTransport transport;
private Metadata metadata = new Metadata();
@Mock private StreamBuilderFactory factory;
@Mock private ExperimentalBidirectionalStream cronetStream;
@Mock private BidirectionalStream cronetStream;
@Mock private ClientStreamListener clientListener;
@Mock private ExperimentalBidirectionalStream.Builder builder;
@Mock private BidirectionalStream.Builder builder;
private final Object lock = new Object();
private final TransportTracer transportTracer = TransportTracer.getDefaultFactory().create();
private final Executor executor = new Executor() {
Expand Down Expand Up @@ -681,8 +680,8 @@ public void getUnaryRequest() {
true,
false);
callback.setStream(stream);
ExperimentalBidirectionalStream.Builder getBuilder =
mock(ExperimentalBidirectionalStream.Builder.class);
BidirectionalStream.Builder getBuilder =
mock(BidirectionalStream.Builder.class);
when(getFactory.newBidirectionalStreamBuilder(
any(String.class), any(BidirectionalStream.Callback.class), any(Executor.class)))
.thenReturn(getBuilder);
Expand Down Expand Up @@ -738,8 +737,8 @@ public void idempotentMethod_usesHttpPut() {
true,
true);
callback.setStream(stream);
ExperimentalBidirectionalStream.Builder builder =
mock(ExperimentalBidirectionalStream.Builder.class);
BidirectionalStream.Builder builder =
mock(BidirectionalStream.Builder.class);
when(factory.newBidirectionalStreamBuilder(
any(String.class), any(BidirectionalStream.Callback.class), any(Executor.class)))
.thenReturn(builder);
Expand Down Expand Up @@ -770,8 +769,8 @@ public void alwaysUsePutOption_usesHttpPut() {
true,
true);
callback.setStream(stream);
ExperimentalBidirectionalStream.Builder builder =
mock(ExperimentalBidirectionalStream.Builder.class);
BidirectionalStream.Builder builder =
mock(BidirectionalStream.Builder.class);
when(factory.newBidirectionalStreamBuilder(
any(String.class), any(BidirectionalStream.Callback.class), any(Executor.class)))
.thenReturn(builder);
Expand Down Expand Up @@ -810,8 +809,8 @@ public void reservedHeadersStripped() {
false,
false);
callback.setStream(stream);
ExperimentalBidirectionalStream.Builder builder =
mock(ExperimentalBidirectionalStream.Builder.class);
BidirectionalStream.Builder builder =
mock(BidirectionalStream.Builder.class);
when(factory.newBidirectionalStreamBuilder(
any(String.class), any(BidirectionalStream.Callback.class), any(Executor.class)))
.thenReturn(builder);
Expand Down
6 changes: 2 additions & 4 deletions gradle/libs.versions.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ auto-value-annotations = "com.google.auto.value:auto-value-annotations:1.10.4"
checkstyle = "com.puppycrawl.tools:checkstyle:10.12.5"
commons-math3 = "org.apache.commons:commons-math3:3.6.1"
conscrypt = "org.conscrypt:conscrypt-openjdk-uber:2.5.2"
# Update notes / 2023-07-19 sergiitk:
# Cronet (API and Embedded) upgrade is blocked by https://github.com/grpc/grpc-java/issues/10396.
cronet-api = "org.chromium.net:cronet-api:108.5359.79"
cronet-embedded = "org.chromium.net:cronet-embedded:108.5359.79"
cronet-api = "org.chromium.net:cronet-api:119.6045.31"
cronet-embedded = "org.chromium.net:cronet-embedded:119.6045.31"
errorprone-annotations = "com.google.errorprone:error_prone_annotations:2.23.0"
errorprone-core = "com.google.errorprone:error_prone_core:2.23.0"
google-api-protos = "com.google.api.grpc:proto-google-common-protos:2.29.0"
Expand Down

0 comments on commit 5a8da19

Please sign in to comment.