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.

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

Adopt more smart pointers in LocalFrame #27870

Conversation

rwlbuis
Copy link
Contributor

@rwlbuis rwlbuis commented Apr 29, 2024

088a3e5

Adopt more smart pointers in LocalFrame
https://bugs.webkit.org/show_bug.cgi?id=273397

Reviewed by Sihui Liu.

Adopt more smart pointers in LocalFrame based on the
[alpha.webkit.UncountedLocalVarsChecker] warning.

* Source/WebCore/page/LocalFrame.cpp:
(WebCore::parentPageZoomFactor):
(WebCore::parentTextZoomFactor):
(WebCore::rootFrame):
(WebCore::LocalFrame::LocalFrame):
(WebCore::LocalFrame::~LocalFrame):
(WebCore::LocalFrame::shouldUsePrintingLayout const):
(WebCore::LocalFrame::frameForWidget):
(WebCore::LocalFrame::rangeForPoint):
(WebCore::LocalFrame::debugDescription const):
(WebCore::LocalFrame::contentFrameFromWindowOrFrameElement):

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

881201e

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 βœ… πŸ§ͺ api-wpe
βœ… πŸ§ͺ ios-wk2-wpt loading πŸ§ͺ mac-wk1 βœ… πŸ›  wpe-skia
βœ… πŸ§ͺ api-ios loading πŸ§ͺ mac-wk2 βœ… πŸ›  gtk
βœ… πŸ›  tv loading πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ§ͺ gtk-wk2
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@rwlbuis rwlbuis requested a review from cdumez as a code owner April 29, 2024 13:48
@rwlbuis rwlbuis self-assigned this Apr 29, 2024
@rwlbuis rwlbuis added the WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). label Apr 29, 2024
@rwlbuis rwlbuis requested review from szewai and removed request for cdumez April 29, 2024 17:45
@@ -134,23 +134,23 @@ DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, frameCounter, ("Frame"));

static inline float parentPageZoomFactor(LocalFrame* frame)
{
LocalFrame* parent = dynamicDowncast<LocalFrame>(frame->tree().parent());
RefPtr parent = dynamicDowncast<LocalFrame>(frame->tree().parent());
if (!parent)
return 1;
return parent->pageZoomFactor();
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a trivial inline function, no? So why are we doing smart pointer adoption here? Please follow the guidelines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, indeed I did not check for trivial inline functions. Fixed this one now and went over all other changes to check for this.

@cdumez cdumez requested a review from szewai April 29, 2024 19:38
@rwlbuis rwlbuis force-pushed the eng/Adopt-more-smart-pointers-in-LocalFrame branch from 20dd3ba to 15d06f8 Compare April 30, 2024 09:13
@rwlbuis rwlbuis requested a review from cdumez April 30, 2024 14:03

if (auto* window = JSDOMWindow::toWrapped(vm, value))
if (RefPtr window = JSDOMWindow::toWrapped(vm, value))
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems the local variable vm is unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the local variable.

@rwlbuis rwlbuis force-pushed the eng/Adopt-more-smart-pointers-in-LocalFrame branch from 15d06f8 to 881201e Compare April 30, 2024 18:58
@rwlbuis rwlbuis added the merge-queue Applied to send a pull request to merge-queue label May 1, 2024
https://bugs.webkit.org/show_bug.cgi?id=273397

Reviewed by Sihui Liu.

Adopt more smart pointers in LocalFrame based on the
[alpha.webkit.UncountedLocalVarsChecker] warning.

* Source/WebCore/page/LocalFrame.cpp:
(WebCore::parentPageZoomFactor):
(WebCore::parentTextZoomFactor):
(WebCore::rootFrame):
(WebCore::LocalFrame::LocalFrame):
(WebCore::LocalFrame::~LocalFrame):
(WebCore::LocalFrame::shouldUsePrintingLayout const):
(WebCore::LocalFrame::frameForWidget):
(WebCore::LocalFrame::rangeForPoint):
(WebCore::LocalFrame::debugDescription const):
(WebCore::LocalFrame::contentFrameFromWindowOrFrameElement):

Canonical link: https://commits.webkit.org/278206@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Adopt-more-smart-pointers-in-LocalFrame branch from 881201e to 088a3e5 Compare May 1, 2024 08:37
@webkit-commit-queue
Copy link
Collaborator

Committed 278206@main (088a3e5): https://commits.webkit.org/278206@main

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

@webkit-commit-queue webkit-commit-queue merged commit 088a3e5 into WebKit:main May 1, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 1, 2024
@rwlbuis rwlbuis deleted the eng/Adopt-more-smart-pointers-in-LocalFrame branch May 1, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).
Projects
None yet
5 participants