| Summary: | Combine event and touch action regions into a single class | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Antti Koivisto <koivisto> | ||||||||
| Component: | Scrolling | Assignee: | Nobody <webkit-unassigned> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | commit-queue, darin, graouts, simon.fraser, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Antti Koivisto
2019-04-05 08:10:59 PDT
Created attachment 366815 [details]
patch
Created attachment 366816 [details]
patch
Comment on attachment 366816 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=366816&action=review > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h:127 > + WEBCORE_EXPORT void setEventRegion(EventRegion&&) override; Kind of surprised that these are override and not final. Maybe still a confusing area of C++ for our coding conventions? > Source/WebCore/rendering/EventRegion.cpp:112 > + if (m_touchActionRegions.size() < index + 1) > + m_touchActionRegions.grow(index + 1); Not new: Seems like Vector should have this operation instead of it being two function calls. > Source/WebCore/rendering/EventRegion.cpp:132 > if (action == TouchAction::None || action == TouchAction::Manipulation) Not new: Does this require that "none" and "manipulation" be earlier in the enumeration than any other actions? If so, then maybe static_assert that here? Created attachment 366887 [details]
patch
Comment on attachment 366887 [details] patch Clearing flags on attachment: 366887 Committed r243962: <https://trac.webkit.org/changeset/243962> All reviewed patches have been landed. Closing bug. |