Prevent URLFetcher::AppendChunkedData from dereferencing NULL pointers.

net/base/chunked_upload_data_stream.h

Index: net/base/chunked_upload_data_stream.h
diff --git a/net/base/chunked_upload_data_stream.h b/net/base/chunked_upload_data_stream.h
index 7b5e2dfecb618393d8a8bc50f05f4739cde5ea08..19ac0a6fe86f3990004e2dc2080771cb787b9154 100644
--- a/net/base/chunked_upload_data_stream.h
+++ b/net/base/chunked_upload_data_stream.h
@@ -12,6 +12,8 @@
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "net/base/completion_callback.h"
 #include "net/base/net_export.h"
 #include "net/base/upload_data_stream.h"
@@ -25,14 +27,53 @@ class IOBuffer;
 // seekable data, due to this buffering behavior.
 class NET_EXPORT ChunkedUploadDataStream : public UploadDataStream {
  public:
+  // Utility class that allows writing data to a particular
+  // ChunkedUploadDataStream. It's needed because URLRequest owns the

[eroman, 2016/03/26 00:55:16]

Before jumping into the "why" it's needed, I suggest mentioning explicitly the
mechanism it supports -- that the Writer can outlive the ChunkedUploadDataStream
/ URLRequest that it is associated with.

This is implied from the following comment, but I think t would be useful to see
it stated explicitly either here, or in the description of CreateWriter().

[mmenke, 2016/03/28 17:33:11]

Done.

+  // ChunkedUploadDataStream and manages its lifetime (And can delete it without
+  // warning, if failures are intercepted and then redirected), but higher level
+  // code is responsible for writing to the ChunkedUploadDataStream.
+  //
+  // The writer may only be used on the ChunkedUploadDataStream's thread.
+  class NET_EXPORT Writer {
+   public:
+    ~Writer();
+
+    // Adds data to the stream. |is_done| should be true if this is the last
+    // data to be appended. |data_len| must not be 0 unless |is_done| is true.
+    // Once called with |is_done| being true, must never be called again.
+    // Returns true if write was passed successfully on to the next layer,
+    // though the data may not actually have been written to the underlying
+    // URLRequest.  Returns false if unable to write the data failed because the
+    // underlying ChunkedUploadDataStream was destroyed.
+    bool AppendData(const char* data, int data_len, bool is_done);
+
+   private:
+    friend class ChunkedUploadDataStream;
+
+    explicit Writer(base::WeakPtr<ChunkedUploadDataStream> upload_data_stream);
+
+    const base::WeakPtr<ChunkedUploadDataStream> upload_data_stream_;
+
+    DISALLOW_COPY_AND_ASSIGN(Writer);
+  };
+
   explicit ChunkedUploadDataStream(int64_t identifier);
 
   ~ChunkedUploadDataStream() override;
 
+  // Creates a Writer for appending data to |this|.  It's generally expected
+  // that only one writer is created per stream, though multiple writers are
+  // allowed.  All writers write to the same stream, and once one of them
+  // appends data with |is_done| being true, no other writers may be used to
+  // append data.
+  scoped_ptr<Writer> CreateWriter();
+
   // Adds data to the stream. |is_done| should be true if this is the last
   // data to be appended. |data_len| must not be 0 unless |is_done| is true.
   // Once called with |is_done| being true, must never be called again.
   // TODO(mmenke):  Consider using IOBuffers instead, to reduce data copies.
+  // TODO(mmenke):  Consider making private, and having all consumers use
+  //     Writers.
   void AppendData(const char* data, int data_len, bool is_done);
 
  private:
@@ -57,6 +98,8 @@ class NET_EXPORT ChunkedUploadDataStream : public UploadDataStream {
   scoped_refptr<IOBuffer> read_buffer_;
   int read_buffer_len_;
 
+  base::WeakPtrFactory<ChunkedUploadDataStream> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ChunkedUploadDataStream);
 };