Skip to content

Commit

Permalink
URLLoader: Remove unnecessary null checks, and an is_pending() check.
Browse files Browse the repository at this point in the history
A URLLoader creates a URLRequest on construction, and destroys it on
destruction. It can never have a null URLRequest. This wasn't always the
case, but is now.

As for the is_pending() check:  A URLRequest can only be canceled in two
places, and can't complete successfully when there's a pending callback.
The cancel calls are in places that only happen in response to different
pending callbacks, and only one pending callback can be outstanding at a
time, so the is_pending() check isn't needed. This is the only
invocation if URLRequest::is_pending() in production, so it can now be
made test only, or removed (The underlying value is still used in
DCHECKs, so we can't just remove it, unless we want to lose those).

Bug: 934009
Change-Id: Iec5eb4467a5cda1a8afc12f204086bbfc00ab7b2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1814884
Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698494}
  • Loading branch information
Matt Menke authored and Commit Bot committed Sep 20, 2019
1 parent 059e5f7 commit d1b73ad
Showing 1 changed file with 0 additions and 21 deletions.
21 changes: 0 additions & 21 deletions services/network/url_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -670,12 +670,6 @@ const void* const URLLoader::kUserDataKey = &URLLoader::kUserDataKey;
void URLLoader::FollowRedirect(const std::vector<std::string>& removed_headers,
const net::HttpRequestHeaders& modified_headers,
const base::Optional<GURL>& new_url) {
if (!url_request_) {
NotifyCompleted(net::ERR_UNEXPECTED);
// |this| may have been deleted.
return;
}

if (!deferred_redirect_url_) {
NOTREACHED();
return;
Expand Down Expand Up @@ -737,9 +731,6 @@ void URLLoader::PauseReadingBodyFromNet() {
<< (url_request_ ? url_request_->original_url().spec()
: "a URL that has completed loading or failed.");

if (!url_request_)
return;

// Please note that we pause reading body in all cases. Even if the URL
// request indicates that the response was cached, there could still be
// network activity involved. For example, the response was only partially
Expand Down Expand Up @@ -1217,8 +1208,6 @@ int URLLoader::OnHeadersReceived(
}

net::LoadState URLLoader::GetLoadStateForTesting() const {
if (!url_request_)
return net::LOAD_STATE_IDLE;
return url_request_->GetLoadState().state;
}

Expand Down Expand Up @@ -1266,9 +1255,6 @@ void URLLoader::OnAuthCredentials(
const base::Optional<net::AuthCredentials>& credentials) {
auth_challenge_responder_receiver_.reset();

if (!url_request_)
return;

if (!credentials.has_value()) {
url_request_->CancelAuth();
} else {
Expand Down Expand Up @@ -1466,10 +1452,6 @@ void URLLoader::OnUploadProgressACK() {

void URLLoader::OnSSLCertificateErrorResponse(const net::SSLInfo& ssl_info,
int net_error) {
// The request can be NULL if it was cancelled by the client.
if (!url_request_ || !url_request_->is_pending())
return;

if (net_error == net::OK) {
url_request_->ContinueDespiteLastError();
return;
Expand All @@ -1483,9 +1465,6 @@ bool URLLoader::HasDataPipe() const {
}

void URLLoader::RecordBodyReadFromNetBeforePausedIfNeeded() {
if (!url_request_)
return;

if (update_body_read_before_paused_)
body_read_before_paused_ = url_request_->GetRawBodyBytes();
if (body_read_before_paused_ != -1) {
Expand Down

0 comments on commit d1b73ad

Please sign in to comment.