[webkit-reviews] review granted: [Bug 227914] Pipe App Highlight scrolling through UI Process in preparation for Note Overlay avoidance. : [Attachment 433540] Patch

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Megan Gardner
<megan_gardner at apple.com>'s request for review:
Bug 227914: Pipe App Highlight scrolling through UI Process in preparation for
Note Overlay avoidance.
https://bugs.webkit.org/show_bug.cgi?id=227914

Attachment 433540: Patch

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




--- Comment #27 from Tim Horton <thorton at apple.com> ---
Comment on attachment 433540
  --> https://bugs.webkit.org/attachment.cgi?id=433540
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=433540&action=review

> Source/WebCore/page/ChromeClient.h:231
> -    virtual void scrollRectIntoView(const IntRect&) const { }; // Currently
only Mac has a non empty implementation.
> +    virtual void scrollMainFrameToRevealRect(const IntRect&) const { }; //
Currently only Mac has a non empty implementation.

See earlier comment about this comment maybe being a lie now (unless I am
wrong?) Honestly though I would just remove it.

> Source/WebKit/UIProcess/WebPageProxy.cpp:10738
> +void WebPageProxy::requestScrollToRect(FloatRect targetRect, FloatPoint
origin)

I think you missed a review comment from Chris here.

> Source/WebKit/UIProcess/WebPageProxy.cpp:10745
> +    process().send(Messages::WebPage::ScrollToRect(targetRect, origin), 0);

You can say this just like this:

    send(Messages::WebPage::ScrollToRect(targetRect, origin));

> Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h:136
> -    void scrollRectIntoView(const WebCore::IntRect&) const final; //
Currently only Mac has a non empty implementation.
> +    void scrollMainFrameToRevealRect(const WebCore::IntRect&) const final;
// Currently only Mac has a non empty implementation.

See earlier comment about this comment maybe being a lie now (unless I am
wrong?) Honestly though I would just remove it.

> Source/WebKit/WebProcess/WebProcess.h:548
> +    void scrollToRect(WebCore::FloatRect targetRect, WebCore::FloatPoint
origin);

I think this is stale and needs deleting