Skip to content

Commit

Permalink
Prototype of adding AbortController to addEventListener
Browse files Browse the repository at this point in the history
I2P: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/9396JedBBOM

This feature adds a new AbortSignal option, named "signal", to the
options parameter of addEventListener.
This new option can be used to remove the event listener with an
AbortController by calling abort().
More context: whatwg/dom#911

Bug: 1146467
Change-Id: I8ab1b93ec8be859c6876296fda4a9b808253c7d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2527343
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#826659}
GitOrigin-RevId: f0bbc3f3ab2bf91bea606fbc5fda751f5e60a0d3
  • Loading branch information
josepharhar authored and copybara-github committed Nov 12, 2020
1 parent 8e7a96f commit 04477fa
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 0 deletions.
1 change: 1 addition & 0 deletions blink/public/mojom/web_feature/web_feature.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -3056,6 +3056,7 @@ enum WebFeature {
kAdFrameDetected = 3727,
kMediaStreamTrackGenerator = 3728,
kMediaStreamTrackProcessor = 3729,
kAddEventListenerWithAbortSignal = 3730,

// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@
dictionary AddEventListenerOptions : EventListenerOptions {
boolean passive;
boolean once = false;
[RuntimeEnabled=AddEventListenerAbortSignal] AbortSignal signal;
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ AddEventListenerOptionsResolved::AddEventListenerOptionsResolved(
setPassive(options->passive());
if (options->hasOnce())
setOnce(options->once());
if (options->hasSignal())
setSignal(options->signal());
// EventListenerOptions
if (options->hasCapture())
setCapture(options->capture());
Expand Down
17 changes: 17 additions & 0 deletions blink/renderer/core/dom/events/event_target.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "third_party/blink/renderer/bindings/core/v8/js_based_event_listener.h"
#include "third_party/blink/renderer/bindings/core/v8/js_event_listener.h"
#include "third_party/blink/renderer/bindings/core/v8/source_location.h"
#include "third_party/blink/renderer/core/dom/abort_signal.h"
#include "third_party/blink/renderer/core/dom/events/add_event_listener_options_resolved.h"
#include "third_party/blink/renderer/core/dom/events/event.h"
#include "third_party/blink/renderer/core/dom/events/event_dispatch_forbidden_scope.h"
Expand Down Expand Up @@ -474,6 +475,8 @@ bool EventTarget::addEventListener(
resolved_options->setOnce(options->once());
if (options->hasCapture())
resolved_options->setCapture(options->capture());
if (options->hasSignal())
resolved_options->setSignal(options->signal());
return addEventListener(event_type, event_listener, resolved_options);
}

Expand Down Expand Up @@ -530,6 +533,20 @@ bool EventTarget::AddEventListenerInternal(
bool added = EnsureEventTargetData().event_listener_map.Add(
event_type, listener, options, &registered_listener);
if (added) {
if (options->signal()) {
options->signal()->AddAlgorithm(WTF::Bind(
[](EventTarget* event_target, const AtomicString& event_type,
const EventListener* listener) {
event_target->removeEventListener(event_type, listener);
},
WrapWeakPersistent(this), event_type, WrapWeakPersistent(listener)));
if (const LocalDOMWindow* executing_window = ExecutingWindow()) {
if (const Document* document = executing_window->document()) {
document->CountUse(WebFeature::kAddEventListenerWithAbortSignal);
}
}
}

AddedEventListener(event_type, registered_listener);
if (IsA<JSBasedEventListener>(listener) &&
IsInstrumentedForAsyncStack(event_type)) {
Expand Down
15 changes: 15 additions & 0 deletions blink/renderer/core/dom/events/event_target_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,19 @@ TEST_F(EventTargetTest, UseCountBeforematch) {
GetDocument().IsUseCounted(WebFeature::kBeforematchHandlerRegistered));
}

TEST_F(EventTargetTest, UseCountAbortSignal) {
EXPECT_FALSE(
GetDocument().IsUseCounted(WebFeature::kAddEventListenerWithAbortSignal));
GetDocument().GetSettings()->SetScriptEnabled(true);
ClassicScript::CreateUnspecifiedScript(ScriptSourceCode(R"HTML(
const element = document.createElement('div');
const ac = new AbortController();
element.addEventListener(
'test', () => {}, {signal: ac.signal});
)HTML"))
->RunScript(GetDocument().domWindow());
EXPECT_TRUE(
GetDocument().IsUseCounted(WebFeature::kAddEventListenerWithAbortSignal));
}

} // namespace blink
4 changes: 4 additions & 0 deletions blink/renderer/platform/runtime_enabled_features.json5
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@
name: "AccessibilityUseAXPositionForDocumentMarkers",
status: "test",
},
{
name: "AddEventListenerAbortSignal",
status: "experimental",
},
{
name: "AddressSpace",
status: "experimental",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!DOCTYPE html>
<meta charset="utf-8">
<link rel="author" title="Joey Arhar" href="mailto:jarhar@chromium.org">

<!-- This behavior has not been specified yet. See https://github.com/whatwg/dom/issues/911 -->

<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>

<body>

<script>
test(t => {
const target = new EventTarget();
const controller = new AbortController();

target.addEventListener('testevent', t.step_func(() => {
assert_unreached('testevent should have been canceled by AbortController');
}), {signal: controller.signal});

controller.abort();
target.dispatchEvent(new Event('testevent'));
}, 'Tests support for EventController to cancel event listeners in addEventListener.');
</script>

0 comments on commit 04477fa

Please sign in to comment.