[webkit-reviews] review granted: [Bug 135787] Refactor ServiceOverlayController in preparation for fading between highlights : [Attachment 236342] patch

[bugzilla-daemon at webkit.org]

Brady Eidson <beidson at apple.com> has granted Tim Horton <thorton at apple.com>'s
request for review:
Bug 135787: Refactor ServiceOverlayController in preparation for fading between
highlights
https://bugs.webkit.org/show_bug.cgi?id=135787

Attachment 236342: patch
https://bugs.webkit.org/attachment.cgi?id=236342&action=review

------- Additional Comments from Brady Eidson <beidson at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=236342&action=review


> Source/WebKit2/WebProcess/WebPage/ServicesOverlayController.h:66
> +	   enum class Type { TelephoneNumber, Selection };

Even if only two thingies, I like seeing this on multiple lines.

enum class Type {
    Thingy1,
    Thingy2,
};

> Source/WebKit2/WebProcess/WebPage/mac/ServicesOverlayController.mm:242
> +    bool hovered = DDHighlightPointIsOnHighlight(highlight.highlight(),
(CGPoint)m_mousePosition, &onButton);

highlight.highlight() reads weirdly to me.

Could the DDHighlightRef accessor be "ddHighlight()" or "ddHighlightRef()"?

It's not too mysterious here because it's being fed into a DD API, but it would
be mysterious without that context.