Bug 196644 - Combine event and touch action regions into a single class
Summary: Combine event and touch action regions into a single class
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-05 08:10 PDT by Antti Koivisto
Modified: 2019-04-06 09:49 PDT (History)
5 users (show)

See Also:


Attachments
patch (60.41 KB, patch)
2019-04-05 08:27 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (60.53 KB, patch)
2019-04-05 08:51 PDT, Antti Koivisto
darin: review+
Details | Formatted Diff | Diff
patch (60.55 KB, patch)
2019-04-06 07:24 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2019-04-05 08:10:59 PDT
They are closely related, this avoid duplication.
Comment 1 Radar WebKit Bug Importer 2019-04-05 08:11:46 PDT
<rdar://problem/49643614>
Comment 2 Antti Koivisto 2019-04-05 08:27:22 PDT
Created attachment 366815 [details]
patch
Comment 3 Antti Koivisto 2019-04-05 08:51:33 PDT
Created attachment 366816 [details]
patch
Comment 4 Darin Adler 2019-04-05 09:28:48 PDT
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?
Comment 5 Antti Koivisto 2019-04-06 07:24:40 PDT
Created attachment 366887 [details]
patch
Comment 6 WebKit Commit Bot 2019-04-06 09:49:04 PDT
Comment on attachment 366887 [details]
patch

Clearing flags on attachment: 366887

Committed r243962: <https://trac.webkit.org/changeset/243962>
Comment 7 WebKit Commit Bot 2019-04-06 09:49:06 PDT
All reviewed patches have been landed.  Closing bug.