| Summary: | Make scroll elasticity an enum class | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Nikos Mouchtaris <nmouchtaris> | ||||||||||||
| Component: | New Bugs | Assignee: | Nikos Mouchtaris <nmouchtaris> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | changseok, cmarcelo, esprehn+autocc, ews-watchlist, fred.wang, glenn, jamesr, kondapallykalyan, luiz, pdr, simon.fraser, tonikitoo, webkit-bug-importer | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Nikos Mouchtaris
2021-11-02 16:48:04 PDT
Created attachment 443147 [details]
Patch
Comment on attachment 443147 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=443147&action=review > Source/WebCore/page/FrameView.cpp:200 > + ScrollElasticity verticalElasticity = ScrollElasticity::None; > + ScrollElasticity horizontalElasticity = ScrollElasticity::None; You could use 'auto' here since it's now very obvious from the RHS what the values are. > Source/WebCore/page/Page.cpp:285 > + , m_verticalScrollElasticity(ScrollElasticity::Allowed) > + , m_horizontalScrollElasticity(ScrollElasticity::Allowed) No need for this since you have the initializers. > Source/WebCore/page/Page.h:1043 > - unsigned m_verticalScrollElasticity : 2; // ScrollElasticity > - unsigned m_horizontalScrollElasticity : 2; // ScrollElasticity > + ScrollElasticity m_verticalScrollElasticity { ScrollElasticity::Allowed }; > + ScrollElasticity m_horizontalScrollElasticity { ScrollElasticity::Allowed }; You've undone some memory saving bitfield stuff here; you'll now use 2 bytes instead of one. But since there are not many Page objects allocated, that's probably OK. > Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:390 > + ts.dumpProperty("horizontal scroll elasticity", static_cast<int>(scrollableAreaParameters.horizontalScrollElasticity)); The ideal way to fix this would be to implement TextStream& operator<<(TextStream& ts, ScrollElasticity) Created attachment 443253 [details]
Patch
Created attachment 443264 [details]
Patch
Tools/Scripts/svn-apply failed to apply attachment 443264 [details] to trunk.
Please resolve the conflicts and upload a new patch.
Created attachment 443356 [details]
Patch
Created attachment 443366 [details]
Patch
Committed r285347 (243908@main): <https://commits.webkit.org/243908@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 443366 [details]. |