Skip to content

Commit

Permalink
Switch net::test_server::SendCompleteCallback to a OnceClosure
Browse files Browse the repository at this point in the history
There were also two places where custom HttpResponses could just use
one of the built-in ones.

Bug: 1007815
Change-Id: I97dad08849c1ca0db04b200c2168ba4ff5939e09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1880529
Commit-Queue: David Benjamin <davidben@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Auto-Submit: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719716}
  • Loading branch information
davidben authored and Commit Bot committed Nov 27, 2019
1 parent 96681c2 commit 3462787
Show file tree
Hide file tree
Showing 26 changed files with 171 additions and 201 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <algorithm>

#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/containers/circular_deque.h"
#include "base/files/file_util.h"
#include "base/guid.h"
Expand Down Expand Up @@ -1789,9 +1790,8 @@ class CustomResponse : public net::test_server::HttpResponse {
first_request_(first_request) {}
~CustomResponse() override {}

void SendResponse(
const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done) override {
void SendResponse(const net::test_server::SendBytesCallback& send,
net::test_server::SendCompleteCallback done) override {
std::string response(
"HTTP/1.1 200 OK\r\n"
"Content-type: application/octet-stream\r\n"
Expand All @@ -1802,7 +1802,7 @@ class CustomResponse : public net::test_server::HttpResponse {
if (first_request_) {
*callback_ = std::move(done);
*task_runner_ = base::ThreadTaskRunnerHandle::Get().get();
send.Run(response, base::BindRepeating([]() {}));
send.Run(response, base::DoNothing());
} else {
send.Run(response, std::move(done));
}
Expand Down Expand Up @@ -1858,7 +1858,8 @@ IN_PROC_BROWSER_TEST_F(DownloadExtensionTest,

item->SimulateErrorForTesting(
download::DOWNLOAD_INTERRUPT_REASON_NETWORK_FAILED);
embedded_test_server_io_runner->PostTask(FROM_HERE, complete_callback);
embedded_test_server_io_runner->PostTask(FROM_HERE,
std::move(complete_callback));

ASSERT_TRUE(WaitFor(downloads::OnChanged::kEventName,
base::StringPrintf("[{\"id\":%d,"
Expand Down
19 changes: 3 additions & 16 deletions chrome/browser/safe_browsing/safe_browsing_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,24 +142,12 @@ const char kNeverCompletesPath[] = "/never_completes";
const char kPrefetchMalwarePage[] = "/safe_browsing/prefetch_malware.html";
const char kBillingInterstitialPage[] = "/safe_browsing/billing.html";

// TODO(ricea): Use net::test_server::HungResponse instead.
class NeverCompletingHttpResponse : public net::test_server::HttpResponse {
public:
~NeverCompletingHttpResponse() override {}

void SendResponse(
const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done) override {
// Do nothing. |done| is never called.
}
};

std::unique_ptr<net::test_server::HttpResponse> HandleNeverCompletingRequests(
const net::test_server::HttpRequest& request) {
if (!base::StartsWith(request.relative_url, kNeverCompletesPath,
base::CompareCase::SENSITIVE))
return nullptr;
return std::make_unique<NeverCompletingHttpResponse>();
return std::make_unique<net::test_server::HungResponse>();
}

// This is not a proper WebSocket server. It does the minimum necessary to make
Expand All @@ -178,9 +166,8 @@ class QuasiWebSocketHttpResponse : public net::test_server::HttpResponse {
}
~QuasiWebSocketHttpResponse() override {}

void SendResponse(
const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done) override {
void SendResponse(const net::test_server::SendBytesCallback& send,
net::test_server::SendCompleteCallback done) override {
const auto response_headers = base::StringPrintf(
"HTTP/1.1 101 WebSocket Protocol Handshake\r\n"
"Upgrade: WebSocket\r\n"
Expand Down
7 changes: 3 additions & 4 deletions chrome/browser/signin/dice_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,11 @@ class BlockedHttpResponse : public net::test_server::BasicHttpResponse {
base::OnceCallback<void(base::OnceClosure)> callback)
: callback_(std::move(callback)) {}

void SendResponse(
const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done) override {
void SendResponse(const net::test_server::SendBytesCallback& send,
net::test_server::SendCompleteCallback done) override {
// Called on the IO thread to unblock the response.
base::OnceClosure unblock_io_thread =
base::BindOnce(send, ToResponseString(), done);
base::BindOnce(send, ToResponseString(), std::move(done));
// Unblock the response from any thread by posting a task to the IO thread.
base::OnceClosure unblock_any_thread =
base::BindOnce(base::IgnoreResult(&base::TaskRunner::PostTask),
Expand Down
4 changes: 2 additions & 2 deletions content/browser/download/download_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -784,8 +784,8 @@ class TestRequestPauseHandler {
}

private:
void OnPauseHandler(const base::Closure& resume_callback) {
resume_callback_ = resume_callback;
void OnPauseHandler(base::OnceClosure resume_callback) {
resume_callback_ = std::move(resume_callback);
if (run_loop_.running())
run_loop_.Quit();
}
Expand Down
17 changes: 8 additions & 9 deletions content/browser/renderer_host/render_process_host_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,12 @@ class DelayedHttpResponseWithResolver final
base::BindOnce(&Resolver::ResolveInServerTaskRunner, this));
}

void Add(const ResponseWithCallbacks& response) {
void Add(ResponseWithCallbacks response) {
base::AutoLock auto_lock(lock_);

if (resolved_) {
response.send_callback.Run(response.response_string,
response.done_callback);
std::move(response.done_callback));
return;
}

Expand All @@ -99,16 +99,16 @@ class DelayedHttpResponseWithResolver final
task_runner_ = std::move(task_runner);
}

responses_with_callbacks_.push_back(response);
responses_with_callbacks_.push_back(std::move(response));
}

private:
void ResolveInServerTaskRunner() {
auto responses_with_callbacks = std::move(responses_with_callbacks_);
for (const auto& response_with_callbacks : responses_with_callbacks) {
for (auto& response_with_callbacks : responses_with_callbacks) {
response_with_callbacks.send_callback.Run(
response_with_callbacks.response_string,
response_with_callbacks.done_callback);
std::move(response_with_callbacks.done_callback));
}
}

Expand All @@ -130,10 +130,9 @@ class DelayedHttpResponseWithResolver final
DelayedHttpResponseWithResolver& operator=(
const DelayedHttpResponseWithResolver&) = delete;

void SendResponse(
const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done) override {
resolver_->Add({send, done, ToResponseString()});
void SendResponse(const net::test_server::SendBytesCallback& send,
net::test_server::SendCompleteCallback done) override {
resolver_->Add({send, std::move(done), ToResponseString()});
}

private:
Expand Down
7 changes: 3 additions & 4 deletions content/browser/service_worker/service_worker_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2121,10 +2121,9 @@ class ServiceWorkerNavigationPreloadTest : public ServiceWorkerBrowserTest {
: response_(response) {}
~CustomResponse() override {}

void SendResponse(
const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done) override {
send.Run(response_, done);
void SendResponse(const net::test_server::SendBytesCallback& send,
net::test_server::SendCompleteCallback done) override {
send.Run(response_, std::move(done));
}

private:
Expand Down
18 changes: 9 additions & 9 deletions content/browser/site_per_process_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9339,10 +9339,11 @@ class RequestDelayingSitePerProcessBrowserTest
private:
// Called on the test server's thread.
void AddDelayedResponse(const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done) {
net::test_server::SendCompleteCallback done) {
// Just create a closure that closes the socket without sending a response.
// This will propagate an error to the underlying request.
send_response_closures_.push_back(base::Bind(send, "", done));
send_response_closures_.push_back(
base::BindOnce(send, "", std::move(done)));
}

// Custom embedded test server handler. Looks for requests matching
Expand Down Expand Up @@ -9376,8 +9377,8 @@ class RequestDelayingSitePerProcessBrowserTest
if (it.second > 0)
return;
}
for (const auto it : send_response_closures_) {
it.Run();
for (auto& it : send_response_closures_) {
std::move(it).Run();
}
}

Expand All @@ -9388,10 +9389,9 @@ class RequestDelayingSitePerProcessBrowserTest
explicit DelayedResponse(
RequestDelayingSitePerProcessBrowserTest* test_harness)
: test_harness_(test_harness) {}
void SendResponse(
const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done) override {
test_harness_->AddDelayedResponse(send, done);
void SendResponse(const net::test_server::SendBytesCallback& send,
net::test_server::SendCompleteCallback done) override {
test_harness_->AddDelayedResponse(send, std::move(done));
}

private:
Expand All @@ -9402,7 +9402,7 @@ class RequestDelayingSitePerProcessBrowserTest

// Set of closures to call which will complete delayed requests. May only be
// modified on the test_server_'s thread.
std::vector<base::Closure> send_response_closures_;
std::vector<base::OnceClosure> send_response_closures_;

// Map from URL paths to the number of requests to delay for that particular
// path. Initialized on the UI thread but modified and read on the test
Expand Down
30 changes: 18 additions & 12 deletions content/public/test/slow_download_http_response.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include "content/public/test/slow_download_http_response.h"

#include <utility>

#include "base/bind.h"
#include "base/strings/stringprintf.h"
#include "base/threading/thread_task_runner_handle.h"
Expand All @@ -15,33 +17,36 @@ namespace {
static bool g_should_finish_download = false;

void SendResponseBodyDone(const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done);
net::test_server::SendCompleteCallback done);

// Sends the response body with the given size.
void SendResponseBody(const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done,
net::test_server::SendCompleteCallback done,
bool finish_download) {
int data_size = finish_download
? SlowDownloadHttpResponse::kSecondDownloadSize
: SlowDownloadHttpResponse::kFirstDownloadSize;
std::string response(data_size, '*');

if (finish_download)
send.Run(response, done);
else
send.Run(response, base::Bind(&SendResponseBodyDone, send, done));
if (finish_download) {
send.Run(response, std::move(done));
} else {
send.Run(response,
base::BindOnce(&SendResponseBodyDone, send, std::move(done)));
}
}

// Called when the response body was sucessfully sent.
void SendResponseBodyDone(const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done) {
net::test_server::SendCompleteCallback done) {
if (g_should_finish_download) {
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, base::BindOnce(&SendResponseBody, send, done, true),
FROM_HERE,
base::BindOnce(&SendResponseBody, send, std::move(done), true),
base::TimeDelta::FromMilliseconds(100));
} else {
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, base::BindOnce(&SendResponseBodyDone, send, done),
FROM_HERE, base::BindOnce(&SendResponseBodyDone, send, std::move(done)),
base::TimeDelta::FromMilliseconds(100));
}
}
Expand Down Expand Up @@ -80,15 +85,15 @@ SlowDownloadHttpResponse::~SlowDownloadHttpResponse() = default;

void SlowDownloadHttpResponse::SendResponse(
const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done) {
net::test_server::SendCompleteCallback done) {
std::string response;
response.append("HTTP/1.1 200 OK\r\n");
if (base::LowerCaseEqualsASCII(kFinishDownloadUrl, url_)) {
response.append("Content-type: text/plain\r\n");
response.append("\r\n");

g_should_finish_download = true;
send.Run(response, done);
send.Run(response, std::move(done));
} else {
response.append("Content-type: application/octet-stream\r\n");
response.append("Cache-Control: max-age=0\r\n");
Expand All @@ -98,7 +103,8 @@ void SlowDownloadHttpResponse::SendResponse(
"Content-Length: %d\r\n", kFirstDownloadSize + kSecondDownloadSize));
}
response.append("\r\n");
send.Run(response, base::Bind(&SendResponseBody, send, done, false));
send.Run(response,
base::BindOnce(&SendResponseBody, send, std::move(done), false));
}
}

Expand Down
5 changes: 2 additions & 3 deletions content/public/test/slow_download_http_response.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,8 @@ class SlowDownloadHttpResponse : public net::test_server::HttpResponse {
~SlowDownloadHttpResponse() override;

// net::test_server::HttpResponse implementations.
void SendResponse(
const net::test_server::SendBytesCallback& send,
const net::test_server::SendCompleteCallback& done) override;
void SendResponse(const net::test_server::SendBytesCallback& send,
net::test_server::SendCompleteCallback done) override;

private:
std::string url_;
Expand Down
Loading

0 comments on commit 3462787

Please sign in to comment.