summaryrefslogtreecommitdiffstats
path: root/storage
diff options
context:
space:
mode:
authorxunjieli <xunjieli@chromium.org>2015-11-20 05:39:28 -0800
committerCommit bot <commit-bot@chromium.org>2015-11-20 13:40:31 +0000
commitaacaaa7fef4ee77110471c1a8e02e6c262d0a0a3 (patch)
tree743da3d7dbd3110708d447e04596cc2cc7716545 /storage
parent1ad7f8c87b370561e0d95acfa0efe9056970b3e3 (diff)
downloadchromium_src-aacaaa7fef4ee77110471c1a8e02e6c262d0a0a3.zip
chromium_src-aacaaa7fef4ee77110471c1a8e02e6c262d0a0a3.tar.gz
chromium_src-aacaaa7fef4ee77110471c1a8e02e6c262d0a0a3.tar.bz2
Revert "Reland: URLRequestJob: change ReadRawData contract"
This reverts commit d77911ac82186d65a8f11555a7a6b1678c769ba2. The previous CL caused a top crasher on Canary. Reverting this CL now since fixing the crash isn't straightforward. TBR=michaeln@chromium.org,mnaganov@chromium.org,skyostil@chromium.org,eugenebut@chromium.org,davidben@chromium.org,falken@chromium.org,mtomasz@chromium.org, sky@chromium.org,jianli@chromium.org,zork@chromium.org,mmenke@chromium.org,rdsmith@chromium.org BUG=558224 BUG=553300 BUG=474859 BUG=329902 Review URL: https://codereview.chromium.org/1459333002 Cr-Commit-Position: refs/heads/master@{#360809}
Diffstat (limited to 'storage')
-rw-r--r--storage/browser/blob/blob_url_request_job.cc42
-rw-r--r--storage/browser/blob/blob_url_request_job.h2
-rw-r--r--storage/browser/fileapi/file_system_dir_url_request_job.cc21
-rw-r--r--storage/browser/fileapi/file_system_dir_url_request_job.h2
-rw-r--r--storage/browser/fileapi/file_system_url_request_job.cc70
-rw-r--r--storage/browser/fileapi/file_system_url_request_job.h5
6 files changed, 84 insertions, 58 deletions
diff --git a/storage/browser/blob/blob_url_request_job.cc b/storage/browser/blob/blob_url_request_job.cc
index deea300..6ec2fe5 100644
--- a/storage/browser/blob/blob_url_request_job.cc
+++ b/storage/browser/blob/blob_url_request_job.cc
@@ -75,37 +75,41 @@ void BlobURLRequestJob::Kill() {
weak_factory_.InvalidateWeakPtrs();
}
-int BlobURLRequestJob::ReadRawData(net::IOBuffer* dest, int dest_size) {
+bool BlobURLRequestJob::ReadRawData(net::IOBuffer* dest,
+ int dest_size,
+ int* bytes_read) {
TRACE_EVENT_ASYNC_BEGIN1("Blob", "BlobRequest::ReadRawData", this, "uuid",
blob_handle_ ? blob_handle_->uuid() : "NotFound");
DCHECK_NE(dest_size, 0);
+ DCHECK(bytes_read);
- // Bail out immediately if we encounter an error. This happens if a previous
- // ReadRawData signalled an error to its caller but the caller called
- // ReadRawData again anyway.
- if (error_)
- return 0;
+ // Bail out immediately if we encounter an error.
+ if (error_) {
+ *bytes_read = 0;
+ return true;
+ }
- int bytes_read = 0;
BlobReader::Status read_status =
- blob_reader_->Read(dest, dest_size, &bytes_read,
+ blob_reader_->Read(dest, dest_size, bytes_read,
base::Bind(&BlobURLRequestJob::DidReadRawData,
weak_factory_.GetWeakPtr()));
switch (read_status) {
case BlobReader::Status::NET_ERROR:
+ NotifyFailure(blob_reader_->net_error());
TRACE_EVENT_ASYNC_END1("Blob", "BlobRequest::ReadRawData", this, "uuid",
blob_handle_ ? blob_handle_->uuid() : "NotFound");
- return blob_reader_->net_error();
+ return false;
case BlobReader::Status::IO_PENDING:
- return net::ERR_IO_PENDING;
+ SetStatus(net::URLRequestStatus(net::URLRequestStatus::IO_PENDING, 0));
+ return false;
case BlobReader::Status::DONE:
TRACE_EVENT_ASYNC_END1("Blob", "BlobRequest::ReadRawData", this, "uuid",
blob_handle_ ? blob_handle_->uuid() : "NotFound");
- return bytes_read;
+ return true;
}
NOTREACHED();
- return 0;
+ return true;
}
bool BlobURLRequestJob::GetMimeType(std::string* mime_type) const {
@@ -218,7 +222,13 @@ void BlobURLRequestJob::DidCalculateSize(int result) {
void BlobURLRequestJob::DidReadRawData(int result) {
TRACE_EVENT_ASYNC_END1("Blob", "BlobRequest::ReadRawData", this, "uuid",
blob_handle_ ? blob_handle_->uuid() : "NotFound");
- ReadRawDataComplete(result);
+ if (result < 0) {
+ NotifyFailure(result);
+ return;
+ }
+ // Clear the IO_PENDING status
+ SetStatus(net::URLRequestStatus());
+ NotifyReadComplete(result);
}
void BlobURLRequestJob::NotifyFailure(int error_code) {
@@ -226,7 +236,11 @@ void BlobURLRequestJob::NotifyFailure(int error_code) {
// If we already return the headers on success, we can't change the headers
// now. Instead, we just error out.
- DCHECK(!response_info_) << "Cannot NotifyFailure after headers.";
+ if (response_info_) {
+ NotifyDone(
+ net::URLRequestStatus(net::URLRequestStatus::FAILED, error_code));
+ return;
+ }
net::HttpStatusCode status_code = net::HTTP_INTERNAL_SERVER_ERROR;
switch (error_code) {
diff --git a/storage/browser/blob/blob_url_request_job.h b/storage/browser/blob/blob_url_request_job.h
index 1f0b9fb..21baa2c 100644
--- a/storage/browser/blob/blob_url_request_job.h
+++ b/storage/browser/blob/blob_url_request_job.h
@@ -44,7 +44,7 @@ class STORAGE_EXPORT BlobURLRequestJob
// net::URLRequestJob methods.
void Start() override;
void Kill() override;
- int ReadRawData(net::IOBuffer* buf, int buf_size) override;
+ bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
bool GetMimeType(std::string* mime_type) const override;
void GetResponseInfo(net::HttpResponseInfo* info) override;
int GetResponseCode() const override;
diff --git a/storage/browser/fileapi/file_system_dir_url_request_job.cc b/storage/browser/fileapi/file_system_dir_url_request_job.cc
index c053773..fc91f3b 100644
--- a/storage/browser/fileapi/file_system_dir_url_request_job.cc
+++ b/storage/browser/fileapi/file_system_dir_url_request_job.cc
@@ -14,6 +14,7 @@
#include "base/time/time.h"
#include "build/build_config.h"
#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
#include "net/base/net_util.h"
#include "net/url_request/url_request.h"
#include "storage/browser/fileapi/file_system_context.h"
@@ -43,14 +44,16 @@ FileSystemDirURLRequestJob::FileSystemDirURLRequestJob(
FileSystemDirURLRequestJob::~FileSystemDirURLRequestJob() {
}
-int FileSystemDirURLRequestJob::ReadRawData(net::IOBuffer* dest,
- int dest_size) {
- int count = std::min(dest_size, base::checked_cast<int>(data_.size()));
+bool FileSystemDirURLRequestJob::ReadRawData(net::IOBuffer* dest,
+ int dest_size,
+ int* bytes_read) {
+ int count = std::min(dest_size, static_cast<int>(data_.size()));
if (count > 0) {
memcpy(dest->data(), data_.data(), count);
data_.erase(0, count);
}
- return count;
+ *bytes_read = count;
+ return true;
}
void FileSystemDirURLRequestJob::Start() {
@@ -96,7 +99,8 @@ void FileSystemDirURLRequestJob::StartAsync() {
false);
return;
}
- NotifyStartError(URLRequestStatus::FromError(net::ERR_FILE_NOT_FOUND));
+ NotifyDone(
+ URLRequestStatus(URLRequestStatus::FAILED, net::ERR_FILE_NOT_FOUND));
return;
}
file_system_context_->operation_runner()->ReadDirectory(
@@ -109,7 +113,8 @@ void FileSystemDirURLRequestJob::DidAttemptAutoMount(base::File::Error result) {
file_system_context_->CrackURL(request_->url()).is_valid()) {
StartAsync();
} else {
- NotifyStartError(URLRequestStatus::FromError(net::ERR_FILE_NOT_FOUND));
+ NotifyDone(
+ URLRequestStatus(URLRequestStatus::FAILED, net::ERR_FILE_NOT_FOUND));
}
}
@@ -121,7 +126,7 @@ void FileSystemDirURLRequestJob::DidReadDirectory(
int rv = net::ERR_FILE_NOT_FOUND;
if (result == base::File::FILE_ERROR_INVALID_URL)
rv = net::ERR_INVALID_URL;
- NotifyStartError(URLRequestStatus::FromError(rv));
+ NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, rv));
return;
}
@@ -170,7 +175,7 @@ void FileSystemDirURLRequestJob::DidGetMetadata(
int rv = net::ERR_FILE_NOT_FOUND;
if (result == base::File::FILE_ERROR_INVALID_URL)
rv = net::ERR_INVALID_URL;
- NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED, rv));
+ NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, rv));
}
if (!request_)
diff --git a/storage/browser/fileapi/file_system_dir_url_request_job.h b/storage/browser/fileapi/file_system_dir_url_request_job.h
index 93fdd1f..01bd372 100644
--- a/storage/browser/fileapi/file_system_dir_url_request_job.h
+++ b/storage/browser/fileapi/file_system_dir_url_request_job.h
@@ -32,7 +32,7 @@ class STORAGE_EXPORT FileSystemDirURLRequestJob : public net::URLRequestJob {
// URLRequestJob methods:
void Start() override;
void Kill() override;
- int ReadRawData(net::IOBuffer* buf, int buf_size) override;
+ bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
bool GetCharset(std::string* charset) override;
// FilterContext methods (via URLRequestJob):
diff --git a/storage/browser/fileapi/file_system_url_request_job.cc b/storage/browser/fileapi/file_system_url_request_job.cc
index 8edcb3d..8d58f64 100644
--- a/storage/browser/fileapi/file_system_url_request_job.cc
+++ b/storage/browser/fileapi/file_system_url_request_job.cc
@@ -62,7 +62,6 @@ FileSystemURLRequestJob::FileSystemURLRequestJob(
file_system_context_(file_system_context),
is_directory_(false),
remaining_bytes_(0),
- range_parse_result_(net::OK),
weak_factory_(this) {}
FileSystemURLRequestJob::~FileSystemURLRequestJob() {}
@@ -80,28 +79,39 @@ void FileSystemURLRequestJob::Kill() {
weak_factory_.InvalidateWeakPtrs();
}
-int FileSystemURLRequestJob::ReadRawData(net::IOBuffer* dest, int dest_size) {
+bool FileSystemURLRequestJob::ReadRawData(net::IOBuffer* dest,
+ int dest_size,
+ int* bytes_read) {
DCHECK_NE(dest_size, 0);
+ DCHECK(bytes_read);
DCHECK_GE(remaining_bytes_, 0);
if (reader_.get() == NULL)
- return net::ERR_FAILED;
+ return false;
if (remaining_bytes_ < dest_size)
- dest_size = remaining_bytes_;
+ dest_size = static_cast<int>(remaining_bytes_);
- if (!dest_size)
- return 0;
+ if (!dest_size) {
+ *bytes_read = 0;
+ return true;
+ }
const int rv = reader_->Read(dest, dest_size,
base::Bind(&FileSystemURLRequestJob::DidRead,
weak_factory_.GetWeakPtr()));
if (rv >= 0) {
+ // Data is immediately available.
+ *bytes_read = rv;
remaining_bytes_ -= rv;
DCHECK_GE(remaining_bytes_, 0);
+ return true;
}
-
- return rv;
+ if (rv == net::ERR_IO_PENDING)
+ SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
+ else
+ NotifyFailed(rv);
+ return false;
}
bool FileSystemURLRequestJob::GetMimeType(std::string* mime_type) const {
@@ -116,12 +126,8 @@ bool FileSystemURLRequestJob::GetMimeType(std::string* mime_type) const {
void FileSystemURLRequestJob::SetExtraRequestHeaders(
const net::HttpRequestHeaders& headers) {
std::string range_header;
- // Currently this job only cares about the Range header. Note that validation
- // is deferred to DidGetMetaData(), because NotifyStartError is not legal to
- // call since the job has not started.
if (headers.GetHeader(net::HttpRequestHeaders::kRange, &range_header)) {
std::vector<net::HttpByteRange> ranges;
-
if (net::HttpUtil::ParseRangeHeader(range_header, &ranges)) {
if (ranges.size() == 1) {
byte_range_ = ranges[0];
@@ -129,7 +135,7 @@ void FileSystemURLRequestJob::SetExtraRequestHeaders(
// We don't support multiple range requests in one single URL request.
// TODO(adamk): decide whether we want to support multiple range
// requests.
- range_parse_result_ = net::ERR_REQUEST_RANGE_NOT_SATISFIABLE;
+ NotifyFailed(net::ERR_REQUEST_RANGE_NOT_SATISFIABLE);
}
}
}
@@ -161,7 +167,7 @@ void FileSystemURLRequestJob::StartAsync() {
}
if (!file_system_context_->CanServeURLRequest(url_)) {
// In incognito mode the API is not usable and there should be no data.
- NotifyStartError(URLRequestStatus::FromError(net::ERR_FILE_NOT_FOUND));
+ NotifyFailed(net::ERR_FILE_NOT_FOUND);
return;
}
file_system_context_->operation_runner()->GetMetadata(
@@ -176,7 +182,7 @@ void FileSystemURLRequestJob::DidAttemptAutoMount(base::File::Error result) {
file_system_context_->CrackURL(request_->url()).is_valid()) {
StartAsync();
} else {
- NotifyStartError(URLRequestStatus::FromError(net::ERR_FILE_NOT_FOUND));
+ NotifyFailed(net::ERR_FILE_NOT_FOUND);
}
}
@@ -184,10 +190,9 @@ void FileSystemURLRequestJob::DidGetMetadata(
base::File::Error error_code,
const base::File::Info& file_info) {
if (error_code != base::File::FILE_OK) {
- NotifyStartError(URLRequestStatus::FromError(
- error_code == base::File::FILE_ERROR_INVALID_URL
- ? net::ERR_INVALID_URL
- : net::ERR_FILE_NOT_FOUND));
+ NotifyFailed(error_code == base::File::FILE_ERROR_INVALID_URL
+ ? net::ERR_INVALID_URL
+ : net::ERR_FILE_NOT_FOUND);
return;
}
@@ -197,14 +202,8 @@ void FileSystemURLRequestJob::DidGetMetadata(
is_directory_ = file_info.is_directory;
- if (range_parse_result_ != net::OK) {
- NotifyStartError(URLRequestStatus::FromError(range_parse_result_));
- return;
- }
-
if (!byte_range_.ComputeBounds(file_info.size)) {
- NotifyStartError(
- URLRequestStatus::FromError(net::ERR_REQUEST_RANGE_NOT_SATISFIABLE));
+ NotifyFailed(net::ERR_REQUEST_RANGE_NOT_SATISFIABLE);
return;
}
@@ -228,12 +227,17 @@ void FileSystemURLRequestJob::DidGetMetadata(
}
void FileSystemURLRequestJob::DidRead(int result) {
- if (result >= 0) {
- remaining_bytes_ -= result;
- DCHECK_GE(remaining_bytes_, 0);
- }
+ if (result > 0)
+ SetStatus(URLRequestStatus()); // Clear the IO_PENDING status
+ else if (result == 0)
+ NotifyDone(URLRequestStatus());
+ else
+ NotifyFailed(result);
+
+ remaining_bytes_ -= result;
+ DCHECK_GE(remaining_bytes_, 0);
- ReadRawDataComplete(result);
+ NotifyReadComplete(result);
}
bool FileSystemURLRequestJob::IsRedirectResponse(GURL* location,
@@ -253,4 +257,8 @@ bool FileSystemURLRequestJob::IsRedirectResponse(GURL* location,
return false;
}
+void FileSystemURLRequestJob::NotifyFailed(int rv) {
+ NotifyDone(URLRequestStatus(URLRequestStatus::FAILED, rv));
+}
+
} // namespace storage
diff --git a/storage/browser/fileapi/file_system_url_request_job.h b/storage/browser/fileapi/file_system_url_request_job.h
index 308b66c..e442cae 100644
--- a/storage/browser/fileapi/file_system_url_request_job.h
+++ b/storage/browser/fileapi/file_system_url_request_job.h
@@ -11,7 +11,6 @@
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
-#include "net/base/net_errors.h"
#include "net/http/http_byte_range.h"
#include "net/url_request/url_request_job.h"
#include "storage/browser/fileapi/file_system_url.h"
@@ -42,7 +41,7 @@ class STORAGE_EXPORT FileSystemURLRequestJob : public net::URLRequestJob {
// URLRequestJob methods:
void Start() override;
void Kill() override;
- int ReadRawData(net::IOBuffer* buf, int buf_size) override;
+ bool ReadRawData(net::IOBuffer* buf, int buf_size, int* bytes_read) override;
bool IsRedirectResponse(GURL* location, int* http_status_code) override;
void SetExtraRequestHeaders(const net::HttpRequestHeaders& headers) override;
void GetResponseInfo(net::HttpResponseInfo* info) override;
@@ -61,6 +60,7 @@ class STORAGE_EXPORT FileSystemURLRequestJob : public net::URLRequestJob {
void DidGetMetadata(base::File::Error error_code,
const base::File::Info& file_info);
void DidRead(int result);
+ void NotifyFailed(int rv);
const std::string storage_domain_;
FileSystemContext* file_system_context_;
@@ -69,7 +69,6 @@ class STORAGE_EXPORT FileSystemURLRequestJob : public net::URLRequestJob {
bool is_directory_;
scoped_ptr<net::HttpResponseInfo> response_info_;
int64 remaining_bytes_;
- net::Error range_parse_result_;
net::HttpByteRange byte_range_;
base::WeakPtrFactory<FileSystemURLRequestJob> weak_factory_;