rpms/chromium
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

update to 83.0.4103.83 [release 83.0.4103.83-1mamba;Mon Jun 01 2020]

Browse Source
This commit is contained in:
Silvan Calarco 2024-01-05 21:13:35 +01:00
parent f33f68076d
commit aeb2435b7f
2 changed files with 144 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

138
chromium-83.0.4103.83-avoid-double-destruction-of-ServiceWorkerObjectHost.patch Normal file
View File

@ -0,0 +1,138 @@
From bd59ce32629ef684624821419c43967b73d2989e Mon Sep 17 00:00:00 2001
From: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Fri, 8 May 2020 08:25:31 +0000
Subject: [PATCH] ServiceWorker: Avoid double destruction of
ServiceWorkerObjectHost on connection error
This CL avoids the case where ServiceWorkerObjectHost is destroyed twice
on ServiceWorkerObjectHost::OnConnectionError() when Chromium is built
with the GCC build toolchain.
> How does the issue happen?
ServiceWorkerObjectHost has a cyclic reference like this:
ServiceWorkerObjectHost
--([1] scoped_refptr)--> ServiceWorkerVersion
--([2] std::unique_ptr)--> ServiceWorkerProviderHost
--([3] std::unique_ptr)--> ServiceWorkerContainerHost
--([4] std::unique_ptr)--> ServiceWorkerObjectHost
Note that ServiceWorkerContainerHost manages ServiceWorkerObjectHost in
map<int64_t version_id, std::unique_ptr<ServiceWorkerObjectHost>>.
When ServiceWorkerObjectHost::OnConnectionError() is called, the
function removes the reference [4] from the map, and destroys
ServiceWorkerObjectHost. If the object host has the last reference [1]
to ServiceWorkerVersion, the destruction also cuts off the references
[2] and [3], and destroys ServiceWorkerProviderHost and
ServiceWorkerContainerHost.
This seems to work well on the Chromium's default toolchain, but not
work on the GCC toolchain. According to the report, destruction of
ServiceWorkerContainerHost happens while the map owned by the container
host is erasing the ServiceWorkerObjectHost, and this results in crash
due to double destruction of the object host.
I don't know the reason why this happens only on the GCC toolchain, but
I suspect the order of object destruction on std::map::erase() could be
different depending on the toolchains.
> How does this CL fix this?
The ideal fix is to redesign the ownership model of
ServiceWorkerVersion, but it's not feasible in the short term.
Instead, this CL avoids destruction of ServiceWorkerObjectHost on
std::map::erase(). The new code takes the ownership of the object host
from the map first, and then erases the entry from the map. This
separates timings to erase the map entry and to destroy the object host,
so the crash should no longer happen.
Bug: 1056598
Change-Id: Id30654cb575bc557c42044d6f0c6f1f9bfaed613
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2094496
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#766770}
---
.../service_worker_container_host.cc | 10 +++++
.../service_worker_object_host_unittest.cc | 38 +++++++++++++++++++
2 files changed, 48 insertions(+)
diff --git a/content/browser/service_worker/service_worker_container_host.cc b/content/browser/service_worker/service_worker_container_host.cc
index ec7fb1449af..98c62093b0e 100644
--- a/content/browser/service_worker/service_worker_container_host.cc
+++ b/content/browser/service_worker/service_worker_container_host.cc
@@ -669,6 +669,16 @@ void ServiceWorkerContainerHost::RemoveServiceWorkerObjectHost(
int64_t version_id) {
DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
DCHECK(base::Contains(service_worker_object_hosts_, version_id));
+
+ // ServiceWorkerObjectHost to be deleted may have the last reference to
+ // ServiceWorkerVersion that indirectly owns this ServiceWorkerContainerHost.
+ // If we erase the object host directly from the map, |this| could be deleted
+ // during the map operation and may crash. To avoid the case, we take the
+ // ownership of the object host from the map first, and then erase the entry
+ // from the map. See https://crbug.com/1056598 for details.
+ std::unique_ptr<ServiceWorkerObjectHost> to_be_deleted =
+ std::move(service_worker_object_hosts_[version_id]);
+ DCHECK(to_be_deleted);
service_worker_object_hosts_.erase(version_id);
}
diff --git a/content/browser/service_worker/service_worker_object_host_unittest.cc b/content/browser/service_worker/service_worker_object_host_unittest.cc
index 408d7c1f9d1..6eab59040ab 100644
--- a/content/browser/service_worker/service_worker_object_host_unittest.cc
+++ b/content/browser/service_worker/service_worker_object_host_unittest.cc
@@ -200,6 +200,19 @@ class ServiceWorkerObjectHostTest : public testing::Test {
return registration_info;
}
+ void CallOnConnectionError(ServiceWorkerContainerHost* container_host,
+ int64_t version_id) {
+ // ServiceWorkerObjectHost has the last reference to the version.
+ ServiceWorkerObjectHost* object_host =
+ GetServiceWorkerObjectHost(container_host, version_id);
+ EXPECT_TRUE(object_host->version_->HasOneRef());
+
+ // Make sure that OnConnectionError induces destruction of the version and
+ // the object host.
+ object_host->receivers_.Clear();
+ object_host->OnConnectionError();
+ }
+
BrowserTaskEnvironment task_environment_;
std::unique_ptr<EmbeddedWorkerTestHelper> helper_;
scoped_refptr<ServiceWorkerRegistration> registration_;
@@ -409,5 +422,30 @@ TEST_F(ServiceWorkerObjectHostTest, DispatchExtendableMessageEvent_FromClient) {
events[0]->source_info_for_client->client_type);
}
+// This is a regression test for https://crbug.com/1056598.
+TEST_F(ServiceWorkerObjectHostTest, OnConnectionError) {
+ const GURL scope("https://www.example.com/");
+ const GURL script_url("https://www.example.com/service_worker.js");
+ Initialize(std::make_unique<EmbeddedWorkerTestHelper>(base::FilePath()));
+ SetUpRegistration(scope, script_url);
+
+ // Create the provider host.
+ ASSERT_EQ(blink::ServiceWorkerStatusCode::kOk,
+ StartServiceWorker(version_.get()));
+
+ // Set up the case where the last reference to the version is owned by the
+ // service worker object host.
+ ServiceWorkerContainerHost* container_host =
+ version_->provider_host()->container_host();
+ ServiceWorkerVersion* version_rawptr = version_.get();
+ version_ = nullptr;
+ ASSERT_TRUE(version_rawptr->HasOneRef());
+
+ // Simulate the connection error that induces the object host destruction.
+ // This shouldn't crash.
+ CallOnConnectionError(container_host, version_rawptr->version_id());
+ base::RunLoop().RunUntilIdle();
+}
+
} // namespace service_worker_object_host_unittest
} // namespace content

7
chromium.spec
View File

@ -1,6 +1,6 @@
Name: chromium
Epoch: 3
Version: 83.0.4103.61
Version: 83.0.4103.83
Release: 1mamba
Summary: An open-source browser project that aims to build a safer, faster, and more stable way to experience the web
Group: Graphical Desktop/Applications/Internet
@ -66,6 +66,7 @@ Patch60: chromium-83.0.4103.61-std-numeric_limits-is-defined-in-limits.pat
Patch61: chromium-83.0.4103.61-make-some-of-blink-custom-iterators-STL-compatible.patch
Patch62: chromium-83.0.4103.61-nclude-memory-header-to-get-the-definition-of-std-u.patch
Patch63: chromium-83.0.4103.61-v8-remove-soon-to-be-removed-getAllFieldPositions.patch
Patch64: chromium-83.0.4103.83-avoid-double-destruction-of-ServiceWorkerObjectHost.patch
License: BSD
## AUTOBUILDREQ-BEGIN
BuildRequires: glibc-devel
@ -188,6 +189,7 @@ Chromium is an open-source browser project that aims to build a safer, faster, a
%patch61 -p1
%patch62 -p1
%patch63 -p1 -d v8
%patch64 -p1
# Allow building against system libraries in official builds
sed -i 's/OFFICIAL_BUILD/GOOGLE_CHROME_BUILD/' \
@ -462,6 +464,9 @@ ln -s %{_libdir}/chromium/chromedriver %{buildroot}%{_bindir}/chromedriver
%{_mandir}/man1/chromium.1*
%changelog
* Mon Jun 01 2020 Silvan Calarco <silvan.calarco@mambasoft.it> 83.0.4103.83-1mamba
- update to 83.0.4103.83
* Wed May 27 2020 Automatic Build System <autodist@mambasoft.it> 83.0.4103.61-1mamba
- automatic version update by autodist