[Cronet] Allow UrlRequest bypass cache

This CL is to correctly support disabling/enabling per-request caching in
HttpURLConnection.

BUG=398997

Review URL: https://codereview.chromium.org/860893002

Cr-Commit-Position: refs/heads/master@{#315327}

9 files changed, 156 insertions, 6 deletions

diff --git a/components/cronet/android/cronet_url_request.cc b/components/cronet/android/cronet_url_request.cc
index 11bfaca..bc7b29f 100644
--- a/components/cronet/android/cronet_url_request.cc
+++ b/components/cronet/android/cronet_url_request.cc
@@ -208,6 +208,16 @@ static void FollowDeferredRedirect(JNIEnv* env,
                  base::Unretained(request_adapter)));
 }
 
+static void DisableCache(JNIEnv* env,
+                         jobject jurl_request,
+                         jlong jurl_request_adapter) {
+  DCHECK(jurl_request_adapter);
+  CronetURLRequestAdapter* request_adapter =
+      reinterpret_cast<CronetURLRequestAdapter*>(jurl_request_adapter);
+  DCHECK(!request_adapter->IsOnNetworkThread());
+  request_adapter->DisableCache();
+}
+
 static void PopulateResponseHeaders(JNIEnv* env,
                                     jobject jurl_request,
                                     jlong jurl_request_adapter,
diff --git a/components/cronet/android/cronet_url_request_adapter.cc b/components/cronet/android/cronet_url_request_adapter.cc
index 265895a..e16ebe1 100644
--- a/components/cronet/android/cronet_url_request_adapter.cc
+++ b/components/cronet/android/cronet_url_request_adapter.cc
@@ -28,7 +28,8 @@ CronetURLRequestAdapter::CronetURLRequestAdapter(
       delegate_(delegate.Pass()),
       initial_url_(url),
       initial_priority_(priority),
-      initial_method_("GET") {
+      initial_method_("GET"),
+      load_flags_(context->default_load_flags()) {
 }
 
 CronetURLRequestAdapter::~CronetURLRequestAdapter() {
@@ -41,6 +42,11 @@ void CronetURLRequestAdapter::AddRequestHeader(const std::string& name,
   initial_request_headers_.SetHeader(name, value);
 }
 
+void CronetURLRequestAdapter::DisableCache() {
+  DCHECK(!IsOnNetworkThread());
+  load_flags_ |= net::LOAD_DISABLE_CACHE;
+}
+
 bool CronetURLRequestAdapter::PostTaskToNetworkThread(
     const tracked_objects::Location& from_here,
     const base::Closure& task) {
@@ -59,7 +65,7 @@ void CronetURLRequestAdapter::Start() {
           << " priority: " << RequestPriorityToString(initial_priority_);
   url_request_ = context_->GetURLRequestContext()->CreateRequest(
       initial_url_, net::DEFAULT_PRIORITY, this, NULL);
-  url_request_->SetLoadFlags(context_->default_load_flags());
+  url_request_->SetLoadFlags(load_flags_);
   url_request_->set_method(initial_method_);
   url_request_->SetExtraRequestHeaders(initial_request_headers_);
   url_request_->SetPriority(initial_priority_);
diff --git a/components/cronet/android/cronet_url_request_adapter.h b/components/cronet/android/cronet_url_request_adapter.h
index 6699808..293f37c 100644
--- a/components/cronet/android/cronet_url_request_adapter.h
+++ b/components/cronet/android/cronet_url_request_adapter.h
@@ -121,6 +121,10 @@ class CronetURLRequestAdapter : public net::URLRequest::Delegate {
   // prior to decompression.
   int64 GetTotalReceivedBytes() const;
 
+  // Bypasses cache. If context is not set up to use cache, this call has no
+  // effect.
+  void DisableCache();
+
   // net::URLRequest::Delegate overrides.
   void OnReceivedRedirect(net::URLRequest* request,
                           const net::RedirectInfo& redirect_info,
@@ -139,6 +143,7 @@ class CronetURLRequestAdapter : public net::URLRequest::Delegate {
   const GURL initial_url_;
   const net::RequestPriority initial_priority_;
   std::string initial_method_;
+  int load_flags_;
   net::HttpRequestHeaders initial_request_headers_;
 
   scoped_refptr<net::IOBufferWithSize> read_buffer_;
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 835bb28..1a085b8 100644
--- a/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
+++ b/components/cronet/android/java/src/org/chromium/net/CronetUrlRequest.java
@@ -34,6 +34,7 @@ final class CronetUrlRequest implements UrlRequest {
     private boolean mStarted = false;
     private boolean mCanceled = false;
     private boolean mInOnDataReceived = false;
+    private boolean mDisableCache = false;
 
     /*
      * Synchronize access to mUrlRequestAdapter, mStarted, mCanceled and
@@ -280,6 +281,9 @@ final class CronetUrlRequest implements UrlRequest {
                             + header.first + "=" + header.second);
                 }
             }
+            if (mDisableCache) {
+                nativeDisableCache(mUrlRequestAdapter);
+            }
             mStarted = true;
             nativeStart(mUrlRequestAdapter);
         }
@@ -322,6 +326,12 @@ final class CronetUrlRequest implements UrlRequest {
         throw new UnsupportedOperationException("Not implemented yet");
     }
 
+    @Override
+    public void disableCache() {
+        checkNotStarted();
+        mDisableCache = true;
+    }
+
     /**
      * Post task to application Executor. Used for Listener callbacks
      * and other tasks that should not be executed on network thread.
@@ -620,4 +630,6 @@ final class CronetUrlRequest implements UrlRequest {
     private native boolean nativeGetWasCached(long urlRequestAdapter);
 
     private native long nativeGetTotalReceivedBytes(long urlRequestAdapter);
+
+    private native void nativeDisableCache(long urlRequestAdapter);
 }
diff --git a/components/cronet/android/java/src/org/chromium/net/UrlRequest.java b/components/cronet/android/java/src/org/chromium/net/UrlRequest.java
index b78dd63..54b72a1 100644
--- a/components/cronet/android/java/src/org/chromium/net/UrlRequest.java
+++ b/components/cronet/android/java/src/org/chromium/net/UrlRequest.java
@@ -91,6 +91,12 @@ public interface UrlRequest {
     public void resume();
 
     /**
+     * Disables cache for the request. If context is not set up to use cache,
+     * this call has no effect.
+     */
+    public void disableCache();
+
+    /**
      * Note:  There are deliberately no accessors for the results of the request
      * here.  Having none removes any ambiguity over when they are populated,
      * particularly in the redirect case.
diff --git a/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java b/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java
index 05262a6..585b6e6 100644
--- a/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java
+++ b/components/cronet/android/java/src/org/chromium/net/urlconnection/CronetHttpURLConnection.java
@@ -71,6 +71,9 @@ public class CronetHttpURLConnection extends HttpURLConnection {
         for (Pair<String, String> requestHeader : mRequestHeaders) {
             mRequest.addHeader(requestHeader.first, requestHeader.second);
         }
+        if (!getUseCaches()) {
+            mRequest.disableCache();
+        }
         mRequest.start();
         // Blocks until onResponseStarted or onFailed is called.
         mMessageLoop.loop();
diff --git a/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/CronetUrlRequestContextTest.java b/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/CronetUrlRequestContextTest.java
index f907d91..8e1bd54 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/CronetUrlRequestContextTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/CronetUrlRequestContextTest.java
@@ -297,9 +297,17 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
     }
 
     private void checkRequestCaching(String url, boolean expectCached) {
+        checkRequestCaching(url, expectCached, false);
+    }
+
+    private void checkRequestCaching(String url, boolean expectCached,
+            boolean disableCache) {
         TestUrlRequestListener listener = new TestUrlRequestListener();
         UrlRequest urlRequest = mActivity.mUrlRequestContext.createRequest(
                 url, listener, listener.getExecutor());
+        if (disableCache) {
+            urlRequest.disableCache();
+        }
         urlRequest.start();
         listener.blockForDone();
         assertEquals(expectCached, listener.mResponseInfo.wasCached());
@@ -348,6 +356,36 @@ public class CronetUrlRequestContextTest extends CronetTestBase {
         checkRequestCaching(url, false);
     }
 
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testDisableCache() throws Exception {
+        enableCache(UrlRequestContextConfig.HttpCache.DISK);
+        String url = NativeTestServer.getFileURL("/cacheable.txt");
+
+        // When cache is disabled, making a request does not write to the cache.
+        checkRequestCaching(url, false, true /** disable cache */);
+        checkRequestCaching(url, false);
+
+        // When cache is enabled, the second request is cached.
+        checkRequestCaching(url, false, true /** disable cache */);
+        checkRequestCaching(url, true);
+
+        // Shut down the server, next request should have a cached response.
+        NativeTestServer.shutdownNativeTestServer();
+        checkRequestCaching(url, true);
+
+        // Cache is disabled after server is shut down, request should fail.
+        TestUrlRequestListener listener = new TestUrlRequestListener();
+        UrlRequest urlRequest = mActivity.mUrlRequestContext.createRequest(
+                url, listener, listener.getExecutor());
+        urlRequest.disableCache();
+        urlRequest.start();
+        listener.blockForDone();
+        assertNotNull(listener.mError);
+        assertEquals("Exception in CronetUrlRequest: net::ERR_CONNECTION_REFUSED",
+                listener.mError.getMessage());
+    }
+
     // TODO(mef): Simple cache uses global thread pool that is not affected by
     // shutdown of UrlRequestContext. This test can be flaky unless that thread
     // pool is destroyed and recreated. Enable the test when crbug.com/442321 is fixed.
diff --git a/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/urlconnection/CronetHttpURLConnectionTest.java b/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/urlconnection/CronetHttpURLConnectionTest.java
index 8ef1ed0..5e436b2 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/urlconnection/CronetHttpURLConnectionTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/urlconnection/CronetHttpURLConnectionTest.java
@@ -11,6 +11,7 @@ import org.chromium.base.test.util.Feature;
 import org.chromium.cronet_test_apk.CronetTestActivity;
 import org.chromium.cronet_test_apk.CronetTestBase;
 import org.chromium.cronet_test_apk.NativeTestServer;
+import org.chromium.net.UrlRequestContextConfig;
 import org.chromium.net.UrlRequestException;
 
 import java.io.ByteArrayOutputStream;
@@ -41,7 +42,14 @@ public class CronetHttpURLConnectionTest extends CronetTestBase {
     @Override
     protected void setUp() throws Exception {
         super.setUp();
-        mActivity = launchCronetTestApp();
+        UrlRequestContextConfig config = new UrlRequestContextConfig();
+        config.setStoragePath(prepareTestStorage());
+        config.enableHttpCache(UrlRequestContextConfig.HttpCache.DISK,
+                1000 * 1024);
+        String[] commandLineArgs = {
+                CronetTestActivity.CONFIG_KEY, config.toString()};
+        mActivity = launchCronetTestAppWithUrlAndCommandLineArgs(null,
+                commandLineArgs);
         assertTrue(NativeTestServer.startNativeTestServer(
                 getInstrumentation().getTargetContext()));
     }
@@ -711,6 +719,68 @@ public class CronetHttpURLConnectionTest extends CronetTestBase {
         connection.disconnect();
     }
 
+    private static enum CacheSetting { USE_CACHE, DONT_USE_CACHE };
+
+    private static enum ExpectedOutcome { SUCCESS, FAILURE };
+
+    /**
+     * Helper method to make a request with cache enabled or disabled, and check
+     * whether the request is successful.
+     * @param requestUrl request url.
+     * @param cacheSetting indicates cache should be used.
+     * @param outcome indicates request is expected to be successful.
+     */
+    private void checkRequestCaching(String requestUrl,
+            CacheSetting cacheSetting,
+            ExpectedOutcome outcome) throws Exception {
+        URL url = new URL(requestUrl);
+        HttpURLConnection connection =
+                (HttpURLConnection) url.openConnection();
+        connection.setUseCaches(cacheSetting == CacheSetting.USE_CACHE);
+        if (outcome == ExpectedOutcome.SUCCESS) {
+            assertEquals(200, connection.getResponseCode());
+            assertEquals("this is a cacheable file\n",
+                    getResponseAsString(connection));
+        } else {
+            try {
+                connection.getResponseCode();
+                fail();
+            } catch (IOException e) {
+                // Expected.
+            }
+        }
+        connection.disconnect();
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    @OnlyRunCronetHttpURLConnection
+    // Strangely, the default implementation fails to return a cached response.
+    // If the server is shut down, the request just fails with a connection
+    // refused error. Therefore, this test and the next only runs Cronet.
+    public void testSetUseCaches() throws Exception {
+        String url = NativeTestServer.getFileURL("/cacheable.txt");
+        checkRequestCaching(url,
+                CacheSetting.USE_CACHE, ExpectedOutcome.SUCCESS);
+        // Shut down the server, we should be able to receive a cached response.
+        NativeTestServer.shutdownNativeTestServer();
+        checkRequestCaching(url,
+                CacheSetting.USE_CACHE, ExpectedOutcome.SUCCESS);
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    @OnlyRunCronetHttpURLConnection
+    public void testSetUseCachesFalse() throws Exception {
+        String url = NativeTestServer.getFileURL("/cacheable.txt");
+        checkRequestCaching(url,
+                CacheSetting.USE_CACHE, ExpectedOutcome.SUCCESS);
+        NativeTestServer.shutdownNativeTestServer();
+        // Disables caching. No cached response is received.
+        checkRequestCaching(url,
+                CacheSetting.DONT_USE_CACHE, ExpectedOutcome.FAILURE);
+    }
+
     private void checkExceptionsAreThrown(HttpURLConnection connection)
             throws Exception {
         try {
diff --git a/components/cronet/android/test/src/org/chromium/cronet_test_apk/CronetTestActivity.java b/components/cronet/android/test/src/org/chromium/cronet_test_apk/CronetTestActivity.java
index 34ce145..df8e6df 100644
--- a/components/cronet/android/test/src/org/chromium/cronet_test_apk/CronetTestActivity.java
+++ b/components/cronet/android/test/src/org/chromium/cronet_test_apk/CronetTestActivity.java
@@ -79,9 +79,6 @@ public class CronetTestActivity extends Activity {
             return;
         }
         mRequestFactory = initRequestFactory();
-        mStreamHandlerFactory = new CronetURLStreamHandlerFactory(
-                getApplicationContext(), null);
-
         if (mRequestFactory == null) {
             return;
         }
@@ -121,6 +118,9 @@ public class CronetTestActivity extends Activity {
 
         mHistogramManager = HistogramManager.createHistogramManager();
 
+        mStreamHandlerFactory = new CronetURLStreamHandlerFactory(
+                getApplicationContext(), config);
+
         return HttpUrlRequestFactory.createFactory(getApplicationContext(),
                                                    config);
     }