Cronet UrlRequest onAppendChunk now checks that request is not destroyed.

components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/UploadTest.java

Index: components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/UploadTest.java
diff --git a/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/UploadTest.java b/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/UploadTest.java
index 959aac222e438d6089dd7b113bd4ac448317bfb3..7547178055c9aa47cdfccba51ee2a5613e0640fe 100644
--- a/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/UploadTest.java
+++ b/components/cronet/android/test/javatests/src/org/chromium/cronet_test_apk/UploadTest.java
@@ -7,11 +7,14 @@ package org.chromium.cronet_test_apk;
 import android.test.suitebuilder.annotation.SmallTest;
 
 import org.chromium.base.test.util.Feature;
+import org.chromium.net.ChromiumUrlRequest;
 import org.chromium.net.HttpUrlRequest;
 import org.chromium.net.HttpUrlRequestListener;
 
 import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.io.InputStream;
+import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.nio.channels.ReadableByteChannel;
 import java.util.HashMap;
@@ -22,6 +25,8 @@ import java.util.HashMap;
 public class UploadTest extends CronetTestBase {
     private static final String UPLOAD_DATA = "Nifty upload data!";
     private static final String UPLOAD_CHANNEL_DATA = "Upload channel data";
+    // URL used for base tests.
+    private static final String URL = "http://127.0.0.1:8000";
 
     private CronetTestActivity mActivity;
 
@@ -234,4 +239,37 @@ public class UploadTest extends CronetTestBase {
             assertEquals(contentType, listener.mResponseAsString);
         }
     }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testAppendChunk() throws Exception {
+        ByteBuffer byteBuffer = ByteBuffer.allocateDirect(1024);
+        byteBuffer.put(UPLOAD_DATA.getBytes());
+        byteBuffer.position(0);
+
+        TestHttpUrlRequestListener listener = new TestHttpUrlRequestListener();
+        ChromiumUrlRequest request = (ChromiumUrlRequest) createRequest(
+                URL, listener);
+        request.setChunkedUpload("dangerous/crocodile");
+        try {
+            request.appendChunk(byteBuffer, false);
+            fail("Exception not thrown.");
+        } catch (IllegalStateException e) {
+            assertEquals("Request not yet started.", e.getMessage());
+        }
+        request.start();
+        request.appendChunk(byteBuffer, false);
+        request.appendChunk(byteBuffer, false);
+        request.appendChunk(byteBuffer, false);
+        request.appendChunk(byteBuffer, true);
+        listener.blockForComplete();

[mmenke, 2014/11/19 18:50:51]

This test doesn't fail without the patch, does it?

[mef, 2014/11/19 19:32:36]

It failed inconsistently when I had appendChunk in a loop because request was
getting completed on network thread while java was posting chunks on java
thread. I've removed the loop to prevent slooow tests.

[mmenke, 2014/11/19 19:37:25]

Why would it fail?  The request only finished after we've uploaded the entire
body, so the last request.appendChunk above this line will be executed before
the request completes.

We have a lock when ChromiumURLRequest calls into the listener and deletes the
adapter, so the appendChunk below this line should always end up seeing the
deleted adapter.  I just can't see how this test would trigger the issue.

+        try {
+            request.appendChunk(byteBuffer, true);
+            fail("Exception not thrown.");
+        } catch (IOException e) {
+            assertEquals("Native peer destroyed.", e.getMessage());
+        }
+        assertEquals(411, listener.mHttpStatusCode);
+    }
+

[mmenke, 2014/11/19 18:50:51]

nit:  Remove extra blank line.

[mef, 2014/11/19 19:32:36]

Done.

 }