Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(454)

Unified Diff: net/url_request/url_fetcher_core.cc

Issue 12224066: Fix UrlFetcher upload retry handling. (Closed) Base URL: http://git.chromium.org/chromium/src.git@master
Patch Set: Explicitly forbid retries for chunked uploads Created 7 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/url_request/url_fetcher_core.h ('k') | net/url_request/url_fetcher_impl_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 016ddcb4d09064f13e5e1026086611ccfd795532..98ca070dbbc542e682d2bad3e1c51ac39f84b28e 100644
--- a/net/url_request/url_fetcher_core.cc
+++ b/net/url_request/url_fetcher_core.cc
@@ -331,44 +331,48 @@ void URLFetcherCore::Stop() {
void URLFetcherCore::SetUploadData(const std::string& upload_content_type,
const std::string& upload_content) {
DCHECK(!is_chunked_upload_);
- DCHECK(!upload_content_);
- DCHECK(upload_content_type_.empty());
-
- // Empty |upload_content_type| is allowed iff the |upload_content| is empty.
- DCHECK(upload_content.empty() || !upload_content_type.empty());
-
+ DCHECK(!upload_data_stream_);
upload_content_type_ = upload_content_type;
- scoped_ptr<UploadElementReader> reader(
- UploadOwnedBytesElementReader::CreateWithString(upload_content));
- upload_content_.reset(UploadDataStream::CreateWithReader(reader.Pass(), 0));
+ upload_content_ = upload_content;
}
void URLFetcherCore::SetUploadDataStream(
const std::string& upload_content_type,
- scoped_ptr<UploadDataStream> upload_content) {
+ scoped_ptr<UploadDataStream> upload_data_stream) {
DCHECK(!is_chunked_upload_);
- DCHECK(!upload_content_);
+ DCHECK(upload_content_.empty());
+ DCHECK(!upload_data_stream_);
DCHECK(upload_content_type_.empty());
+ // Data stream uploading can't be retried, as the stream can't be restarted.
+ DCHECK(!automatically_retry_on_5xx_ || max_retries_on_5xx_ == 0);
+ DCHECK_EQ(max_retries_on_network_changes_, 0);
Joao da Silva 2013/02/13 10:53:18 Same for max_retries_on_network_changes_
+
// Empty |upload_content_type| is not allowed here, because it is impossible
// to ensure non-empty |upload_content| as it may not be initialized yet.
DCHECK(!upload_content_type.empty());
upload_content_type_ = upload_content_type;
- upload_content_ = upload_content.Pass();
+ upload_data_stream_ = upload_data_stream.Pass();
}
void URLFetcherCore::SetChunkedUpload(const std::string& content_type) {
DCHECK(is_chunked_upload_ ||
(upload_content_type_.empty() &&
- !upload_content_));
+ upload_content_.empty() &&
+ !upload_data_stream_));
+
+ // Chunked uploading can't be retried, as chunked data may have been directly
+ // added to the previous URLRequest object.
+ DCHECK(!automatically_retry_on_5xx_ || max_retries_on_5xx_ == 0);
+ DCHECK_EQ(max_retries_on_network_changes_, 0);
Joao da Silva 2013/02/13 10:53:18 Same here
// Empty |content_type| is not allowed here, because it is impossible
// to ensure non-empty upload content as it is not yet supplied.
DCHECK(!content_type.empty());
upload_content_type_ = content_type;
- upload_content_.reset();
+ upload_content_.clear();
is_chunked_upload_ = true;
}
@@ -750,9 +754,6 @@ void URLFetcherCore::StartURLRequest() {
case URLFetcher::POST:
case URLFetcher::PUT:
case URLFetcher::PATCH:
- // Upload content must be set.
- DCHECK(is_chunked_upload_ || upload_content_);
-
request_->set_method(
request_type_ == URLFetcher::POST ? "POST" :
request_type_ == URLFetcher::PUT ? "PUT" : "PATCH");
@@ -760,8 +761,17 @@ void URLFetcherCore::StartURLRequest() {
extra_request_headers_.SetHeader(HttpRequestHeaders::kContentType,
upload_content_type_);
}
- if (upload_content_)
- request_->set_upload(upload_content_.Pass());
+ if (!is_chunked_upload_ &&
+ (!upload_content_.empty() || upload_content_type_.empty())) {
+ DCHECK(!upload_data_stream_);
+ scoped_ptr<UploadElementReader> reader(new UploadBytesElementReader(
+ upload_content_.data(), upload_content_.size()));
+ request_->set_upload(make_scoped_ptr(
+ UploadDataStream::CreateWithReader(reader.Pass(), 0)));
+ } else if (!is_chunked_upload_ && upload_data_stream_) {
+ request_->set_upload(upload_data_stream_.Pass());
+ }
+
current_upload_bytes_ = -1;
// TODO(kinaba): http://crbug.com/118103. Implement upload callback in the
// layer and avoid using timer here.
« no previous file with comments | « net/url_request/url_fetcher_core.h ('k') | net/url_request/url_fetcher_impl_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698