Cronet UrlRequest onAppendChunk now checks that request is not destroyed.

BUG=434855
TEST=cr_cronet.py test -f *testAppendChunk*

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

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

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

@@ -7,14 +7,18 @@ 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;
+import java.util.concurrent.Executors;
 
 /**
  * Test fixture to test upload APIs.  Uses an in-process test server.
@@ -234,4 +238,63 @@ public class UploadTest extends CronetTestBase {
             assertEquals(contentType, listener.mResponseAsString);
         }
     }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testAppendChunkRaceWithCancel() throws Exception {
+        ByteBuffer byteBuffer = ByteBuffer.allocateDirect(1024);
+        byteBuffer.put(UPLOAD_DATA.getBytes());
+        byteBuffer.position(0);
+
+        // Try to recreate race described in crbug.com/434855 when request
+        // is canceled from another thread while adding chunks to upload.
+        for (int test = 0; test < 100; ++test) {
+            TestHttpUrlRequestListener listener =
+                    new TestHttpUrlRequestListener();
+            final ChromiumUrlRequest request =
+                    (ChromiumUrlRequest) createRequest("http://127.0.0.1:8000",
+                                                       listener);
+            request.setChunkedUpload("dangerous/crocodile");
+            request.start();
+            Runnable cancelTask = new Runnable() {
+                public void run() {
+                    request.cancel();
+                }
+            };
+            Executors.newCachedThreadPool().execute(cancelTask);
+            request.appendChunk(byteBuffer, false);
+            request.appendChunk(byteBuffer, false);
+            request.appendChunk(byteBuffer, false);
+            request.appendChunk(byteBuffer, true);
+            listener.blockForComplete();
+        }
+    }
+
+    @SmallTest
+    @Feature({"Cronet"})
+    public void testAppendChunkPreAndPost() throws Exception {
+        ByteBuffer byteBuffer = ByteBuffer.allocateDirect(1024);
+        byteBuffer.put(UPLOAD_DATA.getBytes());
+        byteBuffer.position(0);
+
+        TestHttpUrlRequestListener listener = new TestHttpUrlRequestListener();
+        ChromiumUrlRequest request = (ChromiumUrlRequest) createRequest(
+                UploadTestServer.getEchoBodyURL(), 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, true);
+        listener.blockForComplete();
+        try {
+            request.appendChunk(byteBuffer, true);
+            fail("Exception not thrown.");
+        } catch (IOException e) {
+            assertEquals("Native peer destroyed.", e.getMessage());
+        }
+    }
 }

components/cronet/android/url_request_adapter.cc

@@ -126,6 +126,13 @@ void URLRequestAdapter::Start() {
 void URLRequestAdapter::OnAppendChunk(const scoped_ptr<char[]> bytes,
                                       int bytes_len, bool is_last_chunk) {
   DCHECK(OnNetworkThread());
+  // Request could have completed and been destroyed on the network thread
+  // while appendChunk was posting the task from an application thread.
+  if (!url_request_) {
+    VLOG(1) << "Cannot append chunk to destroyed request: "
+            << url_.possibly_invalid_spec().c_str();
+    return;
+  }
   url_request_->AppendChunkToUpload(bytes.get(), bytes_len, is_last_chunk);
 }