[webkit-reviews] review denied: [Bug 71916] [chromium] scroll deltas are cleared during commit to the main thread : [Attachment 114982] Patch

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Hin-Chung Lam
<hclam at google.com>'s request for review:
Bug 71916: [chromium] scroll deltas are cleared during commit to the main
thread
https://bugs.webkit.org/show_bug.cgi?id=71916

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

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


since you are storing both deltas, did you consider not baking the delta into
the layer's position in processScrollDeltas()?

> Source/WebCore/platform/graphics/chromium/LayerChromium.h:138
> +    IntSize sentScrollDelta() const { return IntSize(); }

this seems unnecessary, who uses this?	this value doesn't make any sense on
LayerChromium

> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:343
> +   
m_scrollLayerImpl->setSentScrollDelta(m_scrollLayerImpl->sentScrollDelta() +
m_scrollLayerImpl->scrollDelta());

adding in the current sent scroll delta doesn't make sense, it should always be
0 here since we only have 0 or 1 BFACs pending at any given point in time. this
should be just

m_scrollLayerImpl->setSentScrollDelta(m_scrollLayerImpl->scrollDelta())