Change net/nqe and some url_request tests to use new URLRequest API

This cl splits the big CL with changes to net/ folder and
has changes only to net/nqe and some url_request tests
according to the changes to URLRequest API.

What the big cl does:
It modifies net/ clients that use URLRequest API as long as
URLRequest::Read returns int net errors and
URLRequest::Delegate and NetworkDelegate methods
(for example, OnResponseStarted or OnCompleted) have int
net_error in the arguments now.

The reason behind splitting the CL into small one is that
an android bot started to be unstable and unittests became
flaky. It was not possible to locate the problem and the
decision was to split the CL and upload small parts with a
6+ hours interval in order to make it possible to locate
the problematic code.

The big CL is located here -
https://codereview.chromium.org/2265873002/

BUG=423484

Review-Url: https://codereview.chromium.org/2332643002
Cr-Commit-Position: refs/heads/master@{#418829}

net/nqe/network_quality_estimator.cc

@@ -807,8 +807,8 @@ void NetworkQualityEstimator::RecordAccuracyAfterMainFrame(
   }
 }
 
-void NetworkQualityEstimator::NotifyRequestCompleted(
-    const URLRequest& request) {
+void NetworkQualityEstimator::NotifyRequestCompleted(const URLRequest& request,
+                                                     int net_error) {
   TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("net"),
                "NetworkQualityEstimator::NotifyRequestCompleted");
   DCHECK(thread_checker_.CalledOnValidThread());
@@ -817,11 +817,11 @@ void NetworkQualityEstimator::NotifyRequestCompleted(
     return;
 
   throughput_analyzer_->NotifyRequestCompleted(request);
-  RecordCorrelationMetric(request);
+  RecordCorrelationMetric(request, net_error);
 }
 
-void NetworkQualityEstimator::RecordCorrelationMetric(
-    const URLRequest& request) const {
+void NetworkQualityEstimator::RecordCorrelationMetric(const URLRequest& request,
+                                                      int net_error) const {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // The histogram is recorded with probability
@@ -846,7 +846,7 @@ void NetworkQualityEstimator::RecordCorrelationMetric(
   }
 
   // Record UMA only for successful requests that have completed.
-  if (!request.status().is_success() || request.status().is_io_pending())
+  if (net_error != OK)
     return;
   if (request.GetResponseCode() != HTTP_OK)
     return;

net/nqe/network_quality_estimator.h

@@ -180,7 +180,7 @@ class NET_EXPORT NetworkQualityEstimator
 
   // Notifies NetworkQualityEstimator that the response body of |request| has
   // been received.
-  void NotifyRequestCompleted(const URLRequest& request);
+  void NotifyRequestCompleted(const URLRequest& request, int net_error);
 
   // Notifies NetworkQualityEstimator that |request| will be destroyed.
   void NotifyURLRequestDestroyed(const URLRequest& request);
@@ -479,7 +479,7 @@ class NET_EXPORT NetworkQualityEstimator
   // Records a correlation metric that can be used for computing the correlation
   // between HTTP-layer RTT, transport-layer RTT, throughput and the time
   // taken to complete |request|.
-  void RecordCorrelationMetric(const URLRequest& request) const;
+  void RecordCorrelationMetric(const URLRequest& request, int net_error) const;
 
   // Returns true if transport RTT should be used for computing the effective
   // connection type.

net/nqe/throughput_analyzer_unittest.cc

@@ -237,4 +237,4 @@ TEST(ThroughputAnalyzerTest, TestThroughputWithNetworkRequestsOverlap) {
 
 }  // namespace nqe
 
-}  // namespace net
\ No newline at end of file
+}  // namespace net

net/url_request/url_request_ftp_job_unittest.cc

@@ -310,7 +310,7 @@ TEST_F(URLRequestFtpJobTest, FtpProxyRequest) {
   // The TestDelegate will by default quit the message loop on completion.
   base::RunLoop().Run();
 
-  EXPECT_TRUE(url_request->status().is_success());
+  EXPECT_THAT(request_delegate.request_status(), IsOk());
   EXPECT_TRUE(url_request->proxy_server().Equals(
       HostPortPair::FromString("localhost:80")));
   EXPECT_EQ(1, network_delegate()->completed_requests());
@@ -406,7 +406,7 @@ TEST_F(URLRequestFtpJobTest, FtpProxyRequestNeedProxyAuthNoCredentials) {
   // The TestDelegate will by default quit the message loop on completion.
   base::RunLoop().Run();
 
-  EXPECT_TRUE(url_request->status().is_success());
+  EXPECT_THAT(request_delegate.request_status(), IsOk());
   EXPECT_TRUE(url_request->proxy_server().Equals(
       HostPortPair::FromString("localhost:80")));
   EXPECT_EQ(1, network_delegate()->completed_requests());
@@ -452,7 +452,7 @@ TEST_F(URLRequestFtpJobTest, FtpProxyRequestNeedProxyAuthWithCredentials) {
   // The TestDelegate will by default quit the message loop on completion.
   base::RunLoop().Run();
 
-  EXPECT_TRUE(url_request->status().is_success());
+  EXPECT_THAT(request_delegate.request_status(), IsOk());
   EXPECT_EQ(1, network_delegate()->completed_requests());
   EXPECT_EQ(0, network_delegate()->error_count());
   EXPECT_TRUE(request_delegate.auth_required_called());
@@ -485,7 +485,7 @@ TEST_F(URLRequestFtpJobTest, FtpProxyRequestNeedServerAuthNoCredentials) {
   // The TestDelegate will by default quit the message loop on completion.
   base::RunLoop().Run();
 
-  EXPECT_TRUE(url_request->status().is_success());
+  EXPECT_THAT(request_delegate.request_status(), IsOk());
   EXPECT_EQ(1, network_delegate()->completed_requests());
   EXPECT_EQ(0, network_delegate()->error_count());
   EXPECT_TRUE(request_delegate.auth_required_called());
@@ -529,7 +529,7 @@ TEST_F(URLRequestFtpJobTest, FtpProxyRequestNeedServerAuthWithCredentials) {
   // The TestDelegate will by default quit the message loop on completion.
   base::RunLoop().Run();
 
-  EXPECT_TRUE(url_request->status().is_success());
+  EXPECT_THAT(request_delegate.request_status(), IsOk());
   EXPECT_EQ(1, network_delegate()->completed_requests());
   EXPECT_EQ(0, network_delegate()->error_count());
   EXPECT_TRUE(request_delegate.auth_required_called());
@@ -603,7 +603,6 @@ TEST_F(URLRequestFtpJobTest, FtpProxyRequestNeedProxyAndServerAuth) {
   // Run until server auth is requested.
   base::RunLoop().Run();
 
-  EXPECT_TRUE(url_request->status().is_success());
   EXPECT_EQ(0, network_delegate()->completed_requests());
   EXPECT_EQ(0, network_delegate()->error_count());
   url_request->SetAuth(
@@ -612,7 +611,7 @@ TEST_F(URLRequestFtpJobTest, FtpProxyRequestNeedProxyAndServerAuth) {
   // The TestDelegate will by default quit the message loop on completion.
   base::RunLoop().Run();
 
-  EXPECT_TRUE(url_request->status().is_success());
+  EXPECT_THAT(request_delegate.request_status(), IsOk());
   EXPECT_EQ(1, network_delegate()->completed_requests());
   EXPECT_EQ(0, network_delegate()->error_count());
   EXPECT_TRUE(request_delegate.auth_required_called());
@@ -643,7 +642,7 @@ TEST_F(URLRequestFtpJobTest, FtpProxyRequestDoNotSaveCookies) {
   // The TestDelegate will by default quit the message loop on completion.
   base::RunLoop().Run();
 
-  EXPECT_TRUE(url_request->status().is_success());
+  EXPECT_THAT(request_delegate.request_status(), IsOk());
   EXPECT_EQ(1, network_delegate()->completed_requests());
   EXPECT_EQ(0, network_delegate()->error_count());
 

net/url_request/url_request_http_job.cc

@@ -1587,7 +1587,8 @@ void URLRequestHttpJob::DoneWithRequest(CompletionCause reason) {
     NetworkQualityEstimator* network_quality_estimator =
         request()->context()->network_quality_estimator();
     if (network_quality_estimator)
-      network_quality_estimator->NotifyRequestCompleted(*request());
+      network_quality_estimator->NotifyRequestCompleted(
+          *request(), request_->status().error());
   }
 
   RecordPerfHistograms(reason);

net/url_request/url_request_http_job_unittest.cc

@@ -163,7 +163,7 @@ TEST_F(URLRequestHttpJobWithMockSocketsTest,
   ASSERT_TRUE(request->is_pending());
   base::RunLoop().Run();
 
-  EXPECT_TRUE(request->status().is_success());
+  EXPECT_THAT(delegate.request_status(), IsOk());
   EXPECT_EQ(12, request->received_response_content_length());
   EXPECT_EQ(CountWriteBytes(writes, arraysize(writes)),
             request->GetTotalSentBytes());
@@ -192,7 +192,7 @@ TEST_F(URLRequestHttpJobWithMockSocketsTest,
   ASSERT_TRUE(request->is_pending());
   base::RunLoop().Run();
 
-  EXPECT_TRUE(request->status().is_success());
+  EXPECT_THAT(delegate.request_status(), IsOk());
   EXPECT_EQ(12, request->received_response_content_length());
   EXPECT_EQ(CountWriteBytes(writes, arraysize(writes)),
             request->GetTotalSentBytes());
@@ -223,7 +223,7 @@ TEST_F(URLRequestHttpJobWithMockSocketsTest, TestContentLengthFailedRequest) {
   ASSERT_TRUE(request->is_pending());
   base::RunLoop().Run();
 
-  EXPECT_EQ(URLRequestStatus::FAILED, request->status().status());
+  EXPECT_THAT(delegate.request_status(), IsError(ERR_FAILED));
   EXPECT_EQ(12, request->received_response_content_length());
   EXPECT_EQ(CountWriteBytes(writes, arraysize(writes)),
             request->GetTotalSentBytes());
@@ -255,7 +255,7 @@ TEST_F(URLRequestHttpJobWithMockSocketsTest,
   request->Start();
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(URLRequestStatus::CANCELED, request->status().status());
+  EXPECT_THAT(delegate.request_status(), IsError(ERR_ABORTED));
   EXPECT_EQ(12, request->received_response_content_length());
   EXPECT_EQ(CountWriteBytes(writes, arraysize(writes)),
             request->GetTotalSentBytes());
@@ -428,7 +428,7 @@ TEST_F(URLRequestHttpJobWithMockSocketsTest,
   ASSERT_TRUE(request->is_pending());
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(OK, request->status().error());
+  EXPECT_THAT(delegate.request_status(), IsOk());
   EXPECT_EQ(12, request->received_response_content_length());
   // Should not include the redirect.
   EXPECT_EQ(CountWriteBytes(final_writes, arraysize(final_writes)),
@@ -460,7 +460,7 @@ TEST_F(URLRequestHttpJobWithMockSocketsTest,
   request->Start();
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(URLRequestStatus::CANCELED, request->status().status());
+  EXPECT_THAT(delegate.request_status(), IsError(ERR_ABORTED));
   EXPECT_EQ(0, request->received_response_content_length());
   EXPECT_EQ(CountWriteBytes(writes, arraysize(writes)),
             request->GetTotalSentBytes());
@@ -485,7 +485,7 @@ TEST_F(URLRequestHttpJobWithMockSocketsTest,
   request->Cancel();
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_EQ(URLRequestStatus::CANCELED, request->status().status());
+  EXPECT_THAT(delegate.request_status(), IsError(ERR_ABORTED));
   EXPECT_EQ(0, request->received_response_content_length());
   EXPECT_EQ(0, request->GetTotalSentBytes());
   EXPECT_EQ(0, request->GetTotalReceivedBytes());
@@ -506,7 +506,7 @@ TEST_F(URLRequestHttpJobTest, TestCancelWhileReadingCookies) {
   request->Cancel();
   base::RunLoop().Run();
 
-  EXPECT_EQ(URLRequestStatus::CANCELED, request->status().status());
+  EXPECT_THAT(delegate.request_status(), IsError(ERR_ABORTED));
 }
 
 // Make sure that SetPriority actually sets the URLRequestHttpJob's
@@ -693,7 +693,7 @@ TEST_F(URLRequestHttpJobWithSdchSupportTest, GetDictionary) {
   request->Start();
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_TRUE(request->status().is_success());
+  EXPECT_THAT(delegate.request_status(), IsOk());
 
   // Second response should be from cache without notification of SdchObserver
   TestDelegate delegate2;
@@ -702,7 +702,7 @@ TEST_F(URLRequestHttpJobWithSdchSupportTest, GetDictionary) {
   request2->Start();
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_TRUE(request->status().is_success());
+  EXPECT_THAT(delegate2.request_status(), IsOk());
 
   // Cleanup manager.
   sdch_manager.RemoveObserver(&sdch_observer);
@@ -739,7 +739,7 @@ TEST_F(URLRequestHttpJobWithBrotliSupportTest, NoBrotliAdvertisementOverHttp) {
   request->Start();
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_TRUE(request->status().is_success());
+  EXPECT_THAT(delegate.request_status(), IsOk());
   EXPECT_EQ(12, request->received_response_content_length());
   EXPECT_EQ(CountWriteBytes(writes, arraysize(writes)),
             request->GetTotalSentBytes());
@@ -774,7 +774,7 @@ TEST_F(URLRequestHttpJobWithBrotliSupportTest, BrotliAdvertisement) {
   request->Start();
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_TRUE(request->status().is_success());
+  EXPECT_THAT(delegate.request_status(), IsOk());
   EXPECT_EQ(12, request->received_response_content_length());
   EXPECT_EQ(CountWriteBytes(writes, arraysize(writes)),
             request->GetTotalSentBytes());
@@ -924,8 +924,7 @@ class FakeWebSocketHandshakeStream : public WebSocketHandshakeStreamBase {
 TEST_F(URLRequestHttpJobWebSocketTest, RejectedWithoutCreateHelper) {
   req_->Start();
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(URLRequestStatus::FAILED, req_->status().status());
-  EXPECT_THAT(req_->status().error(), IsError(ERR_DISALLOWED_URL_SCHEME));
+  EXPECT_THAT(delegate_.request_status(), IsError(ERR_DISALLOWED_URL_SCHEME));
 }
 
 TEST_F(URLRequestHttpJobWebSocketTest, CreateHelperPassedThrough) {
@@ -942,7 +941,7 @@ TEST_F(URLRequestHttpJobWebSocketTest, CreateHelperPassedThrough) {
   req_->SetLoadFlags(LOAD_DISABLE_CACHE);
   req_->Start();
   base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(URLRequestStatus::IO_PENDING, req_->status().status());
+  EXPECT_THAT(delegate_.request_status(), IsError(ERR_IO_PENDING));
   EXPECT_TRUE(fake_handshake_stream->initialize_stream_was_called());
 }