net: Move ownership of UploadDataStream from URLRequestHttpJob to URLRequest

net/url_request/url_request.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/url_request/url_request.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/debug/stack_trace.h"
 #include "base/lazy_instance.h"
 #include "base/memory/singleton.h"
 #include "base/message_loop.h"
 #include "base/metrics/stats_counters.h"
 #include "base/stl_util.h"
 #include "base/synchronization/lock.h"
 #include "net/base/auth.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 #include "net/base/network_change_notifier.h"
 #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"
 #include "net/url_request/url_request_error_job.h"
 #include "net/url_request/url_request_job.h"
 #include "net/url_request/url_request_job_manager.h"
 #include "net/url_request/url_request_netlog_params.h"
 #include "net/url_request/url_request_redirect_job.h"
 
 using base::Time;
@@ skipping 209 matching lines @@
 void URLRequest::RegisterRequestInterceptor(Interceptor* interceptor) {
   URLRequestJobManager::GetInstance()->RegisterRequestInterceptor(interceptor);
 }
 
 // static
 void URLRequest::UnregisterRequestInterceptor(Interceptor* interceptor) {
   URLRequestJobManager::GetInstance()->UnregisterRequestInterceptor(
       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_));
   }
 }
 
 void URLRequest::AppendChunkToUpload(const char* bytes,
                                      int bytes_len,
                                      bool is_last_chunk) {
   DCHECK(upload_);
   DCHECK(upload_->is_chunked());
   DCHECK_GT(bytes_len, 0);
   upload_->AppendChunk(bytes, bytes_len, is_last_chunk);
 }
 
 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,
                                            bool overwrite) {
   DCHECK(!is_pending_ || is_redirecting_);
   NOTREACHED() << "implement me!";
 }
 
 void URLRequest::SetExtraRequestHeaderByName(const string& name,
                                              const string& value,
@@ skipping 218 matching lines @@
 
   net_log_.BeginEvent(
       NetLog::TYPE_URL_REQUEST_START_JOB,
       base::Bind(&NetLogURLRequestStartCallback,
                  &url(), &method_, load_flags_, priority_,
                  upload_.get() ? upload_->identifier() : -1));
 
   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;
 
   response_info_.was_cached = false;
 
   // Don't allow errors to be sent from within Start().
   // TODO(brettw) this may cause NotifyDone to be sent synchronously,
   // we probably don't want this: they should be sent asynchronously so
   // the caller does not get reentered.
@@ skipping 230 matching lines @@
 
   // For 303 redirects, all request methods except HEAD are converted to GET,
   // as per the latest httpbis draft.  The draft also allows POST requests to
   // be converted to GETs when following 301/302 redirects, for historical
   // reasons. Most major browsers do this and so shall we.  Both RFC 2616 and
   // the httpbis draft say to prompt the user to confirm the generation of new
   // requests, other than GET and HEAD requests, but IE omits these prompts and
   // 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
       // the inclusion of a multipart Content-Type header in GET can cause
       // problems with some servers:
       // http://code.google.com/p/chromium/issues/detail?id=843
       StripPostSpecificHeaders(&extra_request_headers_);
     }
   }
 
   // Suppress the referrer if we're redirecting out of https.
   if (referrer_policy_ ==
           CLEAR_REFERRER_ON_TRANSITION_FROM_SECURE_TO_INSECURE &&
       GURL(referrer_).SchemeIsSecure() && !location.SchemeIsSecure()) {
     referrer_.clear();
   }
 
   url_chain_.push_back(location);
   --redirect_limit_;
 
   if (!final_upload_progress_.position())
     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;
 }
 
 const URLRequestContext* URLRequest::context() const {
   return context_;
 }
 
 int64 URLRequest::GetExpectedContentSize() const {
   int64 expected_content_size = -1;
@@ skipping 164 matching lines @@
       new base::debug::StackTrace(NULL, 0);
   *stack_trace_copy = stack_trace;
   stack_trace_.reset(stack_trace_copy);
 }
 
 const base::debug::StackTrace* URLRequest::stack_trace() const {
   return stack_trace_.get();
 }
 
 }  // namespace net