[webkit-reviews] review granted: [Bug 107027] [Chromium] Notify embedder of programmatic scrolls. : [Attachment 195274] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has granted John Knottenbelt
<jknotten at chromium.org>'s request for review:
Bug 107027: [Chromium] Notify embedder of programmatic scrolls.
https://bugs.webkit.org/show_bug.cgi?id=107027

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

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=195274&action=review


R=me but comment and signature need some tweaking.

> Source/WebCore/page/ChromeClient.h:182
> +    // didProgrammaticallyScroll should be called on any attempt to set the
scroll position programmatically,
> +    // even to the same offset.

This comment is wrong - we don't call (and shouldn't call) this function if the
the scroll offset is the same.

> Source/WebCore/page/ChromeClient.h:183
> +    virtual void didProgrammaticallyScroll(Frame*, const IntPoint&) const {
}

The second parameter's meaning is not clear from the type, so please add a
name.  This is the new scroll position, right?