Commit d3e5dafc authored by maksim.sisov's avatar maksim.sisov Committed by Commit bot

Use modifed URLRequest API in net/url_request context builder, data job fuzzer and file dir test

This cl splits the big CL with changes to net/ folder and
has changes only to net/cert_net 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

Committed: https://crrev.com/a74d3cbc572fa1c85cc2c0434a9f8e2f9cba0cd9
Review-Url: https://codereview.chromium.org/2330863002
Cr-Original-Commit-Position: refs/heads/master@{#418014}
Cr-Commit-Position: refs/heads/master@{#419394}
parent fff167cf
...@@ -94,9 +94,9 @@ class BasicNetworkDelegate : public NetworkDelegateImpl { ...@@ -94,9 +94,9 @@ class BasicNetworkDelegate : public NetworkDelegateImpl {
void OnBeforeRedirect(URLRequest* request, void OnBeforeRedirect(URLRequest* request,
const GURL& new_location) override {} const GURL& new_location) override {}
void OnResponseStarted(URLRequest* request) override {} void OnResponseStarted(URLRequest* request, int net_error) override {}
void OnCompleted(URLRequest* request, bool started) override {} void OnCompleted(URLRequest* request, bool started, int net_error) override {}
void OnURLRequestDestroyed(URLRequest* request) override {} void OnURLRequestDestroyed(URLRequest* request) override {}
......
...@@ -99,7 +99,7 @@ class URLRequestDataJobFuzzerHarness : public net::URLRequest::Delegate { ...@@ -99,7 +99,7 @@ class URLRequestDataJobFuzzerHarness : public net::URLRequest::Delegate {
} }
void ReadFromRequest(net::URLRequest* request) { void ReadFromRequest(net::URLRequest* request) {
bool sync = false; int bytes_read = 0;
do { do {
// If possible, pop the next read size. If none exists, then this should // If possible, pop the next read size. If none exists, then this should
// be the last call to Read. // be the last call to Read.
...@@ -110,11 +110,10 @@ class URLRequestDataJobFuzzerHarness : public net::URLRequest::Delegate { ...@@ -110,11 +110,10 @@ class URLRequestDataJobFuzzerHarness : public net::URLRequest::Delegate {
read_lengths_.pop_back(); read_lengths_.pop_back();
} }
int bytes_read = 0; bytes_read = request->Read(buf_.get(), read_size);
sync = request->Read(buf_.get(), read_size, &bytes_read); } while (bytes_read > 0);
} while (sync);
if (!request->status().is_io_pending()) if (bytes_read != net::ERR_IO_PENDING)
QuitLoop(); QuitLoop();
} }
...@@ -130,21 +129,23 @@ class URLRequestDataJobFuzzerHarness : public net::URLRequest::Delegate { ...@@ -130,21 +129,23 @@ class URLRequestDataJobFuzzerHarness : public net::URLRequest::Delegate {
void OnSSLCertificateError(net::URLRequest* request, void OnSSLCertificateError(net::URLRequest* request,
const net::SSLInfo& ssl_info, const net::SSLInfo& ssl_info,
bool fatal) override {} bool fatal) override {}
void OnResponseStarted(net::URLRequest* request) override { void OnResponseStarted(net::URLRequest* request, int net_error) override {
DCHECK(!request->status().is_io_pending());
DCHECK(buf_.get()); DCHECK(buf_.get());
DCHECK(read_loop_); DCHECK(read_loop_);
if (request->status().is_success()) { DCHECK_NE(net::ERR_IO_PENDING, net_error);
if (net_error == net::OK) {
ReadFromRequest(request); ReadFromRequest(request);
} else { } else {
QuitLoop(); QuitLoop();
} }
} }
void OnReadCompleted(net::URLRequest* request, int bytes_read) override { void OnReadCompleted(net::URLRequest* request, int bytes_read) override {
DCHECK(!request->status().is_io_pending()); DCHECK_NE(net::ERR_IO_PENDING, bytes_read);
DCHECK(buf_.get()); DCHECK(buf_.get());
DCHECK(read_loop_); DCHECK(read_loop_);
if (request->status().is_success() && bytes_read > 0) {
if (bytes_read > 0) {
ReadFromRequest(request); ReadFromRequest(request);
} else { } else {
QuitLoop(); QuitLoop();
......
...@@ -125,13 +125,12 @@ TEST_F(URLRequestFileDirTest, ListCompletionOnNoPending) { ...@@ -125,13 +125,12 @@ TEST_F(URLRequestFileDirTest, ListCompletionOnNoPending) {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
ASSERT_TRUE(delegate_.got_response_started()); ASSERT_TRUE(delegate_.got_response_started());
int bytes_read = 0; int bytes_read = request->Read(buffer_.get(), kBufferSize);
EXPECT_FALSE(request->Read(buffer_.get(), kBufferSize, &bytes_read));
// The URLRequestFileDirJobShould return the cached read error synchronously. // The URLRequestFileDirJobShould return the cached read error synchronously.
// If it's not returned synchronously, the code path this is intended to test // If it's not returned synchronously, the code path this is intended to test
// was not executed. // was not executed.
EXPECT_THAT(request->status().ToNetError(), IsError(ERR_FILE_NOT_FOUND)); EXPECT_THAT(bytes_read, IsError(ERR_FILE_NOT_FOUND));
} }
// Test the case where reading the response completes synchronously. // Test the case where reading the response completes synchronously.
...@@ -154,12 +153,9 @@ TEST_F(URLRequestFileDirTest, DirectoryWithASingleFileSync) { ...@@ -154,12 +153,9 @@ TEST_F(URLRequestFileDirTest, DirectoryWithASingleFileSync) {
// entire directory listing and cached it. // entire directory listing and cached it.
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
int bytes_read = 0;
// This will complete synchronously, since the URLRequetsFileDirJob had // This will complete synchronously, since the URLRequetsFileDirJob had
// directory listing cached in memory. // directory listing cached in memory.
EXPECT_TRUE(request->Read(buffer_.get(), kBufferSize, &bytes_read)); int bytes_read = request->Read(buffer_.get(), kBufferSize);
EXPECT_EQ(URLRequestStatus::SUCCESS, request->status().status());
ASSERT_GT(bytes_read, 0); ASSERT_GT(bytes_read, 0);
ASSERT_LE(bytes_read, kBufferSize); ASSERT_LE(bytes_read, kBufferSize);
......
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