[Cronet] Implement methods to get response headers in HttpURLConnection.

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

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 c6e6c2cb4a8151fe21c3245cc569425c62e25437..4afbaf0a737a22c21e99e6fab466bbf2a40022c5 100644
--- a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
+++ b/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
@@ -9,13 +9,15 @@ import android.util.Pair;
 
 import org.chromium.base.CalledByNative;
 import org.chromium.base.JNINamespace;
+import org.chromium.base.annotations.SuppressFBWarnings;
 
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
+import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
 import java.util.concurrent.Executor;
 
 /**
@@ -64,6 +66,24 @@ final class CronetUrlRequest implements UrlRequest {
      */
     private OnDataReceivedRunnable mOnDataReceivedTask;
 
+    // TODO(xunjieli): Share it with CronetHttpURLConnection.
+    private static final Comparator<String> FIELD_NAME_COMPARATOR =
+             new Comparator<String>() {
+        @Override
+        @SuppressFBWarnings("ES_STRING_EQUALS")

[mmenke, 2015/01/16 19:43:13]

Rather than suppress the warning, maybe just:

if (a == null) {
  if (b == null) {
    return 0;
  } else {
    return -1;
  }
}
if (b == null) ...

It's a little uglier, but I think it's better than suppressing a compiler
warning.

Alternatively, may be a little prettier to first if with "if (a == null && b ==
null)"

[xunjieli, 2015/01/16 20:01:12]

Done. That's solved my problem. Didn't see it earlier. Thank you!

+        public int compare(String a, String b) {
+            if (a == b) {
+                return 0;
+            } else if (a == null) {

[mmenke, 2015/01/16 19:43:13]

Actually...Can we even have NULL response header strings?  Why do we even need
this?

If it can happen, we should have tests for it.

[xunjieli, 2015/01/16 20:01:12]

The default implementation has a null key which corresponds to the http status
line. We have not added it. But I think we should keep it in case we add null
key in the future?

[mmenke, 2015/01/16 20:03:59]

My opinion on this (And others may disagree, though they're wrong, of course) is
that the NULL key is a hack, and we should instead have an API to expose the
response line.

If we want to support that behavior in the HttpUrlConnection wrapper (Which I
think makes sense), then we should actually create a new map with the extra
entry added.

+                return -1;
+            } else if (b == null) {
+                return 1;
+            } else {
+                return String.CASE_INSENSITIVE_ORDER.compare(a, b);
+            }
+        }
+    };
+
     static final class HeadersList extends ArrayList<Pair<String, String>> {
     }
 
@@ -117,6 +137,7 @@ final class CronetUrlRequest implements UrlRequest {
         private final HeadersList mAllHeaders = new HeadersList();
         private final boolean mWasCached;
         private final String mNegotiatedProtocol;
+        private Map<String, List<String>> mResponseHeaders;
 
         NativeResponseInfo(String[] urlChain, int httpStatusCode,
                 String httpStatusText, boolean wasCached,
@@ -155,17 +176,21 @@ final class CronetUrlRequest implements UrlRequest {
 
         @Override
         public Map<String, List<String>> getAllHeaders() {
-            Map<String, List<String>> map = new HashMap<String, List<String>>();
+            if (mResponseHeaders != null) {
+                return mResponseHeaders;
+            }
+            Map<String, List<String>> map =
+                    new TreeMap<String, List<String>>(FIELD_NAME_COMPARATOR);

[mmenke, 2015/01/16 19:43:13]

Hrm...The comparison is not "consistent with equals", per docs, but looks like,
at least on Android, we have guaranteed reasonable behavior in that case.

http://developer.android.com/reference/java/util/TreeMap.html

[xunjieli, 2015/01/16 20:01:12]

Acknowledged.

             for (Pair<String, String> entry : mAllHeaders) {
+                List<String> values = new ArrayList<String>();
                 if (map.containsKey(entry.first)) {
-                    map.get(entry.first).add(entry.second);
-                } else {
-                    List<String> values = new ArrayList<String>();
-                    values.add(entry.second);
-                    map.put(entry.first, values);
+                    values.addAll(map.get(entry.first));
                 }
+                values.add(entry.second);
+                map.put(entry.first, Collections.unmodifiableList(values));
             }
-            return Collections.unmodifiableMap(map);
+            mResponseHeaders = Collections.unmodifiableMap(map);
+            return mResponseHeaders;
         }
 
         @Override