From 5a8da19f32a3c4dcf582b4c2516eada9068ce33c Mon Sep 17 00:00:00 2001 From: Ashok Varma Date: Wed, 3 Apr 2024 06:15:31 -0700 Subject: [PATCH] cronet: Update Cronet to latest release + Move to Stable Cronet APIs. --- .../io/grpc/cronet/CronetChannelBuilder.java | 78 +------------------ .../io/grpc/cronet/CronetClientStream.java | 41 +--------- .../grpc/cronet/CronetChannelBuilderTest.java | 4 +- .../grpc/cronet/CronetClientStreamTest.java | 21 +++-- gradle/libs.versions.toml | 6 +- 5 files changed, 20 insertions(+), 130 deletions(-) diff --git a/cronet/src/main/java/io/grpc/cronet/CronetChannelBuilder.java b/cronet/src/main/java/io/grpc/cronet/CronetChannelBuilder.java index 93413aa22a3..2386aa8b2b1 100644 --- a/cronet/src/main/java/io/grpc/cronet/CronetChannelBuilder.java +++ b/cronet/src/main/java/io/grpc/cronet/CronetChannelBuilder.java @@ -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; @@ -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; @@ -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 { - 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( @@ -296,11 +289,6 @@ public Collection> 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; @@ -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); - } - } - } } } diff --git a/cronet/src/main/java/io/grpc/cronet/CronetClientStream.java b/cronet/src/main/java/io/grpc/cronet/CronetClientStream.java index 5bf7118d2fd..6c1a60be7b4 100644 --- a/cronet/src/main/java/io/grpc/cronet/CronetClientStream.java +++ b/cronet/src/main/java/io/grpc/cronet/CronetClientStream.java @@ -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; @@ -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; /** @@ -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 CRONET_ANNOTATION_KEY = CallOptions.Key.create("cronet-annotation"); @@ -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); } } } @@ -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. diff --git a/cronet/src/test/java/io/grpc/cronet/CronetChannelBuilderTest.java b/cronet/src/test/java/io/grpc/cronet/CronetChannelBuilderTest.java index b31b742577d..41f48bc03bb 100644 --- a/cronet/src/test/java/io/grpc/cronet/CronetChannelBuilderTest.java +++ b/cronet/src/test/java/io/grpc/cronet/CronetChannelBuilderTest.java @@ -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; @@ -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 = diff --git a/cronet/src/test/java/io/grpc/cronet/CronetClientStreamTest.java b/cronet/src/test/java/io/grpc/cronet/CronetClientStreamTest.java index cfbe27a6257..bd7d37dd62e 100644 --- a/cronet/src/test/java/io/grpc/cronet/CronetClientStreamTest.java +++ b/cronet/src/test/java/io/grpc/cronet/CronetClientStreamTest.java @@ -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; @@ -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() { @@ -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); @@ -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); @@ -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); @@ -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); diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index a7100e97f0e..e7a1f375cae 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -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"