[webkit-reviews] review denied: [Bug 174097] Introduce ScrollingTreeScrollingNodeDelegateIOS to share code between overflow and frame scrolling : [Attachment 317228] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Frédéric Wang (:fredw)
<fred.wang at free.fr>'s request for review:
Bug 174097: Introduce ScrollingTreeScrollingNodeDelegateIOS to share code
between overflow and frame scrolling
https://bugs.webkit.org/show_bug.cgi?id=174097

Attachment 317228: Patch

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




--- Comment #6 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 317228
  --> https://bugs.webkit.org/attachment.cgi?id=317228
Patch

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

I think this is on the right track.

> Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.h:40
> +#if PLATFORM(IOS)
> +namespace WebKit {
> +class ScrollingTreeScrollingNodeDelegateIOS;
> +}
> +#endif

It's wrong for WebCore to know anything about WebKit, so you can't do this.

Maybe there should be a ScrollingTreeScrollingNodeDelegate base class in
WebCore, which WebKit subclasses.

>
Source/WebKit/UIProcess/Scrolling/ios/ScrollingTreeOverflowScrollingNodeIOS.mm:
145
> +    m_scrollingNodeDelegate.reset(new
ScrollingTreeScrollingNodeDelegateIOS(*this));

We never use bare 'new'. This should use make_unique<>.