Remove ScrollView::scrollOffset() in preparation for scrollOffset vs. scrollPosition clarification
Created attachment 267989 [details] Patch
Created attachment 267992 [details] Patch
Comment on attachment 267992 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=267992&action=review > Source/WebCore/page/FrameView.cpp:2170 > + IntPoint newPosition = scrollPosition(); > + if (oldPosition != newPosition) { > updateLayerPositionsAfterScrolling(); > if (frame().settings().acceleratedCompositingForFixedPositionEnabled()) > updateCompositingLayersAfterScrolling(); > - IntPoint newPosition = scrollPosition(); > - scrollAnimator().setCurrentPosition(scrollPosition()); > + scrollAnimator().setCurrentPosition(newPosition); This subtly changes the logic of this function such that if updateLayerPositionsAfterScrolling() or updateCompositingLayersAfterScrolling() can affect scrollPosition(), the new current position will be different. Are you confident they cannot? > Source/WebCore/svg/SVGSVGElement.cpp:419 > + LayoutPoint scrollOffset = view->scrollPosition(); Given that you are now using a position here, it might make sense to rename the local variable.
(In reply to comment #3) > Comment on attachment 267992 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=267992&action=review > > > Source/WebCore/page/FrameView.cpp:2170 > > + IntPoint newPosition = scrollPosition(); > > + if (oldPosition != newPosition) { > > updateLayerPositionsAfterScrolling(); > > if (frame().settings().acceleratedCompositingForFixedPositionEnabled()) > > updateCompositingLayersAfterScrolling(); > > - IntPoint newPosition = scrollPosition(); > > - scrollAnimator().setCurrentPosition(scrollPosition()); > > + scrollAnimator().setCurrentPosition(newPosition); > > This subtly changes the logic of this function such that if > updateLayerPositionsAfterScrolling() or > updateCompositingLayersAfterScrolling() can affect scrollPosition(), the new > current position will be different. Are you confident they cannot? I am!
https://trac.webkit.org/changeset/194438