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

Unified Diff: content/browser/service_worker/service_worker_write_to_cache_job.cc

Issue 1410643007: URLRequestJob: change ReadRawData contract (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Move NotifyReadCompleted Created 5 years, 2 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/service_worker/service_worker_write_to_cache_job.cc
diff --git a/content/browser/service_worker/service_worker_write_to_cache_job.cc b/content/browser/service_worker/service_worker_write_to_cache_job.cc
index 95d607411899759794b7d7af6340d19b51f67d82..894e86770ee37cc3212673da5f519c3bcf764cd9 100644
--- a/content/browser/service_worker/service_worker_write_to_cache_job.cc
+++ b/content/browser/service_worker/service_worker_write_to_cache_job.cc
@@ -157,40 +157,19 @@ void ServiceWorkerWriteToCacheJob::SetExtraRequestHeaders(
net_request_->SetExtraRequestHeaders(headers);
}
-bool ServiceWorkerWriteToCacheJob::ReadRawData(net::IOBuffer* buf,
- int buf_size,
- int* bytes_read) {
- net::URLRequestStatus status = ReadNetData(buf, buf_size, bytes_read);
- SetStatus(status);
+int ServiceWorkerWriteToCacheJob::ReadRawData(net::IOBuffer* buf,
+ int buf_size) {
+ int bytes_read = 0;
+ net::URLRequestStatus status = ReadNetData(buf, buf_size, &bytes_read);
if (status.is_io_pending())
- return false;
-
- if (!status.is_success()) {
- NotifyDoneHelper(status, kFetchScriptError);
- return false;
- }
-
- HandleNetData(*bytes_read);
- status = GetStatus();
-
- // Synchronous EOFs that do not replace the incumbent entry are considered
- // failures. Since normally the URLRequestJob's status would be set by
- // ReadNetData or HandleNetData, this code has to manually fix up the status
- // to match the failure this function is about to return.
- if (status.status() == net::URLRequestStatus::SUCCESS &&
- *bytes_read == 0 && !cache_writer_->did_replace()) {
- status = net::URLRequestStatus::FromError(kIdenticalScriptError);
- }
+ return net::ERR_IO_PENDING;
if (!status.is_success()) {
- NotifyDoneHelper(status, "");
- return false;
+ NotifyFinishedCaching(status, kFetchScriptError);
+ return status.error();
}
- // Since URLRequestStatus::is_success() means "SUCCESS or IO_PENDING", but the
- // contract of this function is "return true for synchronous successes only",
- // it is important to test against SUCCESS explicitly here.
- return status.status() == net::URLRequestStatus::SUCCESS;
+ return HandleNetData(bytes_read);
}
const net::HttpResponseInfo* ServiceWorkerWriteToCacheJob::http_info() const {
@@ -245,9 +224,9 @@ void ServiceWorkerWriteToCacheJob::OnReceivedRedirect(
TRACE_EVENT0("ServiceWorker",
"ServiceWorkerWriteToCacheJob::OnReceivedRedirect");
// Script resources can't redirect.
- NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
- net::ERR_UNSAFE_REDIRECT),
- kRedirectError);
+ NotifyStartErrorHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+ net::ERR_UNSAFE_REDIRECT),
+ kRedirectError);
}
void ServiceWorkerWriteToCacheJob::OnAuthRequired(
@@ -257,7 +236,7 @@ void ServiceWorkerWriteToCacheJob::OnAuthRequired(
TRACE_EVENT0("ServiceWorker",
"ServiceWorkerWriteToCacheJob::OnAuthRequired");
// TODO(michaeln): Pass this thru to our jobs client.
- NotifyDoneHelper(
+ NotifyStartErrorHelper(
net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_FAILED),
kClientAuthenticationError);
}
@@ -270,7 +249,7 @@ void ServiceWorkerWriteToCacheJob::OnCertificateRequested(
"ServiceWorkerWriteToCacheJob::OnCertificateRequested");
// TODO(michaeln): Pass this thru to our jobs client.
// see NotifyCertificateRequested.
- NotifyDoneHelper(
+ NotifyStartErrorHelper(
net::URLRequestStatus(net::URLRequestStatus::FAILED, net::ERR_FAILED),
kClientAuthenticationError);
}
@@ -284,9 +263,9 @@ void ServiceWorkerWriteToCacheJob::OnSSLCertificateError(
"ServiceWorkerWriteToCacheJob::OnSSLCertificateError");
// TODO(michaeln): Pass this thru to our jobs client,
// see NotifySSLCertificateError.
- NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
- net::ERR_INSECURE_RESPONSE),
- kSSLError);
+ NotifyStartErrorHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+ net::ERR_INSECURE_RESPONSE),
+ kSSLError);
}
void ServiceWorkerWriteToCacheJob::OnBeforeNetworkStart(
@@ -302,15 +281,15 @@ void ServiceWorkerWriteToCacheJob::OnResponseStarted(
net::URLRequest* request) {
DCHECK_EQ(net_request_, request);
if (!request->status().is_success()) {
- NotifyDoneHelper(request->status(), kFetchScriptError);
+ NotifyStartErrorHelper(request->status(), kFetchScriptError);
return;
}
if (request->GetResponseCode() / 100 != 2) {
std::string error_message =
base::StringPrintf(kBadHTTPResponseError, request->GetResponseCode());
- NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
- net::ERR_INVALID_RESPONSE),
- error_message);
+ NotifyStartErrorHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+ net::ERR_INVALID_RESPONSE),
+ error_message);
// TODO(michaeln): Instead of error'ing immediately, send the net
// response to our consumer, just don't cache it?
return;
@@ -321,9 +300,10 @@ void ServiceWorkerWriteToCacheJob::OnResponseStarted(
const net::HttpNetworkSession::Params* session_params =
request->context()->GetNetworkSessionParams();
if (!session_params || !session_params->ignore_certificate_errors) {
- NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
- net::ERR_INSECURE_RESPONSE),
- kSSLError);
+ NotifyStartErrorHelper(
+ net::URLRequestStatus(net::URLRequestStatus::FAILED,
+ net::ERR_INSECURE_RESPONSE),
+ kSSLError);
return;
}
}
@@ -338,9 +318,10 @@ void ServiceWorkerWriteToCacheJob::OnResponseStarted(
mime_type.empty()
? kNoMIMEError
: base::StringPrintf(kBadMIMEError, mime_type.c_str());
- NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
- net::ERR_INSECURE_RESPONSE),
- error_message);
+ NotifyStartErrorHelper(
+ net::URLRequestStatus(net::URLRequestStatus::FAILED,
+ net::ERR_INSECURE_RESPONSE),
+ error_message);
return;
}
@@ -376,53 +357,46 @@ void ServiceWorkerWriteToCacheJob::OnWriteHeadersComplete(net::Error error) {
NotifyHeadersComplete();
}
-void ServiceWorkerWriteToCacheJob::HandleNetData(int bytes_read) {
+int ServiceWorkerWriteToCacheJob::HandleNetData(int bytes_read) {
io_buffer_bytes_ = bytes_read;
net::Error error = cache_writer_->MaybeWriteData(
io_buffer_.get(), bytes_read,
base::Bind(&ServiceWorkerWriteToCacheJob::OnWriteDataComplete,
weak_factory_.GetWeakPtr()));
- SetStatus(net::URLRequestStatus::FromError(error));
// In case of ERR_IO_PENDING, this logic is done in OnWriteDataComplete.
if (error != net::ERR_IO_PENDING && bytes_read == 0) {
NotifyFinishedCaching(net::URLRequestStatus::FromError(error),
std::string());
mmenke 2015/10/28 16:40:42 If NotifyFinishedCaching sets the error to kIdenti
xunjieli 2015/10/28 21:11:52 Done.
}
+ return error == net::OK ? bytes_read : error;
}
void ServiceWorkerWriteToCacheJob::OnWriteDataComplete(net::Error error) {
SetStatus(net::URLRequestStatus::FromError(error));
DCHECK_NE(net::ERR_IO_PENDING, error);
- if (io_buffer_bytes_ == 0) {
- NotifyDoneHelper(net::URLRequestStatus::FromError(error), std::string());
- }
- NotifyReadComplete(error == net::OK ? io_buffer_bytes_ : error);
+ if (io_buffer_bytes_ == 0)
+ NotifyFinishedCaching(net::URLRequestStatus::FromError(error), "");
+ ReadRawDataComplete(error == net::OK ? io_buffer_bytes_ : error);
}
-void ServiceWorkerWriteToCacheJob::OnReadCompleted(
- net::URLRequest* request,
- int bytes_read) {
+void ServiceWorkerWriteToCacheJob::OnReadCompleted(net::URLRequest* request,
+ int result) {
DCHECK_EQ(net_request_, request);
- if (bytes_read < 0) {
- DCHECK(!request->status().is_success());
- NotifyDoneHelper(request->status(), kFetchScriptError);
+ DCHECK_NE(result, net::ERR_IO_PENDING);
+
+ if (result >= 0)
+ result = HandleNetData(result);
+
+ // ReadRawDataComplete will be called in OnWriteDataComplete, so return early.
+ if (result == net::ERR_IO_PENDING)
return;
+
+ if (result < 0) {
+ DCHECK(!request->status().is_success());
mmenke 2015/10/28 16:40:42 This DCHECK seems wrong - shouldn't it be above th
xunjieli 2015/10/28 21:11:52 Done. Thanks for explaining!
+ NotifyFinishedCaching(request->status(), kFetchScriptError);
mmenke 2015/10/28 16:40:42 Seems like moving this above the HandleNetData cal
xunjieli 2015/10/28 21:11:52 Done.
}
- HandleNetData(bytes_read);
- // HandleNetData can cause status of this job to change. If the status changes
- // to IO_PENDING, that means HandleNetData has pending IO, and
- // NotifyReadComplete will be called later by the appropriate callback.
- if (!GetStatus().is_io_pending()) {
- int result = GetStatus().status() == net::URLRequestStatus::SUCCESS
- ? bytes_read
- : GetStatus().error();
- // If bytes_read is 0, HandleNetData synchronously completed and this job is
- // at EOF.
- if (bytes_read == 0)
- NotifyDoneHelper(GetStatus(), std::string());
- NotifyReadComplete(result);
- }
+ ReadRawDataComplete(result);
}
bool ServiceWorkerWriteToCacheJob::CheckPathRestriction(
@@ -436,15 +410,15 @@ bool ServiceWorkerWriteToCacheJob::CheckPathRestriction(
if (!ServiceWorkerUtils::IsPathRestrictionSatisfied(
version_->scope(), url_,
has_header ? &service_worker_allowed : nullptr, &error_message)) {
- NotifyDoneHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
- net::ERR_INSECURE_RESPONSE),
- error_message);
+ NotifyStartErrorHelper(net::URLRequestStatus(net::URLRequestStatus::FAILED,
+ net::ERR_INSECURE_RESPONSE),
+ error_message);
return false;
}
return true;
}
-void ServiceWorkerWriteToCacheJob::NotifyDoneHelper(
+void ServiceWorkerWriteToCacheJob::NotifyStartErrorHelper(
const net::URLRequestStatus& status,
const std::string& status_message) {
DCHECK(!status.is_io_pending());
@@ -465,7 +439,7 @@ void ServiceWorkerWriteToCacheJob::NotifyDoneHelper(
}
SetStatus(reported_status);
- NotifyDone(reported_status);
+ NotifyStartError(reported_status);
}
void ServiceWorkerWriteToCacheJob::NotifyFinishedCaching(

Powered by Google App Engine
This is Rietveld 408576698