[webkit-reviews] review denied: [Bug 108051] Coordinated Graphics: remove the DidChangeScrollPosition message. : [Attachment 186584] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Huang Dongsung
<luxtella at company100.net>'s request for review:
Bug 108051: Coordinated Graphics: remove the DidChangeScrollPosition message.
https://bugs.webkit.org/show_bug.cgi?id=108051

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

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=186584&action=review


I have no problem with the change.
I sign off on this if the ChangeLog is changed to have a good explanation of
the change.

> Source/WebKit2/ChangeLog:10
> +	   Coordinated Graphics: remove the DidChangeScrollPosition message.
> +	   https://bugs.webkit.org/show_bug.cgi?id=108051
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   Currently,
CoordinatedGraphicsScene::m_renderedContentsScrollPosition is updated
> +	   at the moment of flushing, so there is no behavior change if a
scrollPosition is sent
> +	   via DidRenderFrame message instead of DidChangeScrollPosition
message.

This is not a good ChangeLog.

You should explain why you remove DidChangeScrollPosition and how the new
design solve the original problem better than DidChangeScrollPosition.
You also have all the fields bellow the message to help explain the changes.

Great changelogs explain both the "Why" and "What" of the change.