Commit 12bd94e4 authored by Robert Ogden's avatar Robert Ogden Committed by Commit Bot

Expose URLLoaderCompletionStatus in SimpleURLLoader

Adds a new method that returns the URLLoaderCompletionStatus from the
URL Loader's OnComplete. This is needed since the completion status
provides additional metrics needed to compute fetch duration and
encoded body length which are used in
https://chromium-review.googlesource.com/c/chromium/src/+/2463544.

The returned completion status is Optional to account for cases where
the URLLoader never starts, or gets an unexpected response from the
network.

Bug: 1136174
Change-Id: Ib956135f30ec738f85f22739372387b1093815fe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2468347
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Reviewed-by: default avatarClark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816815}
parent b79ce46d
...@@ -235,6 +235,8 @@ class SimpleURLLoaderImpl : public SimpleURLLoader, ...@@ -235,6 +235,8 @@ class SimpleURLLoaderImpl : public SimpleURLLoader,
int NetError() const override; int NetError() const override;
const mojom::URLResponseHead* ResponseInfo() const override; const mojom::URLResponseHead* ResponseInfo() const override;
const base::Optional<URLLoaderCompletionStatus>& CompletionStatus()
const override;
const GURL& GetFinalURL() const override; const GURL& GetFinalURL() const override;
bool LoadedFromCache() const override; bool LoadedFromCache() const override;
int64_t GetContentSize() const override; int64_t GetContentSize() const override;
...@@ -267,9 +269,6 @@ class SimpleURLLoaderImpl : public SimpleURLLoader, ...@@ -267,9 +269,6 @@ class SimpleURLLoaderImpl : public SimpleURLLoader,
~RequestState() = default; ~RequestState() = default;
bool request_completed = false; bool request_completed = false;
// The expected total size of the body, taken from
// URLLoaderCompletionStatus.
int64_t expected_body_size = 0;
bool body_started = false; bool body_started = false;
bool body_completed = false; bool body_completed = false;
...@@ -286,6 +285,8 @@ class SimpleURLLoaderImpl : public SimpleURLLoader, ...@@ -286,6 +285,8 @@ class SimpleURLLoaderImpl : public SimpleURLLoader,
bool loaded_from_cache = false; bool loaded_from_cache = false;
mojom::URLResponseHeadPtr response_info; mojom::URLResponseHeadPtr response_info;
base::Optional<URLLoaderCompletionStatus> completion_status;
}; };
// Prepares internal state to start a request, and then calls StartRequest(). // Prepares internal state to start a request, and then calls StartRequest().
...@@ -1381,36 +1382,43 @@ void SimpleURLLoaderImpl::SetTimeoutDuration(base::TimeDelta timeout_duration) { ...@@ -1381,36 +1382,43 @@ void SimpleURLLoaderImpl::SetTimeoutDuration(base::TimeDelta timeout_duration) {
} }
int SimpleURLLoaderImpl::NetError() const { int SimpleURLLoaderImpl::NetError() const {
// Should only be called once the request is compelete. // Should only be called once the request is complete.
DCHECK(request_state_->finished); DCHECK(request_state_->finished);
DCHECK_NE(net::ERR_IO_PENDING, request_state_->net_error); DCHECK_NE(net::ERR_IO_PENDING, request_state_->net_error);
return request_state_->net_error; return request_state_->net_error;
} }
const GURL& SimpleURLLoaderImpl::GetFinalURL() const { const GURL& SimpleURLLoaderImpl::GetFinalURL() const {
// Should only be called once the request is compelete. // Should only be called once the request is complete.
DCHECK(request_state_->finished); DCHECK(request_state_->finished);
return final_url_; return final_url_;
} }
bool SimpleURLLoaderImpl::LoadedFromCache() const { bool SimpleURLLoaderImpl::LoadedFromCache() const {
// Should only be called once the request is compelete. // Should only be called once the request is complete.
DCHECK(request_state_->finished); DCHECK(request_state_->finished);
return request_state_->loaded_from_cache; return request_state_->loaded_from_cache;
} }
int64_t SimpleURLLoaderImpl::GetContentSize() const { int64_t SimpleURLLoaderImpl::GetContentSize() const {
// Should only be called once the request is compelete. // Should only be called once the request is complete.
DCHECK(request_state_->finished); DCHECK(request_state_->finished);
return request_state_->received_body_size; return request_state_->received_body_size;
} }
const mojom::URLResponseHead* SimpleURLLoaderImpl::ResponseInfo() const { const mojom::URLResponseHead* SimpleURLLoaderImpl::ResponseInfo() const {
// Should only be called once the request is compelete. // Should only be called once the request is complete.
DCHECK(request_state_->finished); DCHECK(request_state_->finished);
return request_state_->response_info.get(); return request_state_->response_info.get();
} }
const base::Optional<URLLoaderCompletionStatus>&
SimpleURLLoaderImpl::CompletionStatus() const {
// Should only be called once the request is complete.
DCHECK(request_state_->finished);
return request_state_->completion_status;
}
void SimpleURLLoaderImpl::OnBodyHandlerDone(net::Error error, void SimpleURLLoaderImpl::OnBodyHandlerDone(net::Error error,
int64_t received_body_size) { int64_t received_body_size) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
...@@ -1419,6 +1427,10 @@ void SimpleURLLoaderImpl::OnBodyHandlerDone(net::Error error, ...@@ -1419,6 +1427,10 @@ void SimpleURLLoaderImpl::OnBodyHandlerDone(net::Error error,
// If there's an error, fail request and report it immediately. // If there's an error, fail request and report it immediately.
if (error != net::OK) { if (error != net::OK) {
// Reset the completion status since the contained metrics like encoded body
// length and net error are not reliable when the body itself was not
// successfully completed.
request_state_->completion_status = base::nullopt;
// When |allow_partial_results_| is true, a valid body|file_path is // When |allow_partial_results_| is true, a valid body|file_path is
// passed to the completion callback even in the case of failures. // passed to the completion callback even in the case of failures.
// For consistency, it makes sense to also hold the actual decompressed // For consistency, it makes sense to also hold the actual decompressed
...@@ -1668,14 +1680,16 @@ void SimpleURLLoaderImpl::OnComplete(const URLLoaderCompletionStatus& status) { ...@@ -1668,14 +1680,16 @@ void SimpleURLLoaderImpl::OnComplete(const URLLoaderCompletionStatus& status) {
client_receiver_.reset(); client_receiver_.reset();
url_loader_.reset(); url_loader_.reset();
request_state_->completion_status = status;
request_state_->request_completed = true; request_state_->request_completed = true;
request_state_->expected_body_size = status.decoded_body_length;
request_state_->net_error = status.error_code; request_state_->net_error = status.error_code;
request_state_->loaded_from_cache = status.exists_in_cache; request_state_->loaded_from_cache = status.exists_in_cache;
// If |status| indicates success, but the body pipe was never received, the // If |status| indicates success, but the body pipe was never received, the
// URLLoader is violating the API contract. // URLLoader is violating the API contract.
if (request_state_->net_error == net::OK && !request_state_->body_started) if (request_state_->net_error == net::OK && !request_state_->body_started) {
request_state_->net_error = net::ERR_UNEXPECTED; request_state_->net_error = net::ERR_UNEXPECTED;
request_state_->completion_status = base::nullopt;
}
MaybeComplete(); MaybeComplete();
} }
...@@ -1693,6 +1707,8 @@ void SimpleURLLoaderImpl::OnMojoDisconnect() { ...@@ -1693,6 +1707,8 @@ void SimpleURLLoaderImpl::OnMojoDisconnect() {
request_state_->request_completed = true; request_state_->request_completed = true;
request_state_->net_error = net::ERR_FAILED; request_state_->net_error = net::ERR_FAILED;
request_state_->completion_status = base::nullopt;
// Wait to receive any pending data on the data pipe before reporting the // Wait to receive any pending data on the data pipe before reporting the
// failure. // failure.
MaybeComplete(); MaybeComplete();
...@@ -1733,17 +1749,21 @@ void SimpleURLLoaderImpl::MaybeComplete() { ...@@ -1733,17 +1749,21 @@ void SimpleURLLoaderImpl::MaybeComplete() {
// When OnCompleted sees a success result, still need to report an error if // When OnCompleted sees a success result, still need to report an error if
// the size isn't what was expected. // the size isn't what was expected.
if (request_state_->net_error == net::OK && if (request_state_->net_error == net::OK &&
request_state_->expected_body_size != request_state_->completion_status &&
request_state_->completion_status->decoded_body_length !=
request_state_->received_body_size) { request_state_->received_body_size) {
if (request_state_->expected_body_size > if (request_state_->completion_status->decoded_body_length >
request_state_->received_body_size) { request_state_->received_body_size) {
// The body pipe was closed before it received the entire body. // The body pipe was closed before it received the entire body.
request_state_->net_error = net::ERR_FAILED; request_state_->net_error = net::ERR_FAILED;
request_state_->completion_status = base::nullopt;
} else { } else {
// The caller provided more data through the pipe than it reported in // The caller provided more data through the pipe than it reported in
// URLLoaderCompletionStatus, so the URLLoader is violating the // URLLoaderCompletionStatus, so the URLLoader is violating the
// API contract. Just fail the request. // API contract. Just fail the request and delete the retained completion
// status.
request_state_->net_error = net::ERR_UNEXPECTED; request_state_->net_error = net::ERR_UNEXPECTED;
request_state_->completion_status = base::nullopt;
} }
} }
......
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
#include "base/callback_forward.h" #include "base/callback_forward.h"
#include "base/component_export.h" #include "base/component_export.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/optional.h"
#include "services/network/public/mojom/url_response_head.mojom.h" #include "services/network/public/mojom/url_response_head.mojom.h"
class GURL; class GURL;
...@@ -26,7 +27,7 @@ namespace base { ...@@ -26,7 +27,7 @@ namespace base {
class FilePath; class FilePath;
class TickClock; class TickClock;
class TimeDelta; class TimeDelta;
} } // namespace base
namespace net { namespace net {
class HttpResponseHeaders; class HttpResponseHeaders;
...@@ -348,6 +349,12 @@ class COMPONENT_EXPORT(NETWORK_CPP) SimpleURLLoader { ...@@ -348,6 +349,12 @@ class COMPONENT_EXPORT(NETWORK_CPP) SimpleURLLoader {
// caller of completion. // caller of completion.
virtual const mojom::URLResponseHead* ResponseInfo() const = 0; virtual const mojom::URLResponseHead* ResponseInfo() const = 0;
// The URLLoaderCompletionStatus for the request. Will be nullopt if the
// response never completed. May only be called once the loader has informed
// the caller of completion.
virtual const base::Optional<URLLoaderCompletionStatus>& CompletionStatus()
const = 0;
// Returns the URL that this loader is processing. May only be called once the // Returns the URL that this loader is processing. May only be called once the
// loader has informed the caller of completion. // loader has informed the caller of completion.
virtual const GURL& GetFinalURL() const = 0; virtual const GURL& GetFinalURL() const = 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment