From 256e43cad8dc37263a53c3ee1eb813def94da646 Mon Sep 17 00:00:00 2001 From: Tsuyoshi Horo Date: Mon, 4 Dec 2023 04:59:40 +0000 Subject: [PATCH] Fix BackgroundURLLoader bug of handling redirect Fixed: 1504317 Change-Id: I492d355590e412dc7cbc453d9072bc479c05e966 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5082996 Reviewed-by: Yoshisato Yanagisawa Commit-Queue: Tsuyoshi Horo Cr-Commit-Position: refs/heads/main@{#1232559} --- .../background_url_loader_unittest.cc | 32 +++++++++++++++++++ .../url_loader/resource_request_sender.cc | 4 +++ 2 files changed, 36 insertions(+) diff --git a/third_party/blink/renderer/platform/loader/fetch/url_loader/background_url_loader_unittest.cc b/third_party/blink/renderer/platform/loader/fetch/url_loader/background_url_loader_unittest.cc index cdca6d24a31fbd..4221f5ce1923df 100644 --- a/third_party/blink/renderer/platform/loader/fetch/url_loader/background_url_loader_unittest.cc +++ b/third_party/blink/renderer/platform/loader/fetch/url_loader/background_url_loader_unittest.cc @@ -622,6 +622,38 @@ TEST_F(BackgroundResourceFecherTest, RedirectAndCancelDoNotCrash) { EXPECT_TRUE(redirected_url.IsEmpty()); } +TEST_F(BackgroundResourceFecherTest, AbortWhileHandlingRedirectDoNotCrash) { + FakeURLLoaderClient client(freezable_task_runner_); + KURL redirected_url; + client.AddWillFollowRedirectCallback( + base::BindLambdaForTesting([&](const WebURL& new_url) { + redirected_url = new_url; + return true; + })); + auto background_url_loader = + CreateBackgroundURLLoaderAndStart(CreateTestRequest(), &client); + + mojo::Remote loader_client_remote( + std::move(loader_client_pending_remote_)); + FakeURLLoader loader(std::move(loader_pending_receiver_)); + + net::RedirectInfo redirect_info; + redirect_info.new_url = GURL(kRedirectedURL); + + loader_client_remote->OnReceiveRedirect( + redirect_info, network::mojom::URLResponseHead::New()); + loader_client_remote->OnComplete( + network::URLLoaderCompletionStatus(net::ERR_FAILED)); + + // Call RunUntilIdle() to receive Mojo IPC. + task_environment_.RunUntilIdle(); + + EXPECT_TRUE(redirected_url.IsEmpty()); + freezable_task_runner_->RunUntilIdle(); + EXPECT_FALSE(redirected_url.IsEmpty()); + task_environment_.RunUntilIdle(); +} + TEST_F(BackgroundResourceFecherTest, CancelSoonAfterStart) { base::WaitableEvent waitable_event( base::WaitableEvent::ResetPolicy::MANUAL, diff --git a/third_party/blink/renderer/platform/loader/fetch/url_loader/resource_request_sender.cc b/third_party/blink/renderer/platform/loader/fetch/url_loader/resource_request_sender.cc index 45fa0959e06b56..4be9c0c3746fb4 100644 --- a/third_party/blink/renderer/platform/loader/fetch/url_loader/resource_request_sender.cc +++ b/third_party/blink/renderer/platform/loader/fetch/url_loader/resource_request_sender.cc @@ -798,6 +798,10 @@ void ResourceRequestSender::OnFollowRedirectCallback( if (!request_info_) { return; } + if (request_info_->net_error != net::ERR_IO_PENDING) { + // The request has been completed. + return; + } // TODO(yoav): If request_info doesn't change above, we could avoid this // copy.