Bug 191957

Summary: Streamline ListHashSet use in floating object code
Product: WebKit Reporter: Sam Weinig <sam>
Component: New BugsAssignee: Sam Weinig <sam>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, commit-queue, darin, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Sam Weinig
Reported 2018-11-25 19:14:12 PST
Streamline ListHashSet use in floating object code
Attachments
Patch (12.31 KB, patch)
2018-11-25 19:22 PST, Sam Weinig
no flags
Sam Weinig
Comment 1 2018-11-25 19:22:46 PST
WebKit Commit Bot
Comment 2 2018-11-26 11:29:31 PST
Comment on attachment 355614 [details] Patch Clearing flags on attachment: 355614 Committed r238504: <https://trac.webkit.org/changeset/238504>
WebKit Commit Bot
Comment 3 2018-11-26 11:29:33 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 4 2018-11-26 11:31:08 PST
Darin Adler
Comment 5 2018-12-02 22:09:59 PST
Comment on attachment 355614 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=355614&action=review > Source/WebCore/rendering/FloatingObjects.cpp:349 > void FloatingObjects::remove(FloatingObject* floatingObject) Since this won’t work with null, probably change this to take a reference at some point. > Source/WebCore/rendering/FloatingObjects.cpp:351 > + ASSERT((m_set.contains(floatingObject))); Would like to remove the extra set of parentheses here, too. > Source/WebCore/rendering/FloatingObjects.h:122 > + static unsigned hash(PtrType key) { return PtrHash<RenderBox*>::hash(&key->renderer()); } > + static bool equal(PtrType a, PtrType b) { return &a->renderer() == &b->renderer(); } Could we have used reference types here instead of pointer types? I’m not thrilled with functions that use "pointers that are known to never be null". We’d have to add some * at the various call sites, but I think that’s better than having the dereferencing hidden inside the functions.
Note You need to log in before you can comment on or make changes to this bug.