Skip to content

Commit

Permalink
Fix all failed and canceled URLRequestStatuses without errors.
Browse files Browse the repository at this point in the history
As a step towards removing URLRequestStatus::Status, stop creating FAILED
and CANCELED URLRequestStatuses without supplying an error.

IO_PENDING is left for later as that'll want a bit more careful auditing of
error() callers.

BUG=490311

Review URL: https://codereview.chromium.org/1239993004

Cr-Commit-Position: refs/heads/master@{#341913}
  • Loading branch information
davidben authored and Commit bot committed Aug 5, 2015
1 parent aa0bbfd commit 5c411d9
Show file tree
Hide file tree
Showing 23 changed files with 132 additions and 131 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "base/message_loop/message_loop.h"
#include "chrome/browser/chromeos/attestation/attestation_ca_client.h"
#include "content/public/test/test_browser_thread.h"
#include "net/base/net_errors.h"
#include "net/http/http_status_code.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_fetcher.h"
Expand Down Expand Up @@ -39,10 +40,10 @@ class AttestationCAClientTest : public ::testing::Test {
}

protected:
void SendResponse(net::URLRequestStatus::Status status, int response_code) {
void SendResponse(net::Error error, int response_code) {
net::TestURLFetcher* fetcher = url_fetcher_factory_.GetFetcherByID(0);
CHECK(fetcher);
fetcher->set_status(net::URLRequestStatus(status, 0));
fetcher->set_status(net::URLRequestStatus::FromError(error));
fetcher->set_response_code(response_code);
fetcher->SetResponseString(fetcher->upload_data() + "_response");
fetcher->delegate()->OnURLFetchComplete(fetcher);
Expand All @@ -64,7 +65,7 @@ TEST_F(AttestationCAClientTest, EnrollRequest) {
"enroll",
base::Bind(&AttestationCAClientTest::DataCallback,
base::Unretained(this)));
SendResponse(net::URLRequestStatus::SUCCESS, net::HTTP_OK);
SendResponse(net::OK, net::HTTP_OK);

EXPECT_EQ(1, num_invocations_);
EXPECT_TRUE(result_);
Expand All @@ -77,7 +78,7 @@ TEST_F(AttestationCAClientTest, CertificateRequest) {
"certificate",
base::Bind(&AttestationCAClientTest::DataCallback,
base::Unretained(this)));
SendResponse(net::URLRequestStatus::SUCCESS, net::HTTP_OK);
SendResponse(net::OK, net::HTTP_OK);

EXPECT_EQ(1, num_invocations_);
EXPECT_TRUE(result_);
Expand All @@ -90,7 +91,7 @@ TEST_F(AttestationCAClientTest, CertificateRequestNetworkFailure) {
"certificate",
base::Bind(&AttestationCAClientTest::DataCallback,
base::Unretained(this)));
SendResponse(net::URLRequestStatus::FAILED, net::HTTP_OK);
SendResponse(net::ERR_FAILED, net::HTTP_OK);

EXPECT_EQ(1, num_invocations_);
EXPECT_FALSE(result_);
Expand All @@ -103,7 +104,7 @@ TEST_F(AttestationCAClientTest, CertificateRequestHttpError) {
"certificate",
base::Bind(&AttestationCAClientTest::DataCallback,
base::Unretained(this)));
SendResponse(net::URLRequestStatus::SUCCESS, net::HTTP_NOT_FOUND);
SendResponse(net::OK, net::HTTP_NOT_FOUND);

EXPECT_EQ(1, num_invocations_);
EXPECT_FALSE(result_);
Expand All @@ -117,7 +118,7 @@ TEST_F(AttestationCAClientTest, DeleteOnCallback) {
base::Bind(&AttestationCAClientTest::DeleteClientDataCallback,
base::Unretained(this),
client));
SendResponse(net::URLRequestStatus::SUCCESS, net::HTTP_OK);
SendResponse(net::OK, net::HTTP_OK);

EXPECT_EQ(1, num_invocations_);
EXPECT_TRUE(result_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include "content/public/test/test_browser_thread_bundle.h"
#include "google_apis/gaia/gaia_constants.h"
#include "google_apis/gaia/google_service_auth_error.h"
#include "net/base/net_errors.h"
#include "net/http/http_status_code.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_request_context_getter.h"
Expand Down Expand Up @@ -602,7 +603,7 @@ TEST_F(UserPolicySigninServiceTest, RegisterPolicyClientOAuthFailure) {
GoogleServiceAuthError::FromServiceError("fail"));
#else
net::TestURLFetcher* fetcher = url_factory_.GetFetcherByID(0);
fetcher->set_status(net::URLRequestStatus(net::URLRequestStatus::FAILED, -1));
fetcher->set_status(net::URLRequestStatus::FromError(net::ERR_FAILED));
fetcher->delegate()->OnURLFetchComplete(fetcher);
#endif

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/values.h"
#include "chrome/browser/supervised_user/child_accounts/family_info_fetcher.h"
#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
#include "net/base/net_errors.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_request_test_util.h"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -142,34 +143,30 @@ class FamilyInfoFetcherTest : public testing::Test,
return url_fetcher;
}

void SendResponse(net::URLRequestStatus::Status status,
const std::string& response) {
void SendResponse(net::Error error, const std::string& response) {
net::TestURLFetcher* url_fetcher = GetURLFetcher();
url_fetcher->set_status(net::URLRequestStatus(status, 0));
url_fetcher->set_status(net::URLRequestStatus::FromError(error));
url_fetcher->set_response_code(net::HTTP_OK);
url_fetcher->SetResponseString(response);
url_fetcher->delegate()->OnURLFetchComplete(url_fetcher);
}

void SendValidGetFamilyProfileResponse(
const FamilyInfoFetcher::FamilyProfile& family) {
SendResponse(net::URLRequestStatus::SUCCESS,
BuildGetFamilyProfileResponse(family));
SendResponse(net::OK, BuildGetFamilyProfileResponse(family));
}

void SendValidGetFamilyMembersResponse(
const std::vector<FamilyInfoFetcher::FamilyMember>& members) {
SendResponse(net::URLRequestStatus::SUCCESS,
BuildGetFamilyMembersResponse(members));
SendResponse(net::OK, BuildGetFamilyMembersResponse(members));
}

void SendInvalidGetFamilyProfileResponse() {
SendResponse(net::URLRequestStatus::SUCCESS,
BuildEmptyGetFamilyProfileResponse());
SendResponse(net::OK, BuildEmptyGetFamilyProfileResponse());
}

void SendFailedResponse() {
SendResponse(net::URLRequestStatus::CANCELED, std::string());
SendResponse(net::ERR_ABORTED, std::string());
}

base::MessageLoop message_loop_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "base/values.h"
#include "chrome/browser/supervised_user/child_accounts/permission_request_creator_apiary.h"
#include "components/signin/core/browser/fake_profile_oauth2_token_service.h"
#include "net/base/net_errors.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_request_test_util.h"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -70,25 +71,21 @@ class PermissionRequestCreatorApiaryTest : public testing::Test {
}

void SendResponse(int url_fetcher_id,
net::URLRequestStatus::Status status,
net::Error error,
const std::string& response) {
net::TestURLFetcher* url_fetcher = GetURLFetcher(url_fetcher_id);
url_fetcher->set_status(net::URLRequestStatus(status, 0));
url_fetcher->set_status(net::URLRequestStatus::FromError(error));
url_fetcher->set_response_code(net::HTTP_OK);
url_fetcher->SetResponseString(response);
url_fetcher->delegate()->OnURLFetchComplete(url_fetcher);
}

void SendValidResponse(int url_fetcher_id) {
SendResponse(url_fetcher_id,
net::URLRequestStatus::SUCCESS,
BuildResponse());
SendResponse(url_fetcher_id, net::OK, BuildResponse());
}

void SendFailedResponse(int url_fetcher_id) {
SendResponse(url_fetcher_id,
net::URLRequestStatus::CANCELED,
std::string());
SendResponse(url_fetcher_id, net::ERR_ABORTED, std::string());
}

MOCK_METHOD1(OnRequestCreated, void(bool success));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/thread_task_runner_handle.h"
#include "base/values.h"
#include "chrome/browser/supervised_user/experimental/supervised_user_async_url_checker.h"
#include "net/base/net_errors.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_request_test_util.h"
#include "testing/gmock/include/gmock/gmock.h"
Expand Down Expand Up @@ -94,22 +95,20 @@ class SupervisedUserAsyncURLCheckerTest : public testing::Test {
return url_fetcher;
}

void SendResponse(bool safe,
net::URLRequestStatus::Status status,
const std::string& response) {
void SendResponse(bool safe, net::Error error, const std::string& response) {
net::TestURLFetcher* url_fetcher = GetURLFetcher(safe);
url_fetcher->set_status(net::URLRequestStatus(status, 0));
url_fetcher->set_status(net::URLRequestStatus::FromError(error));
url_fetcher->set_response_code(net::HTTP_OK);
url_fetcher->SetResponseString(response);
url_fetcher->delegate()->OnURLFetchComplete(url_fetcher);
}

void SendValidResponse(bool safe, const GURL& url) {
SendResponse(safe, net::URLRequestStatus::SUCCESS, BuildResponse(url));
SendResponse(safe, net::OK, BuildResponse(url));
}

void SendFailedResponse(bool safe) {
SendResponse(safe, net::URLRequestStatus::CANCELED, std::string());
SendResponse(safe, net::ERR_ABORTED, std::string());
}

size_t next_url_;
Expand Down
5 changes: 3 additions & 2 deletions chromeos/login/auth/mock_url_fetchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/single_thread_task_runner.h"
#include "base/strings/stringprintf.h"
#include "base/thread_task_runner_handle.h"
#include "net/base/net_errors.h"
#include "net/http/http_status_code.h"
#include "net/url_request/url_fetcher.h"
#include "net/url_request/url_fetcher_delegate.h"
Expand Down Expand Up @@ -50,7 +51,7 @@ GotCanceledFetcher::GotCanceledFetcher(
net::URLFetcherDelegate* d)
: net::TestURLFetcher(0, url, d) {
set_url(url);
set_status(net::URLRequestStatus(net::URLRequestStatus::CANCELED, 0));
set_status(net::URLRequestStatus::FromError(net::ERR_ABORTED));
set_response_code(net::HTTP_FORBIDDEN);
}

Expand Down Expand Up @@ -86,7 +87,7 @@ FailFetcher::FailFetcher(bool success,
net::URLFetcherDelegate* d)
: net::TestURLFetcher(0, url, d) {
set_url(url);
set_status(net::URLRequestStatus(net::URLRequestStatus::FAILED, ECONNRESET));
set_status(net::URLRequestStatus::FromError(net::ERR_CONNECTION_RESET));
set_response_code(net::HTTP_OK);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "components/data_reduction_proxy/proto/client_config.pb.h"
#include "net/base/host_port_pair.h"
#include "net/base/load_flags.h"
#include "net/base/net_errors.h"
#include "net/base/url_util.h"
#include "net/http/http_response_headers.h"
#include "net/http/http_status_code.h"
Expand Down Expand Up @@ -301,7 +302,7 @@ void DataReductionProxyConfigServiceClient::RetrieveRemoteConfig() {
GetURLFetcherForConfig(config_service_url_, serialized_request);
if (!fetcher.get()) {
HandleResponse(std::string(),
net::URLRequestStatus(net::URLRequestStatus::CANCELED, 0),
net::URLRequestStatus::FromError(net::ERR_ABORTED),
net::URLFetcher::RESPONSE_CODE_INVALID);
return;
}
Expand Down
17 changes: 0 additions & 17 deletions components/domain_reliability/monitor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -339,23 +339,6 @@ TEST_F(DomainReliabilityMonitorTest, ClearContexts) {
EXPECT_EQ(0u, monitor_.contexts_size_for_testing());
}

// TODO(davidben): When https://crbug.com/490311 is resolved, this test can be
// removed.
TEST_F(DomainReliabilityMonitorTest, IgnoreSuccessError) {
RequestInfo request = MakeRequestInfo();
request.url = GURL("http://example/always_report");
request.status = net::URLRequestStatus(net::URLRequestStatus::SUCCESS,
net::ERR_QUIC_PROTOCOL_ERROR);
OnRequestLegComplete(request);

BeaconVector beacons;
context_->GetQueuedBeaconsForTesting(&beacons);
EXPECT_EQ(1u, beacons.size());
EXPECT_EQ(net::OK, beacons[0].chrome_error);

EXPECT_TRUE(CheckRequestCounts(kAlwaysReportIndex, 1u, 0u));
}

TEST_F(DomainReliabilityMonitorTest, WildcardMatchesSelf) {
DomainReliabilityContext* context = CreateAndAddContext("*.wildcard");

Expand Down
Loading

0 comments on commit 5c411d9

Please sign in to comment.