Skip to content

GitLab

T
tangled
Commit 45280ea9 authored by pauljensen's avatar pauljensen Committed by Commit bot
Browse files
Options

[Cronet] Add callback wrapper classes to enforce API version checking. 

Wrap Cronet callback/listener classes in wrapper classes whose job it
is to enforce that callbacks added in later versions are not called
when the client API is of an older version.  For now all wrapper
classes simply pass through all the callbacks as we're still working
on the first version of the API, which we shall support indefinitly.

A side benefit is this facilitates only passing implementation, not
API, classes to JNI functions like nativeGetStatus.  This is
required to support cases where the Cronet API may be repackaged
into another class at the Java layer, but not at the JNI layer.

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
BUG=629299
R=kapishnikov

Review-Url: https://codereview.chromium.org/2514783002
Cr-Commit-Position: refs/heads/master@{#434891}
parent 7104b0c4
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 368 additions and 50 deletions
components/cronet/android/BUILD.gn
View file @ 45280ea9
......@@ -318,6 +318,7 @@ android_library("cronet_impl_common_java") {
"java/src/org/chromium/net/impl/UrlRequestBuilderImpl.java",
"java/src/org/chromium/net/impl/UrlResponseInfoImpl.java",
"java/src/org/chromium/net/impl/UserAgent.java",
"java/src/org/chromium/net/impl/VersionSafeCallbacks.java",
]
deps = [
......
components/cronet/android/java/src/org/chromium/net/impl/CronetBidirectionalStream.java
View file @ 45280ea9
......@@ -79,7 +79,7 @@ public class CronetBidirectionalStream extends ExperimentalBidirectionalStream {
private final CronetUrlRequestContext mRequestContext;
private final Executor mExecutor;
private final Callback mCallback;
private final VersionSafeCallbacks.BidirectionalStreamCallback mCallback;
private final String mInitialUrl;
private final int mInitialPriority;
private final String mInitialMethod;
......@@ -230,7 +230,7 @@ public class CronetBidirectionalStream extends ExperimentalBidirectionalStream {
mRequestContext = requestContext;
mInitialUrl = url;
mInitialPriority = convertStreamPriority(priority);
mCallback = callback;
mCallback = new VersionSafeCallbacks.BidirectionalStreamCallback(callback);
mExecutor = executor;
mInitialMethod = httpMethod;
mRequestHeaders = stringsFromHeaderList(requestHeaders);
......
components/cronet/android/java/src/org/chromium/net/impl/CronetEngineBuilderImpl.java
View file @ 45280ea9
......@@ -87,7 +87,7 @@ public class CronetEngineBuilderImpl extends ICronetEngineBuilder {
private String mUserAgent;
private String mStoragePath;
private boolean mLegacyModeEnabled;
private CronetEngine.Builder.LibraryLoader mLibraryLoader;
private VersionSafeCallbacks.LibraryLoader mLibraryLoader;
private boolean mQuicEnabled;
private boolean mHttp2Enabled;
private boolean mSdchEnabled;
......@@ -158,11 +158,11 @@ public class CronetEngineBuilderImpl extends ICronetEngineBuilder {
@Override
public CronetEngineBuilderImpl setLibraryLoader(CronetEngine.Builder.LibraryLoader loader) {
mLibraryLoader = loader;
mLibraryLoader = new VersionSafeCallbacks.LibraryLoader(loader);
return this;
}
CronetEngine.Builder.LibraryLoader libraryLoader() {
VersionSafeCallbacks.LibraryLoader libraryLoader() {
return mLibraryLoader;
}
......
components/cronet/android/java/src/org/chromium/net/impl/CronetUploadDataStream.java
View file @ 45280ea9
......@@ -36,7 +36,7 @@ public final class CronetUploadDataStream extends UploadDataSink {
private static final String TAG = "CronetUploadDataStream";
// These are never changed, once a request starts.
private final Executor mExecutor;
private final UploadDataProvider mDataProvider;
private final VersionSafeCallbacks.UploadDataProviderWrapper mDataProvider;
private long mLength;
private long mRemainingLength;
private CronetUrlRequest mRequest;
......@@ -98,7 +98,7 @@ public final class CronetUploadDataStream extends UploadDataSink {
*/
public CronetUploadDataStream(UploadDataProvider dataProvider, Executor executor) {
mExecutor = executor;
mDataProvider = dataProvider;
mDataProvider = new VersionSafeCallbacks.UploadDataProviderWrapper(dataProvider);
}
/**
......
components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequest.java
View file @ 45280ea9
......@@ -39,8 +39,8 @@ import javax.annotation.concurrent.GuardedBy;
* any thread it is protected by mUrlRequestAdapterLock.
*/
@JNINamespace("cronet")
// Qualifies UrlRequest.StatusListener which is used in onStatus, a JNI method.
@JNIAdditionalImport(UrlRequest.class)
// Qualifies VersionSafeCallbacks.UrlRequestStatusListener which is used in onStatus, a JNI method.
@JNIAdditionalImport(VersionSafeCallbacks.class)
@VisibleForTesting
public final class CronetUrlRequest extends UrlRequestBase {
private final boolean mAllowDirectExecutor;
......@@ -74,7 +74,7 @@ public final class CronetUrlRequest extends UrlRequestBase {
private final List<String> mUrlChain = new ArrayList<String>();
private long mReceivedBytesCountFromRedirects;
private final UrlRequest.Callback mCallback;
private final VersionSafeCallbacks.UrlRequestCallback mCallback;
private final String mInitialUrl;
private final int mPriority;
private String mInitialMethod;
......@@ -143,7 +143,7 @@ public final class CronetUrlRequest extends UrlRequestBase {
mInitialUrl = url;
mUrlChain.add(url);
mPriority = convertRequestPriority(priority);
mCallback = callback;
mCallback = new VersionSafeCallbacks.UrlRequestCallback(callback);
mExecutor = executor;
mRequestAnnotations = requestAnnotations;
mDisableCache = disableCache;
......@@ -314,7 +314,9 @@ public final class CronetUrlRequest extends UrlRequestBase {
}
@Override
public void getStatus(final UrlRequest.StatusListener listener) {
public void getStatus(UrlRequest.StatusListener unsafeListener) {
final VersionSafeCallbacks.UrlRequestStatusListener listener =
new VersionSafeCallbacks.UrlRequestStatusListener(unsafeListener);
synchronized (mUrlRequestAdapterLock) {
if (mUrlRequestAdapter != 0) {
nativeGetStatus(mUrlRequestAdapter, listener);
......@@ -675,7 +677,8 @@ public final class CronetUrlRequest extends UrlRequestBase {
*/
@SuppressWarnings("unused")
@CalledByNative
private void onStatus(final UrlRequest.StatusListener listener, final int loadState) {
private void onStatus(
final VersionSafeCallbacks.UrlRequestStatusListener listener, final int loadState) {
Runnable task = new Runnable() {
@Override
public void run() {
......@@ -777,5 +780,6 @@ public final class CronetUrlRequest extends UrlRequestBase {
private native void nativeDestroy(long nativePtr, boolean sendOnCanceled);
@NativeClassQualifiedName("CronetURLRequestAdapter")
private native void nativeGetStatus(long nativePtr, UrlRequest.StatusListener listener);
private native void nativeGetStatus(
long nativePtr, VersionSafeCallbacks.UrlRequestStatusListener listener);
}
components/cronet/android/java/src/org/chromium/net/impl/CronetUrlRequestContext.java
View file @ 45280ea9
......@@ -33,6 +33,7 @@ import java.net.URLConnection;
import java.net.URLStreamHandlerFactory;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.Executor;
......@@ -115,16 +116,21 @@ public class CronetUrlRequestContext extends CronetEngineBase {
private int mDownstreamThroughputKbps = RttThroughputValues.INVALID_RTT_THROUGHPUT;
@GuardedBy("mNetworkQualityLock")
private final ObserverList<NetworkQualityRttListener> mRttListenerList =
new ObserverList<NetworkQualityRttListener>();
private final ObserverList<VersionSafeCallbacks.NetworkQualityRttListenerWrapper>
mRttListenerList =
new ObserverList<VersionSafeCallbacks.NetworkQualityRttListenerWrapper>();
@GuardedBy("mNetworkQualityLock")
private final ObserverList<NetworkQualityThroughputListener> mThroughputListenerList =
new ObserverList<NetworkQualityThroughputListener>();
private final ObserverList<VersionSafeCallbacks.NetworkQualityThroughputListenerWrapper>
mThroughputListenerList =
new ObserverList<VersionSafeCallbacks
.NetworkQualityThroughputListenerWrapper>();
@GuardedBy("mFinishedListenerLock")
private final List<RequestFinishedInfo.Listener> mFinishedListenerList =
new ArrayList<RequestFinishedInfo.Listener>();
private final Map<RequestFinishedInfo.Listener,
VersionSafeCallbacks.RequestFinishedInfoListener> mFinishedListenerMap =
new HashMap<RequestFinishedInfo.Listener,
VersionSafeCallbacks.RequestFinishedInfoListener>();
/**
* Synchronize access to mCertVerifierData.
......@@ -412,7 +418,8 @@ public class CronetUrlRequestContext extends CronetEngineBase {
nativeProvideRTTObservations(mUrlRequestContextAdapter, true);
}
}
mRttListenerList.addObserver(listener);
mRttListenerList.addObserver(
new VersionSafeCallbacks.NetworkQualityRttListenerWrapper(listener));
}
}
......@@ -422,11 +429,13 @@ public class CronetUrlRequestContext extends CronetEngineBase {
throw new IllegalStateException("Network quality estimator must be enabled");
}
synchronized (mNetworkQualityLock) {
mRttListenerList.removeObserver(listener);
if (mRttListenerList.isEmpty()) {
synchronized (mLock) {
checkHaveAdapter();
nativeProvideRTTObservations(mUrlRequestContextAdapter, false);
if (mRttListenerList.removeObserver(
new VersionSafeCallbacks.NetworkQualityRttListenerWrapper(listener))) {
if (mRttListenerList.isEmpty()) {
synchronized (mLock) {
checkHaveAdapter();
nativeProvideRTTObservations(mUrlRequestContextAdapter, false);
}
}
}
}
......@@ -444,7 +453,8 @@ public class CronetUrlRequestContext extends CronetEngineBase {
nativeProvideThroughputObservations(mUrlRequestContextAdapter, true);
}
}
mThroughputListenerList.addObserver(listener);
mThroughputListenerList.addObserver(
new VersionSafeCallbacks.NetworkQualityThroughputListenerWrapper(listener));
}
}
......@@ -454,11 +464,14 @@ public class CronetUrlRequestContext extends CronetEngineBase {
throw new IllegalStateException("Network quality estimator must be enabled");
}
synchronized (mNetworkQualityLock) {
mThroughputListenerList.removeObserver(listener);
if (mThroughputListenerList.isEmpty()) {
synchronized (mLock) {
checkHaveAdapter();
nativeProvideThroughputObservations(mUrlRequestContextAdapter, false);
if (mThroughputListenerList.removeObserver(
new VersionSafeCallbacks.NetworkQualityThroughputListenerWrapper(
listener))) {
if (mThroughputListenerList.isEmpty()) {
synchronized (mLock) {
checkHaveAdapter();
nativeProvideThroughputObservations(mUrlRequestContextAdapter, false);
}
}
}
}
......@@ -467,20 +480,21 @@ public class CronetUrlRequestContext extends CronetEngineBase {
@Override
public void addRequestFinishedListener(RequestFinishedInfo.Listener listener) {
synchronized (mFinishedListenerLock) {
mFinishedListenerList.add(listener);
mFinishedListenerMap.put(
listener, new VersionSafeCallbacks.RequestFinishedInfoListener(listener));
}
}
@Override
public void removeRequestFinishedListener(RequestFinishedInfo.Listener listener) {
synchronized (mFinishedListenerLock) {
mFinishedListenerList.remove(listener);
mFinishedListenerMap.remove(listener);
}
}
boolean hasRequestFinishedListener() {
synchronized (mFinishedListenerLock) {
return !mFinishedListenerList.isEmpty();
return !mFinishedListenerMap.isEmpty();
}
}
......@@ -613,7 +627,8 @@ public class CronetUrlRequestContext extends CronetEngineBase {
@CalledByNative
private void onRttObservation(final int rttMs, final long whenMs, final int source) {
synchronized (mNetworkQualityLock) {
for (final NetworkQualityRttListener listener : mRttListenerList) {
for (final VersionSafeCallbacks.NetworkQualityRttListenerWrapper listener :
mRttListenerList) {
Runnable task = new Runnable() {
@Override
public void run() {
......@@ -630,7 +645,8 @@ public class CronetUrlRequestContext extends CronetEngineBase {
private void onThroughputObservation(
final int throughputKbps, final long whenMs, final int source) {
synchronized (mNetworkQualityLock) {
for (final NetworkQualityThroughputListener listener : mThroughputListenerList) {
for (final VersionSafeCallbacks.NetworkQualityThroughputListenerWrapper listener :
mThroughputListenerList) {
Runnable task = new Runnable() {
@Override
public void run() {
......@@ -650,11 +666,12 @@ public class CronetUrlRequestContext extends CronetEngineBase {
}
void reportFinished(final RequestFinishedInfo requestInfo) {
ArrayList<RequestFinishedInfo.Listener> currentListeners;
ArrayList<VersionSafeCallbacks.RequestFinishedInfoListener> currentListeners;
synchronized (mFinishedListenerLock) {
currentListeners = new ArrayList<RequestFinishedInfo.Listener>(mFinishedListenerList);
currentListeners = new ArrayList<VersionSafeCallbacks.RequestFinishedInfoListener>(
mFinishedListenerMap.values());
}
for (final RequestFinishedInfo.Listener listener : currentListeners) {
for (final VersionSafeCallbacks.RequestFinishedInfoListener listener : currentListeners) {
Runnable task = new Runnable() {
@Override
public void run() {
......
components/cronet/android/java/src/org/chromium/net/impl/JavaUrlRequest.java
View file @ 45280ea9
......@@ -77,7 +77,7 @@ final class JavaUrlRequest extends UrlRequestBase {
/* These don't change with redirects */
private String mInitialMethod;
private UploadDataProvider mUploadDataProvider;
private VersionSafeCallbacks.UploadDataProviderWrapper mUploadDataProvider;
private Executor mUploadExecutor;
/**
......@@ -96,7 +96,7 @@ final class JavaUrlRequest extends UrlRequestBase {
/* These change with redirects. */
private String mCurrentUrl;
private ReadableByteChannel mResponseChannel;
private UrlResponseInfo mUrlResponseInfo;
private UrlResponseInfoImpl mUrlResponseInfo;
private String mPendingRedirectUrl;
/**
* The happens-before edges created by the executor submission and AtomicReference setting are
......@@ -249,7 +249,8 @@ final class JavaUrlRequest extends UrlRequestBase {
if (mInitialMethod == null) {
mInitialMethod = "POST";
}
this.mUploadDataProvider = uploadDataProvider;
this.mUploadDataProvider =
new VersionSafeCallbacks.UploadDataProviderWrapper(uploadDataProvider);
if (mAllowDirectExecutor) {
this.mUploadExecutor = executor;
} else {
......@@ -271,7 +272,7 @@ final class JavaUrlRequest extends UrlRequestBase {
final HttpURLConnection mUrlConnection;
WritableByteChannel mOutputChannel;
OutputStream mUrlConnectionOutputStream;
final UploadDataProvider mUploadProvider;
final VersionSafeCallbacks.UploadDataProviderWrapper mUploadProvider;
ByteBuffer mBuffer;
/** This holds the total bytes to send (the content-length). -1 if unknown. */
long mTotalBytes;
......@@ -279,7 +280,8 @@ final class JavaUrlRequest extends UrlRequestBase {
long mWrittenBytes = 0;
OutputStreamDataSink(final Executor userExecutor, Executor executor,
HttpURLConnection urlConnection, UploadDataProvider provider) {
HttpURLConnection urlConnection,
VersionSafeCallbacks.UploadDataProviderWrapper provider) {
this.mUserUploadExecutor = new Executor() {
@Override
public void execute(Runnable runnable) {
......@@ -790,17 +792,18 @@ final class JavaUrlRequest extends UrlRequestBase {
throw new IllegalStateException("Switch is exhaustive: " + state);
}
mCallbackAsync.sendStatus(listener, status);
mCallbackAsync.sendStatus(
new VersionSafeCallbacks.UrlRequestStatusListener(listener), status);
}
/** This wrapper ensures that callbacks are always called on the correct executor */
private final class AsyncUrlRequestCallback {
final Callback mCallback;
final VersionSafeCallbacks.UrlRequestCallback mCallback;
final Executor mUserExecutor;
final Executor mFallbackExecutor;
AsyncUrlRequestCallback(Callback callback, final Executor userExecutor) {
this.mCallback = callback;
this.mCallback = new VersionSafeCallbacks.UrlRequestCallback(callback);
if (mAllowDirectExecutor) {
this.mUserExecutor = userExecutor;
this.mFallbackExecutor = null;
......@@ -810,7 +813,8 @@ final class JavaUrlRequest extends UrlRequestBase {
}
}
void sendStatus(final StatusListener listener, final int status) {
void sendStatus(
final VersionSafeCallbacks.UrlRequestStatusListener listener, final int status) {
mUserExecutor.execute(new Runnable() {
@Override
public void run() {
......
components/cronet/android/java/src/org/chromium/net/impl/UrlResponseInfoImpl.java
View file @ 45280ea9
......@@ -29,7 +29,7 @@ public final class UrlResponseInfoImpl extends UrlResponseInfo {
private final String mNegotiatedProtocol;
private final String mProxyServer;
private final AtomicLong mReceivedBytesCount;
private final HeaderBlock mHeaders;
private final HeaderBlockImpl mHeaders;
/**
* Unmodifiable container of response headers or trailers.
......
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