Provide a safer URLLoader ReadResponseBody API

ppapi/cpp/url_loader.h

Index: ppapi/cpp/url_loader.h
diff --git a/ppapi/cpp/url_loader.h b/ppapi/cpp/url_loader.h
index 3ed5fe6e9255109ce323ef70b8a9bcce46f83846..a1a2deec96c8142debdf67ab682f7d3331aec10c 100644
--- a/ppapi/cpp/url_loader.h
+++ b/ppapi/cpp/url_loader.h
@@ -5,7 +5,10 @@
 #ifndef PPAPI_CPP_URL_LOADER_H_
 #define PPAPI_CPP_URL_LOADER_H_
 
+#include <vector>
+
 #include "ppapi/c/pp_stdint.h"
+#include "ppapi/cpp/completion_callback.h"
 #include "ppapi/cpp/resource.h"
 
 /// @file
@@ -124,6 +127,19 @@ class URLLoader : public Resource {
   /// must be large enough to hold the specified number of bytes to read.
   /// This function might perform a partial read.
   ///
+  /// <strong>Caveat:</strong> This ReadResponseBody() is potentially unsafe if
+  /// you're using a CompletionCallbackFactory to scope callbacks to the
+  /// lifetime of your class.  When your class goes out of scope, the callback
+  /// factory will not actually cancel the callback, but will rather just skip
+  /// issuing the callback on your class.  This means that if the UrlLoader
+  /// object outlives your class (if you made a copy saved somewhere else, for
+  /// example), then the browser will still try to write into your buffer when
+  /// the asynchronous read completes, potentially causing a crash.
+  ///
+  /// See the other version of ReadResponseBody() which avoids this problem by
+  /// writing into CompletionCallbackWithOutput, where the output buffer is
+  /// automatically managed by the callback.
+  ///
   /// @param[in,out] buffer A pointer to the buffer for the response body.
   /// @param[in] bytes_to_read The number of bytes to read.
   /// @param[in] cc A <code>CompletionCallback</code> to run on asynchronous
@@ -137,6 +153,23 @@ class URLLoader : public Resource {
                            int32_t bytes_to_read,
                            const CompletionCallback& cc);
 
+  /// This function is used to read the responsed body. A PP_ArrayOutput must be
+  /// provided so that output will be stored in its allocated buffer.  This
+  /// function might perform a partial read.
+  ///
+  /// @param[in] max_read_length The number of bytes to read.
+  /// @param[in] cc A <code>CompletionCallbackWithOutput</code> to hold a
+  /// PP_ArrayOutput for output, and run on asynchronous CompletionCallback.
+  /// ReadResponseBody(). The callback will run if the bytes (full or partial)
+  /// are read or an error occurs asynchronously. This callback will run only if
+  /// this function returns <code>PP_OK_COMPLETIONPENDING</code>.
+  ///
+  /// @return An int32_t containing the number of bytes read or an error code
+  /// from <code>pp_errors.h</code>.
+  int32_t ReadResponseBody(
+      int32_t max_read_length,
+      const CompletionCallbackWithOutput< std::vector<char> >& cc);
+
   /// This function is used to wait for the response body to be completely
   /// downloaded to the file provided by the GetBodyAsFileRef() in the current
   /// <code>URLResponseInfo</code>. This function is only used if
@@ -161,6 +194,21 @@ class URLLoader : public Resource {
   /// while it is still open, then it will be implicitly closed so you are not
   /// required to call Close().
   void Close();
+
+ private:
+  struct CallbackData1_0 {
+    PP_ArrayOutput output;
+    char* temp_buffer;
+    PP_CompletionCallback original_callback;
+  };
+
+  // Provide backwards-compatability for older ReadResponseBody versions.
+  // Converts the old-style "char*" output buffer of 1.0 to the new
+  // "PP_ArrayOutput" interface in 1.1.
+  //
+  // This takes a heap-allocated CallbackData1_0 struct passed as the user data
+  // and deletes it when the call completes.
+  static void CallbackConverter(void* user_data, int32_t result);
 };
 
 }  // namespace pp