components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java - Issue 1359343005: Update ResponseInfo to UrlResponseInfo with API review comments.

Unified Diff: components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java

Issue 1359343005: Update ResponseInfo to UrlResponseInfo with API review comments. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Rebase to CronetEngine builders. Created 5 years, 2 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« no previous file with comments | « components/cronet.gypi ('k') | components/cronet/android/java/src/org/chromium/net/ExtendedResponseInfo.java » ('j') | components/cronet/android/java/src/org/chromium/net/UrlResponseInfo.java » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java

diff --git a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java b/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java

index 104b7220197f4a273da0dfc35d066254f8464087..2285fb21a1900694e93df4e4b8342c00e9507ae7 100644

--- a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java

+++ b/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java

@@ -5,7 +5,6 @@

package org.chromium.net;

import android.util.Log;

-import android.util.Pair;

import org.chromium.base.VisibleForTesting;

import org.chromium.base.annotations.CalledByNative;

@@ -14,11 +13,10 @@ import org.chromium.base.annotations.JNINamespace;

import org.chromium.base.annotations.NativeClassQualifiedName;

import java.nio.ByteBuffer;

+import java.util.AbstractMap;

import java.util.ArrayList;

-import java.util.Collections;

import java.util.List;

import java.util.Map;

-import java.util.TreeMap;

import java.util.concurrent.Executor;

import java.util.concurrent.RejectedExecutionException;

@@ -73,7 +71,7 @@ final class CronetUrlRequest implements UrlRequest {

private CronetUploadDataStream mUploadDataStream;

- private NativeResponseInfo mResponseInfo;

+ private UrlResponseInfo mResponseInfo;

* Listener callback is repeatedly called when each read is completed, so it

@@ -83,7 +81,7 @@ final class CronetUrlRequest implements UrlRequest {

private Runnable mOnDestroyedCallbackForTests;

- private static final class HeadersList extends ArrayList<Pair<String, String>> {}

+ private static final class HeadersList extends ArrayList<Map.Entry<String, String>> {}

private final class OnReadCompletedRunnable implements Runnable {

ByteBuffer mByteBuffer;

@@ -113,113 +111,6 @@ final class CronetUrlRequest implements UrlRequest {

}

- private static final class NativeResponseInfo implements ResponseInfo {

- private final String[] mResponseInfoUrlChain;

- private final int mHttpStatusCode;

- private final String mHttpStatusText;

- private final boolean mWasCached;

- private final String mNegotiatedProtocol;

- private final String mProxyServer;

- private final HeadersList mAllHeaders = new HeadersList();

- private Map<String, List<String>> mResponseHeaders;

- private List<Pair<String, String>> mUnmodifiableAllHeaders;

- NativeResponseInfo(String[] urlChain, int httpStatusCode,

- String httpStatusText, boolean wasCached,

- String negotiatedProtocol, String proxyServer) {

- mResponseInfoUrlChain = urlChain;

- mHttpStatusCode = httpStatusCode;

- mHttpStatusText = httpStatusText;

- mWasCached = wasCached;

- mNegotiatedProtocol = negotiatedProtocol;

- mProxyServer = proxyServer;

- }

- @Override

- public String getUrl() {

- return mResponseInfoUrlChain[mResponseInfoUrlChain.length - 1];

- }

- @Override

- public String[] getUrlChain() {

- return mResponseInfoUrlChain;

- }

- @Override

- public int getHttpStatusCode() {

- return mHttpStatusCode;

- }

- @Override

- public String getHttpStatusText() {

- return mHttpStatusText;

- }

- @Override

- public List<Pair<String, String>> getAllHeadersAsList() {

- if (mUnmodifiableAllHeaders == null) {

- mUnmodifiableAllHeaders =

- Collections.unmodifiableList(mAllHeaders);

- }

- return mUnmodifiableAllHeaders;

- }

- @Override

- public Map<String, List<String>> getAllHeaders() {

- if (mResponseHeaders != null) {

- return mResponseHeaders;

- }

- Map<String, List<String>> map = new TreeMap<String, List<String>>(

- String.CASE_INSENSITIVE_ORDER);

- for (Pair<String, String> entry : mAllHeaders) {

- List<String> values = new ArrayList<String>();

- if (map.containsKey(entry.first)) {

- values.addAll(map.get(entry.first));

- }

- values.add(entry.second);

- map.put(entry.first, Collections.unmodifiableList(values));

- }

- mResponseHeaders = Collections.unmodifiableMap(map);

- return mResponseHeaders;

- }

- @Override

- public boolean wasCached() {

- return mWasCached;

- }

- @Override

- public String getNegotiatedProtocol() {

- return mNegotiatedProtocol;

- }

- @Override

- public String getProxyServer() {

- return mProxyServer;

- }

- };

- private static final class NativeExtendedResponseInfo implements ExtendedResponseInfo {

- private final ResponseInfo mResponseInfo;

- private final long mTotalReceivedBytes;

- NativeExtendedResponseInfo(ResponseInfo responseInfo,

- long totalReceivedBytes) {

- mResponseInfo = responseInfo;

- mTotalReceivedBytes = totalReceivedBytes;

- }

- @Override

- public ResponseInfo getResponseInfo() {

- return mResponseInfo;

- }

- @Override

- public long getTotalReceivedBytes() {

- return mTotalReceivedBytes;

- }

- };

CronetUrlRequest(CronetUrlRequestContext requestContext,

long urlRequestContextAdapter,

String url,

@@ -262,7 +153,7 @@ final class CronetUrlRequest implements UrlRequest {

if (value == null) {

throw new NullPointerException("Invalid header value.");

}

- mRequestHeaders.add(Pair.create(header, value));

+ mRequestHeaders.add(new AbstractMap.SimpleImmutableEntry<String, String>(header, value));

}

@Override

@@ -292,15 +183,16 @@ final class CronetUrlRequest implements UrlRequest {

}

boolean hasContentType = false;

- for (Pair<String, String> header : mRequestHeaders) {

- if (header.first.equalsIgnoreCase("Content-Type")

- && !header.second.isEmpty()) {

+ for (Map.Entry<String, String> header : mRequestHeaders) {

+ if (header.getKey().equalsIgnoreCase("Content-Type")

+ && !header.getValue().isEmpty()) {

hasContentType = true;

}

- if (!nativeAddRequestHeader(mUrlRequestAdapter, header.first, header.second)) {

+ if (!nativeAddRequestHeader(

+ mUrlRequestAdapter, header.getKey(), header.getValue())) {

destroyRequestAdapter();

throw new IllegalArgumentException(

- "Invalid header " + header.first + "=" + header.second);

+ "Invalid header " + header.getKey() + "=" + header.getValue());

}

if (mUploadDataStream != null) {

@@ -481,8 +373,7 @@ final class CronetUrlRequest implements UrlRequest {

}

- private NativeResponseInfo prepareResponseInfoOnNetworkThread(

- int httpStatusCode) {

+ private UrlResponseInfo prepareResponseInfoOnNetworkThread(int httpStatusCode) {

long urlRequestAdapter;

synchronized (mUrlRequestAdapterLock) {

if (mUrlRequestAdapter == 0) {

@@ -494,15 +385,14 @@ final class CronetUrlRequest implements UrlRequest {

// safe to preserve and use urlRequestAdapter outside the lock.

urlRequestAdapter = mUrlRequestAdapter;

}

- NativeResponseInfo responseInfo = new NativeResponseInfo(

- mUrlChain.toArray(new String[mUrlChain.size()]),

- httpStatusCode,

- nativeGetHttpStatusText(urlRequestAdapter),

+ HeadersList headersList = new HeadersList();

+ nativePopulateResponseHeaders(urlRequestAdapter, headersList);

+ UrlResponseInfo responseInfo = new UrlResponseInfo(new ArrayList<String>(mUrlChain),

+ httpStatusCode, nativeGetHttpStatusText(urlRequestAdapter), headersList,

nativeGetWasCached(urlRequestAdapter),

nativeGetNegotiatedProtocol(urlRequestAdapter),

nativeGetProxyServer(urlRequestAdapter));

- nativePopulateResponseHeaders(urlRequestAdapter,

- responseInfo.mAllHeaders);

return responseInfo;

}

@@ -606,8 +496,7 @@ final class CronetUrlRequest implements UrlRequest {

@CalledByNative

private void onReceivedRedirect(final String newLocation,

int httpStatusCode) {

- final NativeResponseInfo responseInfo =

- prepareResponseInfoOnNetworkThread(httpStatusCode);

+ final UrlResponseInfo responseInfo = prepareResponseInfoOnNetworkThread(httpStatusCode);

// Have to do this after creating responseInfo.

mUrlChain.add(newLocation);

@@ -701,9 +590,7 @@ final class CronetUrlRequest implements UrlRequest {

@SuppressWarnings("unused")

@CalledByNative

private void onSucceeded(long totalReceivedBytes) {

- final NativeExtendedResponseInfo extendedResponseInfo =

- new NativeExtendedResponseInfo(mResponseInfo,

- totalReceivedBytes);

+ mResponseInfo.setReceivedBytesCount(totalReceivedBytes);

Runnable task = new Runnable() {

public void run() {

synchronized (mUrlRequestAdapterLock) {

@@ -715,8 +602,7 @@ final class CronetUrlRequest implements UrlRequest {

destroyRequestAdapter();

}

try {

- mListener.onSucceeded(CronetUrlRequest.this,

- extendedResponseInfo);

+ mListener.onSucceeded(CronetUrlRequest.this, mResponseInfo);

} catch (Exception e) {

Log.e(CronetUrlRequestContext.LOG_TAG,

"Exception in onComplete method", e);

@@ -748,7 +634,7 @@ final class CronetUrlRequest implements UrlRequest {

@CalledByNative

private void onAppendResponseHeader(HeadersList headersList,

String name, String value) {

- headersList.add(Pair.create(name, value));

+ headersList.add(new AbstractMap.SimpleImmutableEntry<String, String>(name, value));

}

/**