chore: cherry-pick 5 changes from Release-2-M112 (#38083)

* chore: [23-x-y] cherry-pick 5 changes from Release-2-M112

* 1d491fff578b from chromium
* 2b30a50d0e62 from chromium
* f58218891f8c from chromium
* ec53103cc72d from chromium
* f098ff0d1230 from chromium

* chore: update patches

---------

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
This commit is contained in:
Keeley Hammond
2023-04-24 01:13:01 -07:00
committed by GitHub
parent 4d8b9a8eec
commit 26abaeddf1
6 changed files with 483 additions and 0 deletions

View File

@@ -131,4 +131,9 @@ cherry-pick-d9081493c4b2.patch
cherry-pick-d6946b70b431.patch
revert_x11_keep_windowcache_alive_for_a_time_interval.patch
cherry-pick-9585757f9fad.patch
cherry-pick-1d491fff578b.patch
cherry-pick-2b30a50d0e62.patch
cherry-pick-f58218891f8c.patch
cherry-pick-ec53103cc72d.patch
cherry-pick-f098ff0d1230.patch
cherry-pick-63686953dc22.patch

View File

@@ -0,0 +1,94 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Kenichi Ishibashi <bashi@chromium.org>
Date: Sat, 8 Apr 2023 01:56:25 +0000
Subject: Remove the second WeakPtrFactory from SpdyProxyClientSocket
It was introduced [1] to work around an old issue that wouldn't happen
any more since we store a write callback in the class. Instead of having
the second WeakPtrFactory and moving the callback, we can just keep it
until RunWriteCallback() is called.
This is a speculative fix for the linked bug.
[1] https://codereview.chromium.org/338583003/
(cherry picked from commit 01b25615896b911e21103dd381fafc1f85886d91)
Bug: 1428820
Change-Id: I0b5af2675b68188e208c2ecd42293251b2722b28
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4393905
Reviewed-by: Adam Rice <ricea@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1125216}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4410320
Auto-Submit: Kenichi Ishibashi <bashi@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5615@{#1171}
Cr-Branched-From: 9c6408ef696e83a9936b82bbead3d41c93c82ee4-refs/heads/main@{#1109224}
diff --git a/net/spdy/spdy_proxy_client_socket.cc b/net/spdy/spdy_proxy_client_socket.cc
index 9b5b47ac05de773024e46f1cce534ddabed5ebd3..5af712b05af47bd207e9ef3fb9fe33342d152be0 100644
--- a/net/spdy/spdy_proxy_client_socket.cc
+++ b/net/spdy/spdy_proxy_client_socket.cc
@@ -122,7 +122,6 @@ void SpdyProxyClientSocket::Disconnect() {
write_buffer_len_ = 0;
write_callback_.Reset();
- write_callback_weak_factory_.InvalidateWeakPtrs();
next_state_ = STATE_DISCONNECTED;
@@ -277,9 +276,9 @@ int SpdyProxyClientSocket::GetLocalAddress(IPEndPoint* address) const {
return spdy_stream_->GetLocalAddress(address);
}
-void SpdyProxyClientSocket::RunWriteCallback(CompletionOnceCallback callback,
- int result) const {
- std::move(callback).Run(result);
+void SpdyProxyClientSocket::RunWriteCallback(int result) {
+ CHECK(write_callback_);
+ std::move(write_callback_).Run(result);
if (end_stream_state_ == EndStreamState::kEndStreamReceived) {
base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
@@ -516,8 +515,7 @@ void SpdyProxyClientSocket::OnDataSent() {
// stream's write callback chain to unwind (see crbug.com/355511).
base::SingleThreadTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE, base::BindOnce(&SpdyProxyClientSocket::RunWriteCallback,
- write_callback_weak_factory_.GetWeakPtr(),
- std::move(write_callback_), rv));
+ weak_factory_.GetWeakPtr(), rv));
}
void SpdyProxyClientSocket::OnTrailers(const spdy::Http2HeaderBlock& trailers) {
diff --git a/net/spdy/spdy_proxy_client_socket.h b/net/spdy/spdy_proxy_client_socket.h
index 7576ebed45d4983592528f90856c170ece0c04c6..87a731fe42159770098624e3073012061ed0b360 100644
--- a/net/spdy/spdy_proxy_client_socket.h
+++ b/net/spdy/spdy_proxy_client_socket.h
@@ -121,9 +121,9 @@ class NET_EXPORT_PRIVATE SpdyProxyClientSocket : public ProxyClientSocket,
STATE_CLOSED
};
- // Calls |callback.Run(result)|. Used to run a callback posted to the
+ // Calls `write_callback_(result)`. Used to run a callback posted to the
// message loop.
- void RunWriteCallback(CompletionOnceCallback callback, int result) const;
+ void RunWriteCallback(int result);
void OnIOComplete(int result);
@@ -195,13 +195,7 @@ class NET_EXPORT_PRIVATE SpdyProxyClientSocket : public ProxyClientSocket,
};
EndStreamState end_stream_state_ = EndStreamState::kNone;
- // The default weak pointer factory.
base::WeakPtrFactory<SpdyProxyClientSocket> weak_factory_{this};
-
- // Only used for posting write callbacks. Weak pointers created by this
- // factory are invalidated in Disconnect().
- base::WeakPtrFactory<SpdyProxyClientSocket> write_callback_weak_factory_{
- this};
};
} // namespace net

View File

@@ -0,0 +1,91 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Date: Mon, 10 Apr 2023 05:32:06 +0000
Subject: Use ScriptState::Scope instead of setting HandleScope.
Since `GetEffectiveFunction` may call `Get` if the given v8 listener is
an object, we need to prepare `v8::Context::Scope` before calling it.
Blink already have a helper class to prepare the environment for the
script execution, which has already been used used in other
ServiceWorkerGlobalScope member functions. It is `ScriptState::Scope`
This CL also use it instead.
(cherry picked from commit 299385e09d41d5ce3abd434879b5f9b0a8880cd7)
Bug: 1429197
Change-Id: Idbcfdfa9c06160a18b57155a9540f72eed4ec0b8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4387655
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Auto-Submit: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1125148}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4411454
Reviewed-by: Shunya Shishido <sisidovski@chromium.org>
Cr-Commit-Position: refs/branch-heads/5615@{#1191}
Cr-Branched-From: 9c6408ef696e83a9936b82bbead3d41c93c82ee4-refs/heads/main@{#1109224}
diff --git a/content/browser/service_worker/service_worker_version_browsertest.cc b/content/browser/service_worker/service_worker_version_browsertest.cc
index 8ab1f54b2453fe100fdc9b4f1b41ce031c5ecf29..5834127a6e70389540cbf347e65ae4bc80495317 100644
--- a/content/browser/service_worker/service_worker_version_browsertest.cc
+++ b/content/browser/service_worker/service_worker_version_browsertest.cc
@@ -976,6 +976,18 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
version_->fetch_handler_type());
}
+IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
+ NonFunctionFetchHandlerWithHandleEventProperty) {
+ StartServerAndNavigateToSetup();
+ ASSERT_EQ(
+ Install("/service_worker/fetch_event_with_handle_event_property.js"),
+ blink::ServiceWorkerStatusCode::kOk);
+ EXPECT_EQ(ServiceWorkerVersion::FetchHandlerExistence::EXISTS,
+ version_->fetch_handler_existence());
+ EXPECT_EQ(ServiceWorkerVersion::FetchHandlerType::kNotSkippable,
+ version_->fetch_handler_type());
+}
+
// Check that fetch event handler added in the install event should result in a
// service worker that doesn't count as having a fetch event handler.
IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
diff --git a/content/test/data/service_worker/fetch_event_with_handle_event_property.js b/content/test/data/service_worker/fetch_event_with_handle_event_property.js
new file mode 100644
index 0000000000000000000000000000000000000000..2fe6153af242a10162f7ecb8eaab93c17d840211
--- /dev/null
+++ b/content/test/data/service_worker/fetch_event_with_handle_event_property.js
@@ -0,0 +1,11 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let obj = {};
+Object.defineProperty(obj, "handleEvent", {
+ get: () => {},
+ configurable: true,
+ enumerable: true,
+});
+self.addEventListener('fetch', obj);
diff --git a/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc b/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc
index bbbbcd3691ac646716d78a17251909fc4ba43ea0..9438405e331f1978a5c9b76bf8a7148064fb6b7d 100644
--- a/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc
+++ b/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc
@@ -2619,12 +2619,15 @@ ServiceWorkerGlobalScope::FetchHandlerType() {
if (!elv) {
return mojom::blink::ServiceWorkerFetchHandlerType::kNoHandler;
}
- v8::Isolate* isolate = GetIsolate();
- v8::HandleScope handle_scope(isolate);
+
+ ScriptState* script_state = ScriptController()->GetScriptState();
+ // Do not remove this, |scope| is needed by `GetEffectiveFunction`.
+ ScriptState::Scope scope(script_state);
+
// TODO(crbug.com/1349613): revisit the way to implement this.
// The following code returns kEmptyFetchHandler if all handlers are nop.
for (RegisteredEventListener& e : *elv) {
- EventTarget* et = EventTarget::Create(ScriptController()->GetScriptState());
+ EventTarget* et = EventTarget::Create(script_state);
v8::Local<v8::Value> v =
To<JSBasedEventListener>(e.Callback())->GetEffectiveFunction(*et);
if (!v->IsFunction() ||

View File

@@ -0,0 +1,52 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: chromium-autoroll
<chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Wed, 12 Apr 2023 21:54:57 +0000
Subject: Roll Skia from 288bdaeb99a4 to d5846fb1f236 (5 revisions)
https://skia.googlesource.com/skia.git/+log/288bdaeb99a4..d5846fb1f236
2023-04-12 johnstiles@google.com Use packed contexts for binary ops in SkRP.
2023-04-12 kjlubick@google.com Add more stubs for encoders
2023-04-12 johnstiles@google.com Enforce program stack limits on function parameters.
2023-04-12 johnstiles@google.com Reland "Use packed contexts for copy/splat-constant ops in SkRP."
2023-04-12 jamesgk@google.com [graphite] Use replay translation in Dawn backend
If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/skia-autoroll
Please CC borenet@google.com,lovisolo@google.com on the revert to ensure that a human
is aware of the problem.
To file a bug in Skia: https://bugs.chromium.org/p/skia/issues/entry
To file a bug in Chromium: https://bugs.chromium.org/p/chromium/issues/entry
To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug
Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux-blink-rel;luci.chromium.try:linux-chromeos-compile-dbg;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Cq-Do-Not-Cancel-Tryjobs: true
Bug: chromium:1432603
Tbr: lovisolo@google.com
Change-Id: I8e08120b5eabe293c64fecfd76ff60b2d73401c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4420129
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Bot-Commit: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#1129520}
diff --git a/DEPS b/DEPS
index eefdda416ecc032bf2fad613c64efde49a59e9ad..7440c4f0394f9457b69da3ab41906650847ae6c8 100644
--- a/DEPS
+++ b/DEPS
@@ -299,7 +299,7 @@ vars = {
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling Skia
# and whatever else without interference from each other.
- 'skia_revision': 'aab9fb4100da797d25fe340e9a2fcb2ae30fc2e1',
+ 'skia_revision': 'd5846fb1f236b9a115f0acd432daa3de18a64419',
# Three lines of non-changing comments so that
# the commit queue can handle CLs rolling V8
# and whatever else without interference from each other.

View File

@@ -0,0 +1,143 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Andrey Kosyakov <caseq@chromium.org>
Date: Thu, 13 Apr 2023 03:55:24 +0000
Subject: Retain DevToolsAgentHost after ForceDetachAllSessions()
(cherry picked from commit 8c4aee2a90d08535cfb1bf0a59e00cae956b1762)
Bug: 1424337
Change-Id: Ie0ebe2a49ffbd2356b896c39446b93e09cd81f5a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4378100
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1123772}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4420271
Commit-Queue: Srinivas Sista <srinivassista@chromium.org>
Cr-Commit-Position: refs/branch-heads/5615@{#1244}
Cr-Branched-From: 9c6408ef696e83a9936b82bbead3d41c93c82ee4-refs/heads/main@{#1109224}
diff --git a/content/browser/devtools/auction_worklet_devtools_agent_host.cc b/content/browser/devtools/auction_worklet_devtools_agent_host.cc
index 20a989999b6da5d269bca3d6c9bf181eea7180e7..34a13a884885926e3985098315fd9121902099e9 100644
--- a/content/browser/devtools/auction_worklet_devtools_agent_host.cc
+++ b/content/browser/devtools/auction_worklet_devtools_agent_host.cc
@@ -96,7 +96,7 @@ AuctionWorkletDevToolsAgentHost::~AuctionWorkletDevToolsAgentHost() = default;
void AuctionWorkletDevToolsAgentHost::WorkletDestroyed() {
worklet_ = nullptr;
- ForceDetachAllSessions();
+ auto retain_this = ForceDetachAllSessionsImpl();
associated_agent_remote_.reset();
}
diff --git a/content/browser/devtools/devtools_agent_host_impl.cc b/content/browser/devtools/devtools_agent_host_impl.cc
index 12779c33fd74ce96ebfd7da2666f9fa3865ccfae..002e41a67314327a9b1354c3d6ed79f17b4dace8 100644
--- a/content/browser/devtools/devtools_agent_host_impl.cc
+++ b/content/browser/devtools/devtools_agent_host_impl.cc
@@ -358,12 +358,18 @@ bool DevToolsAgentHostImpl::Inspect() {
}
void DevToolsAgentHostImpl::ForceDetachAllSessions() {
- scoped_refptr<DevToolsAgentHostImpl> protect(this);
+ std::ignore = ForceDetachAllSessionsImpl();
+}
+
+scoped_refptr<DevToolsAgentHost>
+DevToolsAgentHostImpl::ForceDetachAllSessionsImpl() {
+ scoped_refptr<DevToolsAgentHost> retain_this(this);
while (!sessions_.empty()) {
DevToolsAgentHostClient* client = (*sessions_.begin())->GetClient();
DetachClient(client);
client->AgentHostClosed(this);
}
+ return retain_this;
}
void DevToolsAgentHostImpl::ForceDetachRestrictedSessions(
diff --git a/content/browser/devtools/devtools_agent_host_impl.h b/content/browser/devtools/devtools_agent_host_impl.h
index 7c1bb43345084074b27d4b87aa99af582657fb0f..552b3ec81de3b6e5f222f94835802e6c6aac645f 100644
--- a/content/browser/devtools/devtools_agent_host_impl.h
+++ b/content/browser/devtools/devtools_agent_host_impl.h
@@ -62,7 +62,6 @@ class CONTENT_EXPORT DevToolsAgentHostImpl : public DevToolsAgentHost {
void DisconnectWebContents() override;
void ConnectWebContents(WebContents* wc) override;
RenderProcessHost* GetProcessHost() override;
- void ForceDetachAllSessions() override;
struct NetworkLoaderFactoryParamsAndInfo {
NetworkLoaderFactoryParamsAndInfo();
@@ -126,8 +125,18 @@ class CONTENT_EXPORT DevToolsAgentHostImpl : public DevToolsAgentHost {
DevToolsRendererChannel* GetRendererChannel() { return &renderer_channel_; }
const std::vector<DevToolsSession*>& sessions() const { return sessions_; }
+ // Returns refptr retaining `this`. All other references may be removed
+ // at this point, so `this` will become invalid as soon as returned refptr
+ // gets destroyed.
+ [[nodiscard]] scoped_refptr<DevToolsAgentHost> ForceDetachAllSessionsImpl();
private:
+ // Note that calling this may result in the instance being deleted,
+ // as instance may be owned by client sessions. This should not be
+ // used by methods of derived classes, use `ForceDetachAllSessionsImpl()`
+ // above instead.
+ void ForceDetachAllSessions() override;
+
friend class DevToolsAgentHost; // for static methods
friend class DevToolsSession;
friend class DevToolsRendererChannel;
diff --git a/content/browser/devtools/render_frame_devtools_agent_host.cc b/content/browser/devtools/render_frame_devtools_agent_host.cc
index 2e0ecd508c3b6b3b3593985b19c0b2d92ebbe4f4..c4da2a97f95c0e213c0ce3fddc96b5596fc8bc0b 100644
--- a/content/browser/devtools/render_frame_devtools_agent_host.cc
+++ b/content/browser/devtools/render_frame_devtools_agent_host.cc
@@ -541,9 +541,9 @@ void RenderFrameDevToolsAgentHost::RenderFrameDeleted(RenderFrameHost* rfh) {
}
void RenderFrameDevToolsAgentHost::DestroyOnRenderFrameGone() {
- scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
+ scoped_refptr<DevToolsAgentHost> retain_this;
if (IsAttached()) {
- ForceDetachAllSessions();
+ retain_this = ForceDetachAllSessionsImpl();
UpdateRawHeadersAccess(frame_host_);
}
ChangeFrameHostAndObservedProcess(nullptr);
diff --git a/content/browser/devtools/service_worker_devtools_agent_host.cc b/content/browser/devtools/service_worker_devtools_agent_host.cc
index 47d2d169a3eb6cf7c135beeb667cfa5b186987a2..14165045182712b71fef70b758f269b1cbbfc7e8 100644
--- a/content/browser/devtools/service_worker_devtools_agent_host.cc
+++ b/content/browser/devtools/service_worker_devtools_agent_host.cc
@@ -320,8 +320,9 @@ void ServiceWorkerDevToolsAgentHost::UpdateProcessHost() {
void ServiceWorkerDevToolsAgentHost::RenderProcessHostDestroyed(
RenderProcessHost* host) {
+ scoped_refptr<DevToolsAgentHost> retain_this;
if (context_wrapper_->process_manager()->IsShutdown())
- ForceDetachAllSessions();
+ retain_this = ForceDetachAllSessionsImpl();
GetRendererChannel()->SetRenderer(mojo::NullRemote(), mojo::NullReceiver(),
ChildProcessHost::kInvalidUniqueID);
process_observation_.Reset();
diff --git a/content/browser/devtools/web_contents_devtools_agent_host.cc b/content/browser/devtools/web_contents_devtools_agent_host.cc
index fb8e10a18bb725eb9280db8695ae12899dba2eef..2203dcef48c70ecc1758062ac871e80983059464 100644
--- a/content/browser/devtools/web_contents_devtools_agent_host.cc
+++ b/content/browser/devtools/web_contents_devtools_agent_host.cc
@@ -299,7 +299,7 @@ DevToolsAgentHostImpl* WebContentsDevToolsAgentHost::GetPrimaryFrameAgent() {
void WebContentsDevToolsAgentHost::WebContentsDestroyed() {
DCHECK_EQ(this, FindAgentHost(web_contents()));
- ForceDetachAllSessions();
+ auto retain_this = ForceDetachAllSessionsImpl();
auto_attacher_.reset();
g_agent_host_instances.Get().erase(web_contents());
Observe(nullptr);
diff --git a/content/browser/devtools/worker_devtools_agent_host.cc b/content/browser/devtools/worker_devtools_agent_host.cc
index db788e7298d6696ec5a354cb387dfdc4030d8ce0..0984b3ae35459d8676b903394577cc6a43601ac8 100644
--- a/content/browser/devtools/worker_devtools_agent_host.cc
+++ b/content/browser/devtools/worker_devtools_agent_host.cc
@@ -87,7 +87,7 @@ void WorkerDevToolsAgentHost::ChildWorkerCreated(
}
void WorkerDevToolsAgentHost::Disconnected() {
- ForceDetachAllSessions();
+ auto retain_this = ForceDetachAllSessionsImpl();
GetRendererChannel()->SetRenderer(mojo::NullRemote(), mojo::NullReceiver(),
ChildProcessHost::kInvalidUniqueID);
std::move(destroyed_callback_).Run(this);

View File

@@ -0,0 +1,98 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Date: Tue, 11 Apr 2023 07:12:34 +0000
Subject: Stop supporting { handleEvent }.
Make the code aligned with the following specification update:
https://github.com/w3c/ServiceWorker/pull/1676
With the previous specification and code, event listener vector
can be modified during the GetEffectiveFunction execution, which may
bring unexpected vector state.
(cherry picked from commit 5105ce37a6853d52ec97894bf6969b3c29a23afd)
Change-Id: I732c4c9ab2caebc49a7f4ef52640df7b8476d838
Bug: 1429201
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4394402
Commit-Queue: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1126483}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4408837
Reviewed-by: Shunya Shishido <sisidovski@chromium.org>
Reviewed-by: Minoru Chikamune <chikamune@chromium.org>
Cr-Commit-Position: refs/branch-heads/5615@{#1203}
Cr-Branched-From: 9c6408ef696e83a9936b82bbead3d41c93c82ee4-refs/heads/main@{#1109224}
diff --git a/content/browser/service_worker/service_worker_version_browsertest.cc b/content/browser/service_worker/service_worker_version_browsertest.cc
index 5834127a6e70389540cbf347e65ae4bc80495317..bff53482eb05f25542bcd9d7cb5b5a05932b9064 100644
--- a/content/browser/service_worker/service_worker_version_browsertest.cc
+++ b/content/browser/service_worker/service_worker_version_browsertest.cc
@@ -988,6 +988,17 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
version_->fetch_handler_type());
}
+IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
+ RemoveFetchEventListenersInGet) {
+ StartServerAndNavigateToSetup();
+ ASSERT_EQ(Install("/service_worker/fetch_event_object_removing_itself.js"),
+ blink::ServiceWorkerStatusCode::kOk);
+ EXPECT_EQ(ServiceWorkerVersion::FetchHandlerExistence::EXISTS,
+ version_->fetch_handler_existence());
+ EXPECT_EQ(ServiceWorkerVersion::FetchHandlerType::kNotSkippable,
+ version_->fetch_handler_type());
+}
+
// Check that fetch event handler added in the install event should result in a
// service worker that doesn't count as having a fetch event handler.
IN_PROC_BROWSER_TEST_F(ServiceWorkerVersionBrowserTest,
diff --git a/content/test/data/service_worker/fetch_event_object_removing_itself.js b/content/test/data/service_worker/fetch_event_object_removing_itself.js
new file mode 100644
index 0000000000000000000000000000000000000000..110bc4821fae3a63a374d3dc6ca954d4bd744952
--- /dev/null
+++ b/content/test/data/service_worker/fetch_event_object_removing_itself.js
@@ -0,0 +1,19 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+let obj = {};
+function victim() {}
+
+Object.defineProperty(obj, 'handleEvent', {
+ get: () => {
+ // Remove the victim function from the listener vector to break the loop.
+ self.removeEventListener('fetch', victim);
+ return () => {};
+ },
+ configurable: true,
+ enumerable: true,
+});
+
+self.addEventListener('fetch', obj);
+self.addEventListener('fetch', victim);
diff --git a/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc b/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc
index 9438405e331f1978a5c9b76bf8a7148064fb6b7d..75d728dd76aec7e5b5a6d2d15e65c65b4c036ca7 100644
--- a/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc
+++ b/third_party/blink/renderer/modules/service_worker/service_worker_global_scope.cc
@@ -2621,7 +2621,7 @@ ServiceWorkerGlobalScope::FetchHandlerType() {
}
ScriptState* script_state = ScriptController()->GetScriptState();
- // Do not remove this, |scope| is needed by `GetEffectiveFunction`.
+ // Do not remove this, |scope| is needed by `GetListenerObject`.
ScriptState::Scope scope(script_state);
// TODO(crbug.com/1349613): revisit the way to implement this.
@@ -2629,8 +2629,8 @@ ServiceWorkerGlobalScope::FetchHandlerType() {
for (RegisteredEventListener& e : *elv) {
EventTarget* et = EventTarget::Create(script_state);
v8::Local<v8::Value> v =
- To<JSBasedEventListener>(e.Callback())->GetEffectiveFunction(*et);
- if (!v->IsFunction() ||
+ To<JSBasedEventListener>(e.Callback())->GetListenerObject(*et);
+ if (v.IsEmpty() || !v->IsFunction() ||
!v.As<v8::Function>()->Experimental_IsNopFunction()) {
return mojom::blink::ServiceWorkerFetchHandlerType::kNotSkippable;
}