Skip to content

Commit 2b9471c

Browse files
committed
Minor adoption of queueTaskKeepingNodeAlive()
https://bugs.webkit.org/show_bug.cgi?id=309352 Reviewed by Ryosuke Niwa. We also rename queueTaskKeepingThisNodeAlive() to queueTaskKeepingNodeAlive() as you now need to pass this as an argument to make the method work with Safer CPP. No functional changes and therefore no tests. Canonical link: https://commits.webkit.org/308882@main
1 parent 9d9e8b9 commit 2b9471c

File tree

9 files changed

+40
-46
lines changed

9 files changed

+40
-46
lines changed

Source/WebCore/SaferCPPExpectations/UncountedLambdaCapturesCheckerExpectations

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ animation/KeyframeEffect.cpp
1818
contentextensions/ContentExtensionsBackend.cpp
1919
crypto/SubtleCrypto.cpp
2020
css/CSSValue.cpp
21-
dom/ContentVisibilityDocumentState.cpp
2221
dom/CustomElementDefaultARIA.cpp
2322
dom/RejectedPromiseTracker.cpp
2423
inspector/agents/page/PageDOMDebuggerAgent.cpp

Source/WebCore/dom/ContentVisibilityDocumentState.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -168,14 +168,15 @@ bool ContentVisibilityDocumentState::checkRelevancyOfContentVisibilityElement(El
168168
auto isSkippedContent = target.isRelevantToUser() ? IsSkippedContent::No : IsSkippedContent::Yes;
169169
target.invalidateStyle();
170170
updateAnimations(target, wasSkippedContent, isSkippedContent);
171-
target.queueTaskKeepingThisNodeAlive(TaskSource::DOMManipulation, [&, isSkippedContent] {
172-
if (target.isConnected()) {
173-
ContentVisibilityAutoStateChangeEvent::Init init {
174-
{ false, false, false },
175-
isSkippedContent == IsSkippedContent::Yes
176-
};
177-
target.dispatchEvent(ContentVisibilityAutoStateChangeEvent::create(eventNames().contentvisibilityautostatechangeEvent, WTF::move(init)));
178-
}
171+
Node::queueTaskKeepingNodeAlive(target, TaskSource::DOMManipulation, [isSkippedContent](auto& element) {
172+
if (!element.isConnected())
173+
return;
174+
175+
ContentVisibilityAutoStateChangeEvent::Init init {
176+
{ false, false, false },
177+
isSkippedContent == IsSkippedContent::Yes
178+
};
179+
element.dispatchEvent(ContentVisibilityAutoStateChangeEvent::create(eventNames().contentvisibilityautostatechangeEvent, WTF::move(init)));
179180
});
180181
return true;
181182
}

Source/WebCore/dom/Node.cpp

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,9 @@
4444
#include "ElementTraversal.h"
4545
#include "EventDispatcher.h"
4646
#include "EventHandler.h"
47-
#include "EventLoop.h"
4847
#include "EventNames.h"
4948
#include "EventTargetInlines.h"
5049
#include "FrameInlines.h"
51-
#include "GCReachableRef.h"
5250
#include "HTMLAreaElement.h"
5351
#include "HTMLBodyElement.h"
5452
#include "HTMLDialogElement.h"
@@ -1480,17 +1478,10 @@ Node& Node::getRootNode(const GetRootNodeOptions& options) const
14801478
return options.composed ? shadowIncludingRoot() : rootNode();
14811479
}
14821480

1483-
void Node::queueTaskKeepingThisNodeAlive(TaskSource source, Function<void ()>&& task)
1484-
{
1485-
document().eventLoop().queueTask(source, [protectedThis = GCReachableRef(*this), task = WTF::move(task)] () {
1486-
task();
1487-
});
1488-
}
1489-
14901481
void Node::queueTaskToDispatchEvent(TaskSource source, Ref<Event>&& event)
14911482
{
1492-
queueTaskKeepingThisNodeAlive(source, [protectedThis = Ref { *this }, event = WTF::move(event)]() {
1493-
protectedThis->dispatchEvent(event);
1483+
queueTaskKeepingNodeAlive(*this, source, [event = WTF::move(event)](Node& node) {
1484+
node.dispatchEvent(event);
14941485
});
14951486
}
14961487

Source/WebCore/dom/Node.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ class Node : public EventTarget, public CanMakeCheckedPtr<Node> {
321321
inline WebCoreOpaqueRoot opaqueRoot() const;
322322
WebCoreOpaqueRoot NODELETE traverseToOpaqueRoot() const;
323323

324-
void queueTaskKeepingThisNodeAlive(TaskSource, Function<void ()>&&);
324+
template<typename T, typename Task> static void queueTaskKeepingNodeAlive(T&, TaskSource, Task&&);
325325
void queueTaskToDispatchEvent(TaskSource, Ref<Event>&&);
326326

327327
// Use when it's guaranteed to that shadowHost is null.

Source/WebCore/dom/NodeInlines.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
#include <WebCore/CharacterData.h>
2424
#include <WebCore/Document.h>
2525
#include <WebCore/Element.h>
26+
#include <WebCore/EventLoop.h>
27+
#include <WebCore/GCReachableRef.h>
2628
#include <WebCore/InspectorInstrumentationPublic.h>
2729
#include <WebCore/LayoutRect.h>
2830
#include <WebCore/Node.h>
@@ -273,4 +275,13 @@ inline void collectChildNodes(Node& node, NodeVector& children)
273275
children.append(*child);
274276
}
275277

278+
template<typename T, typename Task>
279+
void Node::queueTaskKeepingNodeAlive(T& node, TaskSource source, Task&& task)
280+
{
281+
protect(protect(node.document())->eventLoop())->queueTask(source, [protectedThis = GCReachableRef(node), task = std::forward<Task>(task)]() mutable {
282+
// Static analyzer does not know about GCReachableRef.
283+
SUPPRESS_UNCOUNTED_ARG task(protectedThis.get());
284+
});
285+
}
286+
276287
} // namespace WebCore

Source/WebCore/dom/ToggleEventTask.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include "ToggleEventTask.h"
2828

2929
#include "EventNames.h"
30+
#include "NodeInlines.h"
3031
#include "TaskSource.h"
3132
#include "ToggleEvent.h"
3233

@@ -47,7 +48,7 @@ void ToggleEventTask::queue(ToggleState oldState, ToggleState newState, Element*
4748
return;
4849

4950
m_data = { oldState, newState, source };
50-
element->queueTaskKeepingThisNodeAlive(TaskSource::DOMManipulation, [task = Ref { *this }, element, newState] {
51+
Node::queueTaskKeepingNodeAlive(*element, TaskSource::DOMManipulation, [task = Ref { *this }, newState](auto& element) {
5152
if (!task->m_data || task->m_data->newState != newState)
5253
return;
5354

@@ -60,7 +61,7 @@ void ToggleEventTask::queue(ToggleState oldState, ToggleState newState, Element*
6061
init.oldState = stringForState(data.oldState);
6162
init.newState = stringForState(data.newState);
6263
init.source = data.source;
63-
element->dispatchEvent(ToggleEvent::create(eventNames().toggleEvent, init, Event::IsCancelable::No));
64+
element.dispatchEvent(ToggleEvent::create(eventNames().toggleEvent, init, Event::IsCancelable::No));
6465
});
6566
}
6667

Source/WebCore/html/HTMLDialogElement.cpp

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
#include "ContainerNodeInlines.h"
3030
#include "CSSSelector.h"
3131
#include "DocumentPage.h"
32-
#include "EventLoop.h"
3332
#include "EventNames.h"
3433
#include "FocusOptions.h"
3534
#include "HTMLButtonElement.h"
@@ -206,7 +205,7 @@ void HTMLDialogElement::requestClose(const String& returnValue, Element* source)
206205
if (!isOpen())
207206
return;
208207

209-
auto cancelEvent = Event::create(eventNames().cancelEvent, Event::CanBubble::No, Event::IsCancelable::Yes);
208+
Ref cancelEvent = Event::create(eventNames().cancelEvent, Event::CanBubble::No, Event::IsCancelable::Yes);
210209
dispatchEvent(cancelEvent);
211210
if (!cancelEvent->defaultPrevented())
212211
close(returnValue, source);
@@ -247,14 +246,11 @@ bool HTMLDialogElement::handleCommandInternal(HTMLButtonElement& invoker, const
247246

248247
void HTMLDialogElement::queueCancelTask()
249248
{
250-
queueTaskKeepingThisNodeAlive(TaskSource::UserInteraction, [weakThis = WeakPtr { *this }] {
251-
RefPtr protectedThis = weakThis.get();
252-
if (!protectedThis)
253-
return;
254-
auto cancelEvent = Event::create(eventNames().cancelEvent, Event::CanBubble::No, Event::IsCancelable::Yes);
255-
protectedThis->dispatchEvent(cancelEvent);
249+
queueTaskKeepingNodeAlive(*this, TaskSource::UserInteraction, [](auto& dialog) {
250+
Ref cancelEvent = Event::create(eventNames().cancelEvent, Event::CanBubble::No, Event::IsCancelable::Yes);
251+
dialog.dispatchEvent(cancelEvent);
256252
if (!cancelEvent->defaultPrevented())
257-
protectedThis->close(nullString());
253+
dialog.close(nullString());
258254
});
259255
}
260256

Source/WebCore/html/HTMLPlugInElement.cpp

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,20 +31,17 @@
3131
#include "CommonVM.h"
3232
#include "ContainerNodeInlines.h"
3333
#include "ContentSecurityPolicy.h"
34-
#include "DocumentEventLoop.h"
3534
#include "DocumentLoader.h"
3635
#include "DocumentPage.h"
3736
#include "DocumentSecurityOrigin.h"
3837
#include "DocumentView.h"
3938
#include "ElementInlines.h"
4039
#include "Event.h"
4140
#include "EventHandler.h"
42-
#include "EventLoop.h"
4341
#include "EventNames.h"
4442
#include "FrameDestructionObserverInlines.h"
4543
#include "FrameLoader.h"
4644
#include "FrameTree.h"
47-
#include "GCReachableRef.h"
4845
#include "HTMLImageLoader.h"
4946
#include "HTMLNames.h"
5047
#include "HitTestResult.h"
@@ -463,13 +460,13 @@ bool HTMLPlugInElement::requestObject(const String& relativeURL, const String& m
463460
if (ScriptDisallowedScope::InMainThread::isScriptAllowed())
464461
return document->frame()->loader().subframeLoader().requestObject(*this, relativeURL, getNameAttribute(), mimeType, paramNames, paramValues);
465462

466-
protect(document->eventLoop())->queueTask(TaskSource::Networking, [this, protectedThis = Ref { *this }, relativeURL, nameAttribute = getNameAttribute(), mimeType, paramNames, paramValues, document]() mutable {
467-
if (!this->isConnected() || &this->document() != document.ptr())
463+
queueTaskKeepingNodeAlive(*this, TaskSource::Networking, [relativeURL, nameAttribute = getNameAttribute(), mimeType, paramNames, paramValues, document](auto& element) mutable {
464+
if (!element.isConnected() || &element.document() != document.ptr())
468465
return;
469-
RefPtr frame = this->document().frame();
466+
RefPtr frame = element.document().frame();
470467
if (!frame)
471468
return;
472-
frame->loader().subframeLoader().requestObject(*this, relativeURL, nameAttribute, mimeType, paramNames, paramValues);
469+
frame->loader().subframeLoader().requestObject(element, relativeURL, nameAttribute, mimeType, paramNames, paramValues);
473470
});
474471
return true;
475472
}
@@ -514,8 +511,8 @@ void HTMLPlugInElement::scheduleUpdateForAfterStyleResolution()
514511

515512
m_hasUpdateScheduledForAfterStyleResolution = true;
516513

517-
protect(document->eventLoop())->queueTask(TaskSource::DOMManipulation, [element = GCReachableRef { *this }] {
518-
element->updateAfterStyleResolution();
514+
queueTaskKeepingNodeAlive(*this, TaskSource::DOMManipulation, [](auto& element) {
515+
element.updateAfterStyleResolution();
519516
});
520517
}
521518

Source/WebCore/html/HTMLTextFormControlElement.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,8 @@
3838
#include "ElementInlines.h"
3939
#include "ElementTextDirection.h"
4040
#include "Event.h"
41-
#include "EventLoop.h"
4241
#include "EventNames.h"
4342
#include "FrameSelection.h"
44-
#include "GCReachableRef.h"
4543
#include "HTMLBRElement.h"
4644
#include "HTMLFormElement.h"
4745
#include "HTMLInputElement.h"
@@ -594,9 +592,9 @@ void HTMLTextFormControlElement::scheduleSelectionChangeEvent()
594592
return;
595593

596594
m_hasScheduledSelectionChangeEvent = true;
597-
document().eventLoop().queueTask(TaskSource::UserInteraction, [textControl = GCReachableRef { *this }] {
598-
textControl->m_hasScheduledSelectionChangeEvent = false;
599-
textControl->dispatchEvent(Event::create(eventNames().selectionchangeEvent, Event::CanBubble::Yes, Event::IsCancelable::No));
595+
queueTaskKeepingNodeAlive(*this, TaskSource::UserInteraction, [](auto& textControl) {
596+
textControl.m_hasScheduledSelectionChangeEvent = false;
597+
textControl.dispatchEvent(Event::create(eventNames().selectionchangeEvent, Event::CanBubble::Yes, Event::IsCancelable::No));
600598
});
601599
}
602600

0 commit comments

Comments
 (0)