Commit d5726c8c authored by Joey Arhar's avatar Joey Arhar Committed by Chromium LUCI CQ

Fix addEventListener signal option memory leak

AddEventListenerOptionsResolved holds a Member<AbortSignal>, which was
causing a circular reference keeping the AbortSignal alive forever.

Bug: 1148326
Fixed: 1155771
Change-Id: I673bc78c3ab2b10f40624958d9ea147be01c002f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2585674Reviewed-by: default avatarMason Freed <masonfreed@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#838272}
parent 9d0120f3
...@@ -529,15 +529,18 @@ bool EventTarget::AddEventListenerInternal( ...@@ -529,15 +529,18 @@ bool EventTarget::AddEventListenerInternal(
event_type, listener, options, &registered_listener); event_type, listener, options, &registered_listener);
if (added) { if (added) {
if (options->hasSignal()) { if (options->hasSignal()) {
// Instead of passing the entire |options| here, which could create a
// circular reference due to |options| holding a Member<AbortSignal>, just
// pass the |options->capture()| boolean, which is the only thing
// removeEventListener actually uses to find and remove the event
// listener.
options->signal()->AddAlgorithm(WTF::Bind( options->signal()->AddAlgorithm(WTF::Bind(
[](EventTarget* event_target, const AtomicString& event_type, [](EventTarget* event_target, const AtomicString& event_type,
const EventListener* listener, const EventListener* listener, bool capture) {
AddEventListenerOptionsResolved* options) { event_target->removeEventListener(event_type, listener, capture);
event_target->removeEventListener(event_type, listener, options);
}, },
WrapWeakPersistent(this), event_type, WrapWeakPersistent(listener), WrapWeakPersistent(this), event_type, WrapWeakPersistent(listener),
WrapPersistent( options->capture()));
MakeGarbageCollected<AddEventListenerOptionsResolved>(options))));
if (const LocalDOMWindow* executing_window = ExecutingWindow()) { if (const LocalDOMWindow* executing_window = ExecutingWindow()) {
if (const Document* document = executing_window->document()) { if (const Document* document = executing_window->document()) {
document->CountUse(WebFeature::kAddEventListenerWithAbortSignal); document->CountUse(WebFeature::kAddEventListenerWithAbortSignal);
......
...@@ -5890,10 +5890,6 @@ crbug.com/1032681 virtual/disable-frequency-capping-for-overlay-popup-detection/ ...@@ -5890,10 +5890,6 @@ crbug.com/1032681 virtual/disable-frequency-capping-for-overlay-popup-detection/
# Sheriff 2020-12-03 # Sheriff 2020-12-03
crbug.com/1154940 [ Win7 ] inspector-protocol/overlay/overlay-persistent-overlays-with-emulation.js [ Pass Failure ] crbug.com/1154940 [ Win7 ] inspector-protocol/overlay/overlay-persistent-overlays-with-emulation.js [ Pass Failure ]
# Sheriff 2020-12-04
# Failing on Webkit Linux Leak
crbug.com/1155771 [ Linux ] external/wpt/dom/events/AddEventListenerOptions-signal.any.html [ Pass Failure ]
# DevTools roll # DevTools roll
crbug.com/1144171 http/tests/devtools/report-API-errors.js [ Skip ] crbug.com/1144171 http/tests/devtools/report-API-errors.js [ Skip ]
......
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