Prevent URLFetcher::AppendChunkedData from dereferencing NULL pointers.

components/cronet/android/url_request_adapter.cc

Index: components/cronet/android/url_request_adapter.cc
diff --git a/components/cronet/android/url_request_adapter.cc b/components/cronet/android/url_request_adapter.cc
index d6576374ba2a3dd62a6cff3aae2c799ba678d4b4..acfb00888eaefc8e0ee2c81aec539dd7ab0ed566 100644
--- a/components/cronet/android/url_request_adapter.cc
+++ b/components/cronet/android/url_request_adapter.cc
@@ -131,12 +131,12 @@ void URLRequestAdapter::OnAppendChunk(const scoped_ptr<char[]> bytes,
   DCHECK(OnNetworkThread());
   // Request could have completed and been destroyed on the network thread
   // while appendChunk was posting the task from an application thread.

[eroman, 2016/03/22 19:05:37]

This comment and subsequent VLOG() don't seem congruent with the new condition.

[mmenke, 2016/03/22 20:01:50]

Comment seems correct - you can actually succeed without completing the upload
(With, for instance, a 413 - we check for a response after the connection has
been closed for just that case), but it's uncommon.

I've restated the comment, and removed the VLOG - I don't think it's that
useful.

-  if (!url_request_) {
+  if (!chunked_upload_writer_->AppendData(bytes.get(), bytes_len,
+                                          is_last_chunk)) {
     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);
 }
 
 void URLRequestAdapter::OnInitiateConnection() {
@@ -166,7 +166,10 @@ void URLRequestAdapter::OnInitiateConnection() {
   if (upload_data_stream_) {
     url_request_->set_upload(std::move(upload_data_stream_));
   } else if (chunked_upload_) {
-    url_request_->EnableChunkedUpload();
+    scoped_ptr<net::ChunkedUploadDataStream> chunked_upload_data_stream(
+        new net::ChunkedUploadDataStream(0));
+    chunked_upload_writer_ = chunked_upload_data_stream->CreateWriter();
+    url_request_->set_upload(std::move(chunked_upload_data_stream));

[eroman, 2016/03/22 19:05:37]

This looked wrong when first reading it because of the apparent reference cycle.

I figured the writer would be holding a weak ptr, but worth adding a comment
IMO.

[mmenke, 2016/03/22 20:01:50]

Done.

   }
 
   url_request_->SetPriority(priority_);