Commit 6cdec9f2 authored by Tsuyoshi Horo's avatar Tsuyoshi Horo Committed by Commit Bot

Make a deep copy of ResourceResponseHead to send NavigationPreload info to DevTools

HttpRawRequestResponseInfo in ResourceResponseInfo is not thread safe.
So if the shared ResourceResponseInfo is deleted on the UI thread,
DCHECK(CalledOnValidSequence()) in RefCountedBase::Release() fails.

To avoid this, this CL makes a deep copy of ResourceResponseHead before passing
it cross-thread.

This CL also introduces |devtools_enabled_| in DelegatingURLLoaderClient to avoid
unnecessary operations.

Bug: 836680
Change-Id: I5cee9b2da8b6183b660e8fc6375d9ee7ab2da343
Reviewed-on: https://chromium-review.googlesource.com/1027390Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553505}
parent 6fa9e99a
......@@ -103,13 +103,13 @@ void NotifyNavigationPreloadRequestSentOnUI(
void NotifyNavigationPreloadResponseReceivedOnUI(
const GURL& url,
const network::ResourceResponseHead& head,
scoped_refptr<network::ResourceResponse> response,
const std::pair<int, int>& worker_id,
const std::string& request_id) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
ServiceWorkerDevToolsManager::GetInstance()
->NavigationPreloadResponseReceived(worker_id.first, worker_id.second,
request_id, url, head);
request_id, url, response->head);
}
void NotifyNavigationPreloadCompletedOnUI(
......@@ -134,9 +134,12 @@ class DelegatingURLLoaderClient final : public network::mojom::URLLoaderClient {
: binding_(this),
client_(std::move(client)),
on_response_(std::move(on_response)),
url_(request.url) {
url_(request.url),
devtools_enabled_(request.report_raw_headers) {
if (!devtools_enabled_)
return;
AddDevToolsCallback(
base::Bind(&NotifyNavigationPreloadRequestSentOnUI, request));
base::BindOnce(&NotifyNavigationPreloadRequestSentOnUI, request));
}
~DelegatingURLLoaderClient() override {
if (!completed_) {
......@@ -144,8 +147,10 @@ class DelegatingURLLoaderClient final : public network::mojom::URLLoaderClient {
network::URLLoaderCompletionStatus status;
status.error_code = net::ERR_ABORTED;
client_->OnComplete(status);
if (!devtools_enabled_)
return;
AddDevToolsCallback(
base::Bind(&NotifyNavigationPreloadCompletedOnUI, status));
base::BindOnce(&NotifyNavigationPreloadCompletedOnUI, status));
}
}
......@@ -176,8 +181,14 @@ class DelegatingURLLoaderClient final : public network::mojom::URLLoaderClient {
client_->OnReceiveResponse(head, std::move(downloaded_file));
DCHECK(on_response_);
std::move(on_response_).Run();
if (!devtools_enabled_)
return;
// Make a deep copy of ResourceResponseHead before passing it cross-thread.
auto resource_response = base::MakeRefCounted<network::ResourceResponse>();
resource_response->head = head;
AddDevToolsCallback(
base::Bind(&NotifyNavigationPreloadResponseReceivedOnUI, url_, head));
base::BindOnce(&NotifyNavigationPreloadResponseReceivedOnUI, url_,
resource_response->DeepCopy()));
}
void OnReceiveRedirect(const net::RedirectInfo& redirect_info,
const network::ResourceResponseHead& head) override {
......@@ -186,11 +197,18 @@ class DelegatingURLLoaderClient final : public network::mojom::URLLoaderClient {
// OnReceiveRedirect IPC and don't send OnComplete IPC. The service worker
// will clean up the preload request when OnReceiveRedirect() is called.
client_->OnReceiveRedirect(redirect_info, head);
if (!devtools_enabled_)
return;
// Make a deep copy of ResourceResponseHead before passing it cross-thread.
auto resource_response = base::MakeRefCounted<network::ResourceResponse>();
resource_response->head = head;
AddDevToolsCallback(
base::Bind(&NotifyNavigationPreloadResponseReceivedOnUI, url_, head));
base::BindOnce(&NotifyNavigationPreloadResponseReceivedOnUI, url_,
resource_response->DeepCopy()));
network::URLLoaderCompletionStatus status;
AddDevToolsCallback(
base::Bind(&NotifyNavigationPreloadCompletedOnUI, status));
base::BindOnce(&NotifyNavigationPreloadCompletedOnUI, status));
}
void OnStartLoadingResponseBody(
mojo::ScopedDataPipeConsumerHandle body) override {
......@@ -201,8 +219,10 @@ class DelegatingURLLoaderClient final : public network::mojom::URLLoaderClient {
return;
completed_ = true;
client_->OnComplete(status);
if (!devtools_enabled_)
return;
AddDevToolsCallback(
base::Bind(&NotifyNavigationPreloadCompletedOnUI, status));
base::BindOnce(&NotifyNavigationPreloadCompletedOnUI, status));
}
void Bind(network::mojom::URLLoaderClientPtr* ptr_info) {
......@@ -211,7 +231,7 @@ class DelegatingURLLoaderClient final : public network::mojom::URLLoaderClient {
private:
void MaybeRunDevToolsCallbacks() {
if (!worker_id_)
if (!worker_id_ || !devtools_enabled_)
return;
while (!devtools_callbacks.empty()) {
BrowserThread::PostTask(
......@@ -222,7 +242,7 @@ class DelegatingURLLoaderClient final : public network::mojom::URLLoaderClient {
}
}
void AddDevToolsCallback(
base::Callback<void(const WorkerId&, const std::string&)> callback) {
base::OnceCallback<void(const WorkerId&, const std::string&)> callback) {
devtools_callbacks.push(std::move(callback));
MaybeRunDevToolsCallbacks();
}
......@@ -232,10 +252,11 @@ class DelegatingURLLoaderClient final : public network::mojom::URLLoaderClient {
base::OnceClosure on_response_;
bool completed_ = false;
const GURL url_;
const bool devtools_enabled_;
base::Optional<std::pair<int, int>> worker_id_;
std::string devtools_request_id_;
base::queue<base::Callback<void(const WorkerId&, const std::string&)>>
base::queue<base::OnceCallback<void(const WorkerId&, const std::string&)>>
devtools_callbacks;
DISALLOW_COPY_AND_ASSIGN(DelegatingURLLoaderClient);
};
......
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