chore: cherry-pick 910e9e40d376 from chromium (#29818)

* chore: cherry-pick 910e9e40d376 from chromium * chore: update patches Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: Electron Bot <electron@github.com>
2026-04-10 03:01:51 -04:00 · 2021-06-23 03:10:09 +02:00
parent 4baa634d7b
commit bfa682dfa3
2 changed files with 205 additions and 0 deletions
--- a/patches/chromium/.patches
+++ b/patches/chromium/.patches
@@ -133,4 +133,5 @@ x11_fix_window_enumeration_order_when_wm_doesn_t_set.patch
 cherry-pick-34d5af37f9ac.patch
 m90-lts_longtaskdetector_remove_container_mutation_during.patch
 m90-lts_reduce_memory_consumption_on.patch
+cherry-pick-910e9e40d376.patch
 cherry-pick-d9556a80a790.patch
--- a/patches/chromium/cherry-pick-910e9e40d376.patch
+++ b/patches/chromium/cherry-pick-910e9e40d376.patch
@@ -0,0 +1,204 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Matt Menke <mmenke@chromium.org>
+Date: Thu, 10 Jun 2021 07:05:07 +0000
+Subject: Fix URLLoader cleanup on CorsURLLoaderFactory destruction.
+
+Destroying one URLLoader can result in other URLLoaders getting errors,
+due to to cache interconnectedness. CorsURLLoaderFactory's destructor
+was not taking that into account.
+
+Also fix a bonus bug:  HttpCache::Transaction::response_ wasn't being
+cleared in HttpCache::Transaction::DoHeadersPhaseCannotProceed(), which
+could result in DCHECKs when calling GetResponseInfo() when a
+transaction that was waiting on a cached response from another
+transaction ended up failing.
+
+[M90]: Fixed trivial conflict
+
+(cherry picked from commit 2f49a3c69a2184c95f43a395e4f33a3959cb8dbc)
+
+(cherry picked from commit baf23e3c5b1394982cff718a0e055d4f239245ad)
+
+Bug: 1209769
+Change-Id: I2c18caa488767a29011aca1e1b0bace24c1ba8fc
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2922826
+Reviewed-by: Maksim Orlovich <morlovich@chromium.org>
+Commit-Queue: Matt Menke <mmenke@chromium.org>
+Cr-Original-Original-Commit-Position: refs/heads/master@{#887522}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2935241
+Auto-Submit: Matt Menke <mmenke@chromium.org>
+Cr-Original-Commit-Position: refs/branch-heads/4472@{#1433}
+Cr-Original-Branched-From: 3d60439cfb36485e76a1c5bb7f513d3721b20da1-refs/heads/master@{#870763}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2948654
+Owners-Override: Victor-Gabriel Savu <vsavu@google.com>
+Reviewed-by: Artem Sumaneev <asumaneev@google.com>
+Reviewed-by: Matt Menke <mmenke@chromium.org>
+Commit-Queue: Victor-Gabriel Savu <vsavu@google.com>
+Cr-Commit-Position: refs/branch-heads/4430@{#1513}
+Cr-Branched-From: e5ce7dc4f7518237b3d9bb93cccca35d25216cbe-refs/heads/master@{#857950}
+
+diff --git a/net/http/http_cache_transaction.cc b/net/http/http_cache_transaction.cc
+index bb0b938e71f92c2743df2c12824e8f634f6ed5e2..6a1ccb4657f8de1a14396366b560bc11f4cee829 100644
+--- a/net/http/http_cache_transaction.cc
+++ b/net/http/http_cache_transaction.cc
+@@ -2115,6 +2115,8 @@ int HttpCache::Transaction::DoHeadersPhaseCannotProceed(int result) {
+   entry_ = nullptr;
+   new_entry_ = nullptr;
+ 
+  SetResponse(HttpResponseInfo());
+
+   // Bypass the cache for timeout scenario.
+   if (result == ERR_CACHE_LOCK_TIMEOUT)
+     effective_load_flags_ |= LOAD_DISABLE_CACHE;
+diff --git a/services/network/cors/cors_url_loader_factory.cc b/services/network/cors/cors_url_loader_factory.cc
+index 7e4d57cf7f2f8e4e0aa208f53f83833fc0c68112..bf744d52473e31fb100ccf1644abd5e9122ba669 100644
+--- a/services/network/cors/cors_url_loader_factory.cc
+++ b/services/network/cors/cors_url_loader_factory.cc
+@@ -228,7 +228,17 @@ CorsURLLoaderFactory::CorsURLLoaderFactory(
+       &CorsURLLoaderFactory::DeleteIfNeeded, base::Unretained(this)));
+ }
+ 
+-CorsURLLoaderFactory::~CorsURLLoaderFactory() = default;
+CorsURLLoaderFactory::~CorsURLLoaderFactory() {
+  // Delete loaders one at a time, since deleting one loader can cause another
+  // loader waiting on it to fail synchronously, which could result in the other
+  // loader calling DestroyURLLoader().
+  while (!loaders_.empty()) {
+    // No need to call context_->LoaderDestroyed(), since this method is only
+    // called from the NetworkContext's destructor, or when there are no
+    // remaining URLLoaders.
+    loaders_.erase(loaders_.begin());
+  }
+}
+ 
+ void CorsURLLoaderFactory::OnLoaderCreated(
+     std::unique_ptr<mojom::URLLoader> loader) {
+diff --git a/services/network/cors/cors_url_loader_factory_unittest.cc b/services/network/cors/cors_url_loader_factory_unittest.cc
+index 13811282f5bc922cce50a2d0c0c041d1cdd586c1..dd3361f818590031b8a843595bea1223e012bf3e 100644
+--- a/services/network/cors/cors_url_loader_factory_unittest.cc
+++ b/services/network/cors/cors_url_loader_factory_unittest.cc
+@@ -7,7 +7,9 @@
+ #include "base/macros.h"
+ #include "base/test/task_environment.h"
+ #include "mojo/public/cpp/bindings/remote.h"
+#include "net/base/load_flags.h"
+ #include "net/proxy_resolution/configured_proxy_resolution_service.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+ #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
+ #include "net/url_request/url_request_context.h"
+ #include "net/url_request/url_request_context_builder.h"
+@@ -49,6 +51,9 @@ class CorsURLLoaderFactoryTest : public testing::Test {
+  protected:
+   // testing::Test implementation.
+   void SetUp() override {
+    test_server_.AddDefaultHandlers();
+    ASSERT_TRUE(test_server_.Start());
+
+     network_service_ = NetworkService::CreateForTesting();
+ 
+     auto context_params = mojom::NetworkContextParams::New();
+@@ -68,7 +73,7 @@ class CorsURLLoaderFactoryTest : public testing::Test {
+     auto factory_params = network::mojom::URLLoaderFactoryParams::New();
+     factory_params->process_id = kProcessId;
+     factory_params->request_initiator_origin_lock =
+-        url::Origin::Create(GURL("http://localhost"));
+        url::Origin::Create(test_server_.base_url());
+     auto resource_scheduler_client =
+         base::MakeRefCounted<ResourceSchedulerClient>(
+             kProcessId, kRouteId, &resource_scheduler_,
+@@ -81,15 +86,25 @@ class CorsURLLoaderFactoryTest : public testing::Test {
+   }
+ 
+   void CreateLoaderAndStart(const ResourceRequest& request) {
+    url_loaders_.emplace_back(mojo::Remote<mojom::URLLoader>());
+    test_cors_loader_clients_.emplace_back(
+        std::make_unique<TestURLLoaderClient>());
+     cors_url_loader_factory_->CreateLoaderAndStart(
+-        url_loader_.BindNewPipeAndPassReceiver(), kRouteId, kRequestId,
+        url_loaders_.back().BindNewPipeAndPassReceiver(), kRouteId, kRequestId,
+         mojom::kURLLoadOptionNone, request,
+-        test_cors_loader_client_.CreateRemote(),
+        test_cors_loader_clients_.back()->CreateRemote(),
+         net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS));
+   }
+ 
+   void ResetFactory() { cors_url_loader_factory_.reset(); }
+ 
+  net::test_server::EmbeddedTestServer* test_server() { return &test_server_; }
+
+  std::vector<std::unique_ptr<TestURLLoaderClient>>&
+  test_cors_loader_clients() {
+    return test_cors_loader_clients_;
+  }
+
+  private:
+   // Test environment.
+   base::test::TaskEnvironment task_environment_;
+@@ -99,15 +114,17 @@ class CorsURLLoaderFactoryTest : public testing::Test {
+   std::unique_ptr<NetworkContext> network_context_;
+   mojo::Remote<mojom::NetworkContext> network_context_remote_;
+ 
+  net::test_server::EmbeddedTestServer test_server_;
+
+   // CorsURLLoaderFactory instance under tests.
+   std::unique_ptr<mojom::URLLoaderFactory> cors_url_loader_factory_;
+   mojo::Remote<mojom::URLLoaderFactory> cors_url_loader_factory_remote_;
+ 
+-  // Holds URLLoader that CreateLoaderAndStart() creates.
+-  mojo::Remote<mojom::URLLoader> url_loader_;
+  // Holds the URLLoaders that CreateLoaderAndStart() creates.
+  std::vector<mojo::Remote<mojom::URLLoader>> url_loaders_;
+ 
+-  // TestURLLoaderClient that records callback activities.
+-  TestURLLoaderClient test_cors_loader_client_;
+  // TestURLLoaderClients that record callback activities.
+  std::vector<std::unique_ptr<TestURLLoaderClient>> test_cors_loader_clients_;
+ 
+   // Holds for allowed origin access lists.
+   OriginAccessList origin_access_list_;
+@@ -118,7 +135,7 @@ class CorsURLLoaderFactoryTest : public testing::Test {
+ // Regression test for https://crbug.com/906305.
+ TEST_F(CorsURLLoaderFactoryTest, DestructionOrder) {
+   ResourceRequest request;
+-  GURL url("http://localhost");
+  GURL url = test_server()->GetURL("/hung");
+   request.mode = mojom::RequestMode::kNoCors;
+   request.credentials_mode = mojom::CredentialsMode::kOmit;
+   request.method = net::HttpRequestHeaders::kGetMethod;
+@@ -141,5 +158,36 @@ TEST_F(CorsURLLoaderFactoryTest, DestructionOrder) {
+   ResetFactory();
+ }
+ 
+TEST_F(CorsURLLoaderFactoryTest, CleanupWithSharedCacheObjectInUse) {
+  // Create a loader for a response that hangs after receiving headers, and run
+  // it until headers are received.
+  ResourceRequest request;
+  GURL url = test_server()->GetURL("/hung-after-headers");
+  request.mode = mojom::RequestMode::kNoCors;
+  request.credentials_mode = mojom::CredentialsMode::kOmit;
+  request.method = net::HttpRequestHeaders::kGetMethod;
+  request.url = url;
+  request.request_initiator = url::Origin::Create(url);
+  CreateLoaderAndStart(request);
+  test_cors_loader_clients().back()->RunUntilResponseReceived();
+
+  // Read only requests will fail synchonously on destruction of the request
+  // they're waiting on if they're in the |done_headers_queue| when the other
+  // request fails. Make a large number of such requests, spin the message loop
+  // so they end up blocked on the hung request, and then destroy all loads. A
+  // large number of loaders is needed because they're stored in a set, indexed
+  // by address, so teardown order is random.
+  request.load_flags =
+      net::LOAD_ONLY_FROM_CACHE | net::LOAD_SKIP_CACHE_VALIDATION;
+  for (int i = 0; i < 10; ++i)
+    CreateLoaderAndStart(request);
+  base::RunLoop().RunUntilIdle();
+
+  // This should result in a crash if tearing down one URLLoaderFactory
+  // resulting in a another one failing causes a crash during teardown. See
+  // https://crbug.com/1209769.
+  ResetFactory();
+}
+
+ }  // namespace cors
+ }  // namespace network