net: Move ownership of UploadDataStream from URLRequestHttpJob to URLRequest

net/url_request/url_request.cc

Index: net/url_request/url_request.cc
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index 7bfa77239054f02df0fa4003a2f0d2c61c8a4f5f..8d1c8cd5c170b57b7720d1e59a897a8658d0cde2 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -24,6 +24,7 @@
 #include "net/base/network_delegate.h"
 #include "net/base/ssl_cert_request_info.h"
 #include "net/base/upload_data.h"
+#include "net/base/upload_data_stream.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_util.h"
 #include "net/url_request/url_request_context.h"
@@ -253,18 +254,12 @@ void URLRequest::UnregisterRequestInterceptor(Interceptor* interceptor) {
       interceptor);
 }
 
-void URLRequest::AppendBytesToUpload(const char* bytes, int bytes_len) {
-  DCHECK(bytes_len > 0 && bytes);
-  if (!upload_)
-    upload_ = new UploadData();
-  upload_->AppendBytes(bytes, bytes_len);
-}
-
 void URLRequest::EnableChunkedUpload() {
   DCHECK(!upload_ || upload_->is_chunked());
   if (!upload_) {
     upload_ = new UploadData();
     upload_->set_is_chunked(true);
+    upload_data_stream_.reset(new UploadDataStream(upload_));
   }
 }
 
@@ -279,19 +274,15 @@ void URLRequest::AppendChunkToUpload(const char* bytes,
 
 void URLRequest::set_upload(UploadData* upload) {
   upload_ = upload;
+  upload_data_stream_.reset(new UploadDataStream(upload_));
 }
 
-// Get the upload data directly.
-const UploadData* URLRequest::get_upload() const {
-  return upload_.get();
-}
-
-UploadData* URLRequest::get_upload_mutable() {
-  return upload_.get();
+const UploadDataStream* URLRequest::get_upload() const {
+  return upload_data_stream_.get();
 }
 
 bool URLRequest::has_upload() const {
-  return upload_ != NULL;
+  return upload_data_stream_.get() != NULL;
 }
 
 void URLRequest::SetExtraRequestHeaderById(int id, const string& value,
@@ -530,8 +521,8 @@ void URLRequest::StartJob(URLRequestJob* job) {
   job_ = job;
   job_->SetExtraRequestHeaders(extra_request_headers_);
 
-  if (upload_.get())
-    job_->SetUpload(upload_.get());
+  if (upload_data_stream_.get())
+    job_->SetUpload(upload_data_stream_.get());
 
   is_pending_ = true;
   is_redirecting_ = false;
@@ -782,10 +773,11 @@ int URLRequest::Redirect(const GURL& location, int http_status_code) {
   // so shall we.
   // See:  https://tools.ietf.org/html/draft-ietf-httpbis-p2-semantics-17#section-7.3
   bool was_post = method_ == "POST";
+  bool discard_upload = false;
   if ((http_status_code == 303 && method_ != "HEAD") ||
       ((http_status_code == 301 || http_status_code == 302) && was_post)) {
     method_ = "GET";
-    upload_ = NULL;
+    discard_upload = true;
     if (was_post) {
       // If being switched from POST to GET, must remove headers that were
       // specific to the POST and don't have meaning in GET. For example
@@ -810,6 +802,8 @@ int URLRequest::Redirect(const GURL& location, int http_status_code) {
     final_upload_progress_ = job_->GetUploadProgress();

[mmenke, 2012/11/20 19:57:03]

Can't we just move this up to before the block of code that rewrites the method,
and get rid of discard_upload?

If I'm missing something, think we should at least have a comment for the reason
discard_upload is needed.

[hashimoto, 2012/11/21 03:41:02]

Moved GetUploadProgress() and PrepareToRestart() to before the method rewriting
block.

PrepareToRestart() is also moved because, from URLRequestJob's point of view,
UploadDataStream should be alive at least until URLRequestJob::DetachRequest(),
which is called by URLRequest::OrphanJob(), is called.

 
   PrepareToRestart();
+  if (discard_upload)
+    upload_data_stream_.reset();
   Start();
   return OK;
 }