[webkit-reviews] review granted: [Bug 196644] Combine event and touch action regions into a single class : [Attachment 366816] patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Antti Koivisto <koivisto at iki.fi>'s
request for review:
Bug 196644: Combine event and touch action regions into a single class
https://bugs.webkit.org/show_bug.cgi?id=196644

Attachment 366816: patch

https://bugs.webkit.org/attachment.cgi?id=366816&action=review




--- Comment #4 from Darin Adler <darin at apple.com> ---
Comment on attachment 366816
  --> https://bugs.webkit.org/attachment.cgi?id=366816
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?