[webkit-reviews] review denied: [Bug 74196] [chromium] Delegate scroll events to the main thread when needed : [Attachment 119033] Patch

[bugzilla-daemon at webkit.org]

Sami Kyostila <skyostil at google.com> has denied Sami Kyostila
<skyostil at google.com>'s request for review:
Bug 74196: [chromium] Delegate scroll events to the main thread when needed
https://bugs.webkit.org/show_bug.cgi?id=74196

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

------- Additional Comments from Sami Kyostila <skyostil at google.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=119033&action=review


Setting as review- until the preceding patch layer scrolling is updated.

>> Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.cpp:341

> 
> I know you're just moving code here, but I don't follow this logic and why
the scroll layer behaves differently.  Would you mind explaining why this is?
Don't all layers (root or no) have the page scale applied to them?
> 
> Also, do you need to apply the page scale delta? Maybe I'd feel more
comfortable if there were more tests for this function that covered page scale.


Turns out the code here is not entirely correct, so I'll rework it in the
preceding patch. Actually there is no need to differentiate between layers or
scale the content point, but instead visibleLayerRect() must be scaled with the
inverse of the page scale to get unscaled css coordinates that match the
coordinate system of contentPoint.

The reason why page scale delta is not needed is that while we are zooming,
visibleLayerRect() is recomputed in terms of the original page scale. It never
includes m_pageScaleDelta so there is no need to undo that here. I'm not really
sure whether this is intended, though.