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

Unified Diff: content/browser/loader/redirect_to_file_resource_handler.cc

Issue 2526983002: Refactor ResourceHandler API. (Closed)
Patch Set: More fixes... Created 3 years, 11 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
Index: content/browser/loader/redirect_to_file_resource_handler.cc
diff --git a/content/browser/loader/redirect_to_file_resource_handler.cc b/content/browser/loader/redirect_to_file_resource_handler.cc
index b4018c843a0d1851bd30026a261a1bd2531331a9..f22fccce231f0aed4d340c955b0d2a168f07db7a 100644
--- a/content/browser/loader/redirect_to_file_resource_handler.cc
+++ b/content/browser/loader/redirect_to_file_resource_handler.cc
@@ -138,7 +138,6 @@ RedirectToFileResourceHandler::RedirectToFileResourceHandler(
write_cursor_(0),
writer_(NULL),
next_buffer_size_(kInitialReadBufSize),
- did_defer_(false),
completed_during_write_(false),
weak_factory_(this) {}
@@ -156,22 +155,24 @@ void RedirectToFileResourceHandler::
create_temporary_file_stream_ = create_temporary_file_stream;
}
-bool RedirectToFileResourceHandler::OnResponseStarted(
+void RedirectToFileResourceHandler::OnResponseStarted(
ResourceResponse* response,
- bool* defer) {
+ std::unique_ptr<ResourceController> controller) {
DCHECK(writer_);
response->head.download_file_path = writer_->path();
- return next_handler_->OnResponseStarted(response, defer);
+ next_handler_->OnResponseStarted(response, std::move(controller));
}
-bool RedirectToFileResourceHandler::OnWillStart(const GURL& url, bool* defer) {
+void RedirectToFileResourceHandler::OnWillStart(
+ const GURL& url,
+ std::unique_ptr<ResourceController> controller) {
DCHECK(!writer_);
// Defer starting the request until we have created the temporary file.
// TODO(darin): This is sub-optimal. We should not delay starting the
// network request like this.
will_start_url_ = url;
- did_defer_ = *defer = true;
+ HoldController(std::move(controller));
request()->LogBlockedBy("RedirectToFileResourceHandler");
if (create_temporary_file_stream_.is_null()) {
CreateTemporaryFileStream(
@@ -182,7 +183,6 @@ bool RedirectToFileResourceHandler::OnWillStart(const GURL& url, bool* defer) {
base::Bind(&RedirectToFileResourceHandler::DidCreateTemporaryFile,
weak_factory_.GetWeakPtr()));
}
- return true;
}
bool RedirectToFileResourceHandler::OnWillRead(
@@ -204,8 +204,9 @@ bool RedirectToFileResourceHandler::OnWillRead(
return true;
}
-bool RedirectToFileResourceHandler::OnReadCompleted(int bytes_read,
- bool* defer) {
+void RedirectToFileResourceHandler::OnReadCompleted(
+ int bytes_read,
+ std::unique_ptr<ResourceController> controller) {
DCHECK(buf_write_pending_);
buf_write_pending_ = false;
@@ -220,28 +221,28 @@ bool RedirectToFileResourceHandler::OnReadCompleted(int bytes_read,
next_buffer_size_ = std::min(next_buffer_size_ * 2, kMaxReadBufSize);
}
- bool success = WriteMore();
-
- if (success && BufIsFull()) {
- did_defer_ = *defer = true;
- request()->LogBlockedBy("RedirectToFileResourceHandler");
+ HoldController(std::move(controller));
+ // WriteMore will resume the request if there's more buffer space.
+ if (!WriteMore()) {
+ CancelWithError(net::ERR_FAILED);
+ return;
}
- return success;
+ if (has_controller())
+ request()->LogBlockedBy("RedirectToFileResourceHandler");
}
void RedirectToFileResourceHandler::OnResponseCompleted(
const net::URLRequestStatus& status,
- bool* defer) {
+ std::unique_ptr<ResourceController> controller) {
if (writer_ && writer_->is_writing()) {
completed_during_write_ = true;
completed_status_ = status;
- did_defer_ = true;
- *defer = true;
+ HoldController(std::move(controller));
request()->LogBlockedBy("RedirectToFileResourceHandler");
return;
}
- next_handler_->OnResponseCompleted(status, defer);
+ next_handler_->OnResponseCompleted(status, std::move(controller));
}
int RedirectToFileResourceHandler::GetBufferSizeForTesting() const {
@@ -253,25 +254,17 @@ void RedirectToFileResourceHandler::DidCreateTemporaryFile(
std::unique_ptr<net::FileStream> file_stream,
ShareableFileReference* deletable_file) {
DCHECK(!writer_);
+ DCHECK(has_controller());
if (error_code != base::File::FILE_OK) {
- controller()->CancelWithError(net::FileErrorToNetError(error_code));
+ CancelWithError(net::FileErrorToNetError(error_code));
return;
}
writer_ = new Writer(this, std::move(file_stream), deletable_file);
// Resume the request.
- DCHECK(did_defer_);
- bool defer = false;
request()->LogUnblocked();
- if (!next_handler_->OnWillStart(will_start_url_, &defer)) {
- controller()->Cancel();
- } else if (!defer) {
- Resume();
- } else {
- did_defer_ = false;
- }
- will_start_url_ = GURL();
+ next_handler_->OnWillStart(std::move(will_start_url_), ReleaseController());
}
void RedirectToFileResourceHandler::DidWriteToFile(int result) {
@@ -279,6 +272,8 @@ void RedirectToFileResourceHandler::DidWriteToFile(int result) {
if (result > 0) {
next_handler_->OnDataDownloaded(result);
write_cursor_ += result;
+ // WriteMore will resume the request if the request hasn't completed and
+ // there's more buffer space.
failed = !WriteMore();
} else {
failed = true;
@@ -294,22 +289,25 @@ void RedirectToFileResourceHandler::DidWriteToFile(int result) {
completed_status_ = net::URLRequestStatus(net::URLRequestStatus::CANCELED,
net::ERR_FAILED);
}
- if (!completed_during_write_)
- controller()->CancelWithError(net::ERR_FAILED);
+ if (!completed_during_write_) {
+ if (has_controller()) {
+ // If the write buffer is full, |this| has deferred the request, and
+ // can do an in-band cancel.
+ CancelWithError(net::ERR_FAILED);
+ } else {
+ OutOfBandCancel(net::ERR_FAILED, true /* tell_renderer */);
+ }
+ return;
+ }
}
if (completed_during_write_ && !writer_->is_writing()) {
// Resume shutdown now that all data has been written to disk. Note that
// this should run even in the |failed| case above, otherwise a failed write
// leaves the handler stuck.
- bool defer = false;
+ DCHECK(has_controller());
request()->LogUnblocked();
- next_handler_->OnResponseCompleted(completed_status_, &defer);
- if (!defer) {
- Resume();
- } else {
- did_defer_ = false;
- }
+ next_handler_->OnResponseCompleted(completed_status_, ReleaseController());
}
}
@@ -357,9 +355,9 @@ bool RedirectToFileResourceHandler::WriteMore() {
write_cursor_ += rv;
}
- // If the request was deferred to allow writing to the file, and the buffer is
- // no longer full, resume the request.
- if (did_defer_ && !completed_during_write_ && !BufIsFull()) {
+ // If the request was defered for a reason other than having been completed,
+ // and the buffer has space, resume the request.
+ if (has_controller() && !completed_during_write_ && !BufIsFull()) {
request()->LogUnblocked();
Resume();
}
@@ -374,10 +372,4 @@ bool RedirectToFileResourceHandler::BufIsFull() const {
return buf_->RemainingCapacity() <= (2 * net::kMaxBytesToSniff);
}
-void RedirectToFileResourceHandler::Resume() {
- DCHECK(did_defer_);
- did_defer_ = false;
- controller()->Resume();
-}
-
} // namespace content

Powered by Google App Engine
This is Rietveld 408576698