[Cronet] Fix two races/leaks in JavaUrlRequest

components/cronet/android/test/javatests/src/org/chromium/net/UploadDataProvidersTest.java

Index: components/cronet/android/test/javatests/src/org/chromium/net/UploadDataProvidersTest.java
diff --git a/components/cronet/android/test/javatests/src/org/chromium/net/UploadDataProvidersTest.java b/components/cronet/android/test/javatests/src/org/chromium/net/UploadDataProvidersTest.java
index 4e1865296c067f6434fdf75f54f9ee98a2f27ce9..a116153dc44d3f51e8694104f537c8ac3926aef8 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/net/UploadDataProvidersTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/net/UploadDataProvidersTest.java
@@ -229,4 +229,26 @@ public class UploadDataProvidersTest extends CronetTestBase {
         assertEquals(200, callback.mResponseInfo.getHttpStatusCode());
         assertEquals(LOREM, callback.mResponseAsString);
     }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    /**
+     * Open many connections and cancel them right away. This test verifies all internal
+     * sockets and other Closeables are properly closed. See crbug.com/726193. This isn't
+     * really a test of UploadDataProviders but is in this class to piggyback on the
+     * StrictMode enforcement it offers.

[Charles, 2017/05/26 13:58:48]

Should we add that same strictmode checking to CronetTestBase?

[pauljensen, 2017/05/26 18:36:03]

Done.

+     */
+    public void testGzipCancel() throws Exception {
+        String url = NativeTestServer.getFileURL("/gzipped.html");
+        for (int i = 0; i < 100; i++) {
+            TestUrlRequestCallback callback = new TestUrlRequestCallback();
+            callback.setAutoAdvance(false);
+            UrlRequest urlRequest =
+                    mTestFramework.mCronetEngine
+                            .newUrlRequestBuilder(url, callback, callback.getExecutor())
+                            .build();
+            urlRequest.start();
+            urlRequest.cancel();
+        }

[Charles, 2017/05/26 13:58:48]

Shouldn't you block on all these operations completing before letting the test
finish?

[pauljensen, 2017/05/26 18:36:03]

If I block until each UrlRequest is done before starting the next then I never
catch the leak.  If I start them all and then wait for them all to finish, I
only catch the leak ~10% of the time.  In its current form I catch the leak ~70%
of the time.

I think the leak requires a lot of busy threads so that the cancel() happens
before the UrlRequest has made much progress (and set mCurrentUrlConnection and
mResponseChannel).  So this explains why the waiting for each one doesn't catch
the leak.

The other quirk of this is that from teardown() we call
JavaCronetEngine.shutdown() which calls ExecutorService.shutdown() which doesn't
wait for the thread to finish running tasks, and then we call GC looking for
leaks.  We could expose the ExecutorService and then have tests call
awaitTermination() but I think the simple test in its current form is probably
good enough.

+    }
 }