Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deploy more smart pointers in ScriptExecutionContext.cpp and RejectedPromiseTracker.cpp #20331

Merged
merged 1 commit into from
Nov 12, 2023
Merged

Deploy more smart pointers in ScriptExecutionContext.cpp and RejectedPromiseTracker.cpp #20331

merged 1 commit into from
Nov 12, 2023

Conversation

rniwa
Copy link
Member

@rniwa rniwa commented Nov 10, 2023 •
edited by webkit-early-warning-system

f739f84

Deploy more smart pointers in ScriptExecutionContext.cpp and RejectedPromiseTracker.cpp
https://bugs.webkit.org/show_bug.cgi?id=264621

Reviewed by Chris Dumez.

Deploy smart pointers as warned by the clang static analyzer.

* Source/WebCore/dom/RejectedPromiseTracker.cpp:
(WebCore::RejectedPromiseTracker::reportUnhandledRejections):
(WebCore::RejectedPromiseTracker::reportRejectionHandled):
* Source/WebCore/dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::reportUnhandledPromiseRejection):
(WebCore::ScriptExecutionContext::dispatchErrorEvent):
(WebCore::ScriptExecutionContext::protectedVM): Added.
(WebCore::ScriptExecutionContext::ensureRejectedPromiseTrackerSlow):
* Source/WebCore/dom/ScriptExecutionContext.h:
* Source/WebCore/dom/ShadowRoot.cpp:
(WebCore::ShadowRoot::~ShadowRoot):

Canonical link: https://commits.webkit.org/270615@main

16b47c6

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@rniwa rniwa requested a review from cdumez as a code owner November 10, 2023 21:33
@rniwa rniwa self-assigned this Nov 10, 2023
@rniwa rniwa added the DOM For bugs specific to XML/HTML DOM elements (including parsing). label Nov 10, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 10, 2023 •
edited

EWS run on previous version of this PR (hash 93621a7)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
❌ 🧪 ios-wk2-wpt ❌ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ❌ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ❌ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

cdumez
cdumez approved these changes Nov 10, 2023
View reviewed changes
Copy link
Contributor

@cdumez cdumez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 11, 2023
rniwa
rniwa commented Nov 11, 2023
View reviewed changes
Source/WebCore/dom/ScriptExecutionContext.cpp Outdated
@@ -724,7 +729,7 @@ CrossOriginMode ScriptExecutionContext::crossOriginMode()
bool ScriptExecutionContext::postTaskTo(ScriptExecutionContextIdentifier identifier, Task&& task)
{
Locker locker { allScriptExecutionContextsMapLock };
auto* context = allScriptExecutionContextsMap().get(identifier);
RefPtr context = allScriptExecutionContextsMap().get(identifier);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently we can't do this.

rniwa
rniwa commented Nov 11, 2023
View reviewed changes
Source/WebCore/dom/ScriptExecutionContext.cpp Outdated
@@ -736,7 +741,7 @@ bool ScriptExecutionContext::postTaskTo(ScriptExecutionContextIdentifier identif
bool ScriptExecutionContext::postTaskForModeToWorkerOrWorklet(ScriptExecutionContextIdentifier identifier, Task&& task, const String& mode)
{
Locker locker { allScriptExecutionContextsMapLock };
auto* context = dynamicDowncast<WorkerOrWorkletGlobalScope>(allScriptExecutionContextsMap().get(identifier));
RefPtr context = dynamicDowncast<WorkerOrWorkletGlobalScope>(allScriptExecutionContextsMap().get(identifier));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this

rniwa
rniwa commented Nov 11, 2023
View reviewed changes
Source/WebCore/dom/ScriptExecutionContext.cpp Outdated
@@ -747,7 +752,7 @@ bool ScriptExecutionContext::postTaskForModeToWorkerOrWorklet(ScriptExecutionCon

bool ScriptExecutionContext::ensureOnContextThread(ScriptExecutionContextIdentifier identifier, Task&& task)
{
ScriptExecutionContext* context = nullptr;
RefPtr<ScriptExecutionContext> context;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or this

@rniwa rniwa removed the merging-blocked Applied to prevent a change from being merged label Nov 11, 2023
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Nov 11, 2023 •
edited

EWS run on current version of this PR (hash 16b47c6)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 🧪 unsafe-merge ✅ 🛠 watch-sim

@rniwa rniwa added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 12, 2023
@rniwa
Deploy more smart pointers in ScriptExecutionContext.cpp and Rejected…
f739f84 
…PromiseTracker.cpp

https://bugs.webkit.org/show_bug.cgi?id=264621

Reviewed by Chris Dumez.

Deploy smart pointers as warned by the clang static analyzer.

* Source/WebCore/dom/RejectedPromiseTracker.cpp:
(WebCore::RejectedPromiseTracker::reportUnhandledRejections):
(WebCore::RejectedPromiseTracker::reportRejectionHandled):
* Source/WebCore/dom/ScriptExecutionContext.cpp:
(WebCore::ScriptExecutionContext::reportUnhandledPromiseRejection):
(WebCore::ScriptExecutionContext::dispatchErrorEvent):
(WebCore::ScriptExecutionContext::protectedVM): Added.
(WebCore::ScriptExecutionContext::ensureRejectedPromiseTrackerSlow):
* Source/WebCore/dom/ScriptExecutionContext.h:
* Source/WebCore/dom/ShadowRoot.cpp:
(WebCore::ShadowRoot::~ShadowRoot):

Canonical link: https://commits.webkit.org/270615@main
@webkit-commit-queue
Copy link
Collaborator

Committed 270615@main (f739f84): https://commits.webkit.org/270615@main

Reviewed commits have been landed. Closing PR #20331 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit f739f84 into WebKit:main Nov 12, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Nov 12, 2023
@rniwa rniwa deleted the fix264621 branch November 13, 2023 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdumez cdumez cdumez approved these changes

Assignees

@rniwa rniwa

Labels
DOM For bugs specific to XML/HTML DOM elements (including parsing).
Projects
None yet
Milestone
No milestone
5 participants
@rniwa @webkit-early-warning-system @webkit-commit-queue @cdumez @webkit-ews-buildbot