Prevent URLFetcher::AppendChunkedData from dereferencing NULL pointers.

net/url_request/url_fetcher_core.cc

Index: net/url_request/url_fetcher_core.cc
diff --git a/net/url_request/url_fetcher_core.cc b/net/url_request/url_fetcher_core.cc
index ee1a3d8c83010938d4c6d10cedc3e69fd348384b..3b1e1cc55d00225a15cfba03cfb7bf1590719df7 100644
--- a/net/url_request/url_fetcher_core.cc
+++ b/net/url_request/url_fetcher_core.cc
@@ -205,6 +205,7 @@ void URLFetcherCore::AppendChunkToUpload(const std::string& content,
                                          bool is_last_chunk) {
   DCHECK(delegate_task_runner_.get());
   DCHECK(network_task_runner_.get());
+  DCHECK(is_chunked_upload_);
   network_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&URLFetcherCore::CompleteAddingUploadDataChunk, this, content,
@@ -516,6 +517,14 @@ URLFetcherCore::~URLFetcherCore() {
 void URLFetcherCore::StartOnIOThread() {
   DCHECK(network_task_runner_->BelongsToCurrentThread());
 
+  // Create ChunkedUploadDataStream, if needed, so the consumer can start
+  // appending data.  Have to do it here because StartURLRequest() may be called
+  // asynchonously.
+  if (is_chunked_upload_) {

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

What is the gap between when this runs, and when StartURLRequest() runs?

Does this actually need to be set up here, will users be able to append data to
chunked_stream_ before it is moved to the URLRequest ?

This seems weird in that ChunkedUploadDataStream() is itself just a buffer that
queues up the data. So we have a writer interface to ChunkedUploadDataStream,
and then URLRequest pulls data from that.

I will have to dig deeper into how we do the upload abstractions to comment
better on this, although your feedback will speed that up :)

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

There are two gaps:  The ResponseWriter may take a while to initialize (Open +
create file, or whatever), and in DidInitializeWriter ->
StartURLRequestWhenAppropriate, we may delay the request based on the
ThrottleManager (A class I would love to kill).

We could move URLRequest creation up here, and have the URLRequest exist for the
duration of both those things.  I'm fine with doing that refactor in this CL.

+    chunked_stream_.reset(new ChunkedUploadDataStream(0));
+    chunked_stream_writer_ = chunked_stream_->CreateWriter();
+  }
+
   if (!response_writer_)
     response_writer_.reset(new URLFetcherStringWriter);
 
@@ -550,8 +559,11 @@ void URLFetcherCore::StartURLRequest() {
   request_->set_stack_trace(stack_trace_);
   int flags = request_->load_flags() | load_flags_;
 
-  if (is_chunked_upload_)
-    request_->EnableChunkedUpload();
+  // TODO(mmenke): This should really be with the other code to set the upload
+  // body, below.
+  if (chunked_stream_)
+    request_->set_upload(std::move(chunked_stream_));
+
   request_->SetLoadFlags(flags);
   request_->SetReferrer(referrer_);
   request_->set_referrer_policy(referrer_policy_);
@@ -835,17 +847,10 @@ base::TimeTicks URLFetcherCore::GetBackoffReleaseTime() {
 
 void URLFetcherCore::CompleteAddingUploadDataChunk(
     const std::string& content, bool is_last_chunk) {
-  if (was_cancelled_) {
-    // Since CompleteAddingUploadDataChunk() is posted as a *delayed* task, it
-    // may run after the URLFetcher was already stopped.
-    return;
-  }
   DCHECK(is_chunked_upload_);
-  DCHECK(request_.get());
   DCHECK(!content.empty());
-  request_->AppendChunkToUpload(content.data(),
-                                static_cast<int>(content.length()),
-                                is_last_chunk);
+  chunked_stream_writer_->AppendData(
+      content.data(), static_cast<int>(content.length()), is_last_chunk);
 }
 
 int URLFetcherCore::WriteBuffer(scoped_refptr<DrainableIOBuffer> data) {