Bug 262721

Summary:	Use a HashSet<CheckedRef<LocalFrame>> for Page::m_rootFrames
Product:	WebKit	Reporter:	Chris Dumez <cdumez>
Component:	Layout and Rendering	Assignee:	Chris Dumez <cdumez>
Status:	RESOLVED FIXED
Severity:	Normal	CC:	bfulgham, commit-queue, fujii.hironori, jameshoward, simon.fraser, webkit-bug-importer, zalan
Priority:	P2	Keywords:	InRadar
Version:	WebKit Nightly Build
Hardware:	Unspecified
OS:	Unspecified
Bug Depends on:	262778
Bug Blocks:

Chris Dumez

Reported 2023-10-05 12:57:29 PDT

Use a HashSet<WeakRef<LocalFrame>> for Page::m_rootFrames instead of a WeakHashSet<LocalFrame>. This map is used in a hot code path and using a WeakHashSet is unnecessarily inefficient.

Attachments
Add attachment proposed patch, testcase, etc.

Chris Dumez

Comment 1 2023-10-05 12:59:18 PDT

Pull request: https://github.com/WebKit/WebKit/pull/18713

EWS

Comment 2 2023-10-05 23:52:36 PDT

Committed 268974@main (86924d729527): <https://commits.webkit.org/268974@main> Reviewed commits have been landed. Closing PR #18713 and removing active labels.

Radar WebKit Bug Importer

Comment 3 2023-10-05 23:53:18 PDT

<rdar://problem/116565539>

James Howard

Comment 4 2023-10-06 01:57:20 PDT

I think this change broke Debug builds, at least on my system (arm64, macOS 13.5.1, Xcode 14.3). All tests crash here: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 JavaScriptCore 0x13730c204 WTFCrash + 20 (Assertions.cpp:333) 1 WebCore 0x282ac42b0 WTFCrashWithInfo(int, char const*, char const*, int) + 32 (Assertions.h:778) 2 WebCore 0x28480a570 WebCore::Page::removeRootFrame(WebCore::LocalFrame&) + 268 (Page.cpp:4510) 3 WebCore 0x2847747cc WebCore::LocalFrame::LocalFrame(WebCore::Page&, WTF::UniqueRef<WebCore::LocalFrameLoaderClient>&&, WebCore::ProcessQualified<WTF::ObjectIdentifierGeneric<WebCore::FrameIdentifierType, WTF::ObjectIdentifierMainThreadAccessTraits>>, WebCore::HTMLFrameOwnerElement*, WebCore::Frame*) + 504 (LocalFrame.cpp:174) ... If I revert the change locally, no more crash.

WebKit Commit Bot

Comment 5 2023-10-06 04:55:34 PDT

Re-opened since this is blocked by bug 262778

Chris Dumez

Comment 6 2023-10-06 07:22:05 PDT

Pull request: https://github.com/WebKit/WebKit/pull/18762

EWS

Comment 7 2023-10-06 12:45:44 PDT

Committed 269009@main (75d236ee7095): <https://commits.webkit.org/269009@main> Reviewed commits have been landed. Closing PR #18762 and removing active labels.

Note You need to log in before you can comment on or make changes to this bug.