Support setReadTimeout in CronetHttpURLConnection

components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.net.urlconnection;
 
 import android.util.Pair;
 
 import org.chromium.base.Log;
 import org.chromium.net.CronetEngine;
@@ skipping 387 matching lines @@
 
     /**
      * Used by {@link CronetInputStream} to get more data from the network
      * stack. This should only be called after the request has started. Note
      * that this call might block if there isn't any more data to be read.
      * Since byteBuffer is passed to the UrlRequest, it must be a direct
      * ByteBuffer.
      */
     void getMoreData(ByteBuffer byteBuffer) throws IOException {
         mRequest.read(byteBuffer);
-        mMessageLoop.loop();
+        mMessageLoop.loop(getReadTimeout());
     }
 
     /**
      * Returns the index of request header in {@link #mRequestHeaders} or
      * -1 if not found.
      */
     private int findRequestProperty(String key) {
         for (int i = 0; i < mRequestHeaders.size(); i++) {
             Pair<String, String> entry = mRequestHeaders.get(i);
             if (entry.first.equalsIgnoreCase(key)) {
@@ skipping 90 matching lines @@
         if (mOutputStream != null) {
             mOutputStream.checkReceivedEnoughContent();
             if (isChunkedUpload()) {
                 // Write last chunk.
                 mOutputStream.close();
             }
         }
         if (!mHasResponse) {
             startRequest();
             // Blocks until onResponseStarted or onFailed is called.
-            mMessageLoop.loop();
+            mMessageLoop.loop(getConnectTimeout());

[rohitagr, 2016/05/16 17:21:56]

I don't think this is safe: as I understand it, the connect timeout is supposed
to cap the amount of time it takes to establish the TCP connection, rather than
the amount of time it takes to get the headers.

[xunjieli, 2016/05/16 17:28:10]

Yea, although the specs is pretty vague about what is a connect timeout. "Sets
the maximum time in milliseconds to wait while connecting."
I think this is a good proxy, although I don't really know what values users of
HttpURLConnection are setting as the connect timeout. If the often-used values
are very short, I agree that doing this is probably not safe.

[kapishnikov, 2016/05/17 20:33:24]

Following up the Helen's PS4 change. I think the good approximation here would
be getConnectTimeout() +
getReadTimeout() value as the timeout (upper bound). That is what actually can
happen here, i.e. establishing the connection and reading the headers. This is
true even if connect() method is called before, since connect() method in Cronet
does not guarantee that the connection was actually established.

             mHasResponse = true;
         }
         checkHasResponse();
     }
 
     /**
      * Checks whether response headers are received, and throws an exception if
      * an exception occurred before headers received. This method should only be
      * called after onResponseStarted or onFailed.
      */
@@ skipping 58 matching lines @@
             // Strips Content-Encoding response header. See crbug.com/592700.
             if (!entry.getKey().equalsIgnoreCase("Content-Encoding")) {
                 mResponseHeadersList.add(
                         new AbstractMap.SimpleImmutableEntry<String, String>(entry));
             }
         }
         mResponseHeadersList = Collections.unmodifiableList(mResponseHeadersList);
         return mResponseHeadersList;
     }
 }