Commit 199523d4 authored by Yu Su's avatar Yu Su Committed by Commit Bot

Enable setting request idempotency for Android.

0-RTT for idempotent request is enabled here:
https://chromium-review.googlesource.com/c/chromium/src/+/2508299
Here an idempotency setter is added in java ExperimentalUrlRequest
builder with a default implementation to enable 0-RTT for idempotent
request on Android.

The setIdempotency API is not added in UrlRequest.Builder as an
abstract method since client has already implemented UrlRequest.

Bug: 1148031
Change-Id: I43fddfbd65a197599924c03aeeede461dcef3dc1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2535657Reviewed-by: default avatarDavid Schinazi <dschinazi@chromium.org>
Commit-Queue: Yu Su <yuyansu@google.com>
Cr-Commit-Position: refs/heads/master@{#827112}
parent baafb726
......@@ -67,6 +67,10 @@ java_cpp_enum("net_request_priority_java") {
sources = [ "//net/base/request_priority.h" ]
}
java_cpp_enum("net_idempotency_java") {
sources = [ "//net/base/idempotency.h" ]
}
java_cpp_enum("network_quality_observation_source_java") {
sources = [ "//net/nqe/network_quality_observation_source.h" ]
}
......@@ -346,6 +350,7 @@ android_library("cronet_impl_fake_base_java") {
}
cronet_impl_native_java_srcjar_deps = [
":net_idempotency_java",
":net_request_priority_java",
":network_quality_observation_source_java",
":url_request_error_java",
......
......@@ -171,12 +171,16 @@ public abstract class org.chromium.net.ExperimentalCronetEngine extends org.chro
public org.chromium.net.UrlRequest$Builder newUrlRequestBuilder(java.lang.String, org.chromium.net.UrlRequest$Callback, java.util.concurrent.Executor);
}
public abstract class org.chromium.net.ExperimentalUrlRequest$Builder extends org.chromium.net.UrlRequest$Builder {
public static final int DEFAULT_IDEMPOTENCY;
public static final int IDEMPOTENT;
public static final int NOT_IDEMPOTENT;
public org.chromium.net.ExperimentalUrlRequest$Builder();
public org.chromium.net.ExperimentalUrlRequest$Builder disableConnectionMigration();
public org.chromium.net.ExperimentalUrlRequest$Builder addRequestAnnotation(java.lang.Object);
public org.chromium.net.ExperimentalUrlRequest$Builder setTrafficStatsTag(int);
public org.chromium.net.ExperimentalUrlRequest$Builder setTrafficStatsUid(int);
public org.chromium.net.ExperimentalUrlRequest$Builder setRequestFinishedListener(org.chromium.net.RequestFinishedInfo$Listener);
public org.chromium.net.ExperimentalUrlRequest$Builder setIdempotency(int);
public abstract org.chromium.net.ExperimentalUrlRequest$Builder setHttpMethod(java.lang.String);
public abstract org.chromium.net.ExperimentalUrlRequest$Builder addHeader(java.lang.String, java.lang.String);
public abstract org.chromium.net.ExperimentalUrlRequest$Builder disableCache();
......@@ -394,4 +398,4 @@ public abstract class org.chromium.net.UrlResponseInfo {
public abstract java.lang.String getProxyServer();
public abstract long getReceivedByteCount();
}
Stamp: 71c5599220fcd59906505a5d599e5f7e
Stamp: 1c548f381e3715d983e967e830d675d8
......@@ -105,6 +105,35 @@ public abstract class ExperimentalUrlRequest extends UrlRequest {
return this;
}
/**
* Default request idempotency, only enable 0-RTT for safe HTTP methods. Passed to {@link
* #setIdempotency}.
*/
public static final int DEFAULT_IDEMPOTENCY = 0;
/**
* Request is idempotent. Passed to {@link #setIdempotency}.
*/
public static final int IDEMPOTENT = 1;
/**
* Request is not idempotent. Passed to {@link #setIdempotency}.
*/
public static final int NOT_IDEMPOTENT = 2;
/**
* Sets idempotency of the request which should be one of the {@link #DEFAULT_IDEMPOTENCY
* IDEMPOTENT NOT_IDEMPOTENT} values. The default idempotency indicates that 0-RTT is only
* enabled for safe HTTP methods (GET, HEAD, OPTIONS, and TRACE).
*
* @param idempotency idempotency of the request which should be one of the {@link
* #DEFAULT_IDEMPOTENCY IDEMPOTENT NOT_IDEMPOTENT} values.
* @return the builder to facilitate chaining.
*/
public Builder setIdempotency(int idempotency) {
return this;
}
// To support method chaining, override superclass methods to return an
// instance of this class instead of the parent.
......
......@@ -16,6 +16,7 @@
#include "components/cronet/android/io_buffer_with_byte_buffer.h"
#include "components/cronet/android/url_request_error.h"
#include "components/cronet/metrics_util.h"
#include "net/base/idempotency.h"
#include "net/base/load_flags.h"
#include "net/base/load_states.h"
#include "net/base/net_errors.h"
......@@ -68,7 +69,8 @@ static jlong JNI_CronetUrlRequest_CreateRequestAdapter(
jboolean jtraffic_stats_tag_set,
jint jtraffic_stats_tag,
jboolean jtraffic_stats_uid_set,
jint jtraffic_stats_uid) {
jint jtraffic_stats_uid,
jint jidempotency) {
CronetURLRequestContextAdapter* context_adapter =
reinterpret_cast<CronetURLRequestContextAdapter*>(
jurl_request_context_adapter);
......@@ -83,7 +85,8 @@ static jlong JNI_CronetUrlRequest_CreateRequestAdapter(
context_adapter, env, jurl_request, url,
static_cast<net::RequestPriority>(jpriority), jdisable_cache,
jdisable_connection_migration, jenable_metrics, jtraffic_stats_tag_set,
jtraffic_stats_tag, jtraffic_stats_uid_set, jtraffic_stats_uid);
jtraffic_stats_tag, jtraffic_stats_uid_set, jtraffic_stats_uid,
static_cast<net::Idempotency>(jidempotency));
return reinterpret_cast<jlong>(adapter);
}
......@@ -100,7 +103,8 @@ CronetURLRequestAdapter::CronetURLRequestAdapter(
jboolean jtraffic_stats_tag_set,
jint jtraffic_stats_tag,
jboolean jtraffic_stats_uid_set,
jint jtraffic_stats_uid)
jint jtraffic_stats_uid,
net::Idempotency idempotency)
: request_(
new CronetURLRequest(context->cronet_url_request_context(),
std::unique_ptr<CronetURLRequestAdapter>(this),
......@@ -113,7 +117,7 @@ CronetURLRequestAdapter::CronetURLRequestAdapter(
jtraffic_stats_tag,
jtraffic_stats_uid_set == JNI_TRUE,
jtraffic_stats_uid,
/*idempotency=*/net::DEFAULT_IDEMPOTENCY)) {
idempotency)) {
owner_.Reset(env, jurl_request);
}
......
......@@ -54,7 +54,8 @@ class CronetURLRequestAdapter : public CronetURLRequest::Callback {
jboolean jtraffic_stats_tag_set,
jint jtraffic_stats_tag,
jboolean jtraffic_stats_uid_set,
jint jtraffic_stats_uid);
jint jtraffic_stats_uid,
net::Idempotency idempotency);
~CronetURLRequestAdapter() override;
// Methods called prior to Start are never called on network thread.
......
......@@ -235,7 +235,8 @@ final class FakeCronetEngine extends CronetEngineBase {
Executor userExecutor, int priority, Collection<Object> connectionAnnotations,
boolean disableCache, boolean disableConnectionMigration, boolean allowDirectExecutor,
boolean trafficStatsTagSet, int trafficStatsTag, boolean trafficStatsUidSet,
int trafficStatsUid, RequestFinishedInfo.Listener requestFinishedListener) {
int trafficStatsUid, RequestFinishedInfo.Listener requestFinishedListener,
int idempotency) {
synchronized (mLock) {
if (mIsShutdown) {
throw new IllegalStateException(
......
......@@ -54,13 +54,17 @@ public abstract class CronetEngineBase extends ExperimentalCronetEngine {
* @param trafficStatsUid UID to attribute traffic used to perform this request.
* @param requestFinishedListener callback to get invoked with metrics when request is finished.
* Set to {@code null} if not used.
* @param idempotency idempotency of the request which should be one of the
* {@link ExperimentalUrlRequest.Builder#DEFAULT_IDEMPOTENCY IDEMPOTENT NOT_IDEMPOTENT}
* values.
* @return new request.
*/
protected abstract UrlRequestBase createRequest(String url, UrlRequest.Callback callback,
Executor executor, @RequestPriority int priority, Collection<Object> requestAnnotations,
boolean disableCache, boolean disableConnectionMigration, boolean allowDirectExecutor,
boolean trafficStatsTagSet, int trafficStatsTag, boolean trafficStatsUidSet,
int trafficStatsUid, @Nullable RequestFinishedInfo.Listener requestFinishedListener);
int trafficStatsUid, @Nullable RequestFinishedInfo.Listener requestFinishedListener,
@Idempotency int idempotency);
/**
* Creates a {@link BidirectionalStream} object. {@code callback} methods will
......@@ -117,4 +121,10 @@ public abstract class CronetEngineBase extends ExperimentalCronetEngine {
})
@Retention(RetentionPolicy.SOURCE)
public @interface StreamPriority {}
@IntDef({ExperimentalUrlRequest.Builder.DEFAULT_IDEMPOTENCY,
ExperimentalUrlRequest.Builder.IDEMPOTENT,
ExperimentalUrlRequest.Builder.NOT_IDEMPOTENT})
@Retention(RetentionPolicy.SOURCE)
public @interface Idempotency {}
}
......@@ -14,6 +14,7 @@ import org.chromium.base.annotations.NativeClassQualifiedName;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.net.CallbackException;
import org.chromium.net.CronetException;
import org.chromium.net.Idempotency;
import org.chromium.net.InlineExecutionProhibitedException;
import org.chromium.net.NetworkException;
import org.chromium.net.RequestFinishedInfo;
......@@ -77,6 +78,7 @@ public final class CronetUrlRequest extends UrlRequestBase {
private final VersionSafeCallbacks.UrlRequestCallback mCallback;
private final String mInitialUrl;
private final int mPriority;
private final int mIdempotency;
private String mInitialMethod;
private final HeadersList mRequestHeaders = new HeadersList();
private final Collection<Object> mRequestAnnotations;
......@@ -139,7 +141,8 @@ public final class CronetUrlRequest extends UrlRequestBase {
UrlRequest.Callback callback, Executor executor, Collection<Object> requestAnnotations,
boolean disableCache, boolean disableConnectionMigration, boolean allowDirectExecutor,
boolean trafficStatsTagSet, int trafficStatsTag, boolean trafficStatsUidSet,
int trafficStatsUid, RequestFinishedInfo.Listener requestFinishedListener) {
int trafficStatsUid, RequestFinishedInfo.Listener requestFinishedListener,
int idempotency) {
if (url == null) {
throw new NullPointerException("URL is required");
}
......@@ -167,6 +170,7 @@ public final class CronetUrlRequest extends UrlRequestBase {
mRequestFinishedListener = requestFinishedListener != null
? new VersionSafeCallbacks.RequestFinishedInfoListener(requestFinishedListener)
: null;
mIdempotency = convertIdempotency(idempotency);
}
@Override
......@@ -213,7 +217,7 @@ public final class CronetUrlRequest extends UrlRequestBase {
mRequestContext.hasRequestFinishedListener()
|| mRequestFinishedListener != null,
mTrafficStatsTagSet, mTrafficStatsTag, mTrafficStatsUidSet,
mTrafficStatsUid);
mTrafficStatsUid, mIdempotency);
mRequestContext.onRequestStarted();
if (mInitialMethod != null) {
if (!CronetUrlRequestJni.get().setHttpMethod(
......@@ -417,6 +421,19 @@ public final class CronetUrlRequest extends UrlRequestBase {
}
}
private static int convertIdempotency(int idempotency) {
switch (idempotency) {
case Builder.DEFAULT_IDEMPOTENCY:
return Idempotency.DEFAULT_IDEMPOTENCY;
case Builder.IDEMPOTENT:
return Idempotency.IDEMPOTENT;
case Builder.NOT_IDEMPOTENT:
return Idempotency.NOT_IDEMPOTENT;
default:
return Idempotency.DEFAULT_IDEMPOTENCY;
}
}
private UrlResponseInfoImpl prepareResponseInfoOnNetworkThread(int httpStatusCode,
String httpStatusText, String[] headers, boolean wasCached, String negotiatedProtocol,
String proxyServer, long receivedByteCount) {
......@@ -832,7 +849,7 @@ public final class CronetUrlRequest extends UrlRequestBase {
long createRequestAdapter(CronetUrlRequest caller, long urlRequestContextAdapter,
String url, int priority, boolean disableCache, boolean disableConnectionMigration,
boolean enableMetrics, boolean trafficStatsTagSet, int trafficStatsTag,
boolean trafficStatsUidSet, int trafficStatsUid);
boolean trafficStatsUidSet, int trafficStatsUid, int idempotency);
@NativeClassQualifiedName("CronetURLRequestAdapter")
boolean setHttpMethod(long nativePtr, CronetUrlRequest caller, String method);
......
......@@ -223,13 +223,14 @@ public class CronetUrlRequestContext extends CronetEngineBase {
int priority, Collection<Object> requestAnnotations, boolean disableCache,
boolean disableConnectionMigration, boolean allowDirectExecutor,
boolean trafficStatsTagSet, int trafficStatsTag, boolean trafficStatsUidSet,
int trafficStatsUid, RequestFinishedInfo.Listener requestFinishedListener) {
int trafficStatsUid, RequestFinishedInfo.Listener requestFinishedListener,
int idempotency) {
synchronized (mLock) {
checkHaveAdapter();
return new CronetUrlRequest(this, url, priority, callback, executor, requestAnnotations,
disableCache, disableConnectionMigration, allowDirectExecutor,
trafficStatsTagSet, trafficStatsTag, trafficStatsUidSet, trafficStatsUid,
requestFinishedListener);
requestFinishedListener, idempotency);
}
}
......
......@@ -70,7 +70,8 @@ public final class JavaCronetEngine extends CronetEngineBase {
int priority, Collection<Object> connectionAnnotations, boolean disableCache,
boolean disableConnectionMigration, boolean allowDirectExecutor,
boolean trafficStatsTagSet, int trafficStatsTag, boolean trafficStatsUidSet,
int trafficStatsUid, RequestFinishedInfo.Listener requestFinishedListener) {
int trafficStatsUid, RequestFinishedInfo.Listener requestFinishedListener,
int idempotency) {
return new JavaUrlRequest(callback, mExecutorService, executor, url, mUserAgent,
allowDirectExecutor, trafficStatsTagSet, trafficStatsTag, trafficStatsUidSet,
trafficStatsUid);
......
......@@ -59,6 +59,9 @@ public class UrlRequestBuilderImpl extends ExperimentalUrlRequest.Builder {
private boolean mTrafficStatsUidSet;
private int mTrafficStatsUid;
private RequestFinishedInfo.Listener mRequestFinishedListener;
// Idempotency of the request.
@CronetEngineBase.Idempotency
private int mIdempotency = DEFAULT_IDEMPOTENCY;
/**
* Creates a builder for {@link UrlRequest} objects. All callbacks for
......@@ -139,6 +142,12 @@ public class UrlRequestBuilderImpl extends ExperimentalUrlRequest.Builder {
return this;
}
@Override
public UrlRequestBuilderImpl setIdempotency(@CronetEngineBase.Idempotency int idempotency) {
mIdempotency = idempotency;
return this;
}
@Override
public UrlRequestBuilderImpl setUploadDataProvider(
UploadDataProvider uploadDataProvider, Executor executor) {
......@@ -200,7 +209,7 @@ public class UrlRequestBuilderImpl extends ExperimentalUrlRequest.Builder {
final UrlRequestBase request = mCronetEngine.createRequest(mUrl, mCallback, mExecutor,
mPriority, mRequestAnnotations, mDisableCache, mDisableConnectionMigration,
mAllowDirectExecutor, mTrafficStatsTagSet, mTrafficStatsTag, mTrafficStatsUidSet,
mTrafficStatsUid, mRequestFinishedListener);
mTrafficStatsUid, mRequestFinishedListener, mIdempotency);
if (mMethod != null) {
request.setHttpMethod(mMethod);
}
......
......@@ -14,6 +14,9 @@ namespace net {
// the request. If the request has any side effects, those effects can happen
// multiple times. It is only safe to enable the 0-RTT if it is known that
// the request is idempotent.
// A Java counterpart will be generated for this enum.
// GENERATED_JAVA_ENUM_PACKAGE: org.chromium.net
// GENERATED_JAVA_CLASS_NAME_OVERRIDE: Idempotency
enum Idempotency {
DEFAULT_IDEMPOTENCY = 0,
IDEMPOTENT = 1,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment