Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(573)

Unified Diff: third_party/WebKit/Source/core/layout/LayoutBox.cpp

Issue 2383113003: Refactor ScrollableArea::setScrollPosition. (Closed)
Patch Set: nits and rebase Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/layout/LayoutBox.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutBox.cpp b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
index 196a2f1d2316777b0f2d53fe467703e35735e793..515cacad14698660e7c442b3bbd0d9c8a45405f9 100644
--- a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutBox.cpp
@@ -263,18 +263,22 @@ void LayoutBox::styleDidChange(StyleDifference diff,
}
// If our zoom factor changes and we have a defined scrollLeft/Top, we need to
- // adjust that value into the new zoomed coordinate space.
+ // adjust that value into the new zoomed coordinate space. Note that the new
+ // scroll position may be outside the normal min/max range of the scrollable
+ // area, which is weird but OK, because the scrollable area will update its
+ // min/max in updateAfterLayout().
if (hasOverflowClip() && oldStyle &&
oldStyle->effectiveZoom() != newStyle.effectiveZoom()) {
PaintLayerScrollableArea* scrollableArea = this->getScrollableArea();
ASSERT(scrollableArea);
- if (int left = scrollableArea->scrollXOffset()) {
- left = (left / oldStyle->effectiveZoom()) * newStyle.effectiveZoom();
- scrollableArea->scrollToXOffset(left);
- }
- if (int top = scrollableArea->scrollYOffset()) {
- top = (top / oldStyle->effectiveZoom()) * newStyle.effectiveZoom();
- scrollableArea->scrollToYOffset(top);
+ // We use scrollPosition() rather than adjustedScrollPosition(), because
+ // scrollPosition is the distance from the beginning of flow for the box,
+ // which is the dimension we want to preserve.
+ DoublePoint oldPosition = scrollableArea->scrollPositionDouble();
+ if (oldPosition.x() || oldPosition.y()) {
+ DoublePoint newPosition = oldPosition.scaledBy(newStyle.effectiveZoom() /
+ oldStyle->effectiveZoom());
+ scrollableArea->setScrollPositionUnconditionally(newPosition);
}
}
@@ -522,13 +526,15 @@ LayoutUnit LayoutBox::scrollHeight() const {
}
LayoutUnit LayoutBox::scrollLeft() const {
- return hasOverflowClip() ? LayoutUnit(getScrollableArea()->scrollXOffset())
- : LayoutUnit();
+ return hasOverflowClip()
+ ? LayoutUnit(getScrollableArea()->adjustedScrollOffset().width())
+ : LayoutUnit();
}
LayoutUnit LayoutBox::scrollTop() const {
- return hasOverflowClip() ? LayoutUnit(getScrollableArea()->scrollYOffset())
- : LayoutUnit();
+ return hasOverflowClip()
+ ? LayoutUnit(getScrollableArea()->adjustedScrollOffset().height())
+ : LayoutUnit();
}
int LayoutBox::pixelSnappedScrollWidth() const {
@@ -549,9 +555,12 @@ void LayoutBox::setScrollLeft(LayoutUnit newLeft) {
// setScrollTop does, presumably this code does as well.
DisableCompositingQueryAsserts disabler;
- if (hasOverflowClip())
- getScrollableArea()->scrollToXOffset(newLeft, ScrollOffsetClamped,
- ScrollBehaviorAuto);
+ if (hasOverflowClip()) {
+ PaintLayerScrollableArea* scrollableArea = getScrollableArea();
+ scrollableArea->scrollToOffset(
+ DoubleSize(newLeft, scrollableArea->adjustedScrollOffset().height()),
+ ScrollBehaviorAuto);
+ }
}
void LayoutBox::setScrollTop(LayoutUnit newTop) {
@@ -559,9 +568,12 @@ void LayoutBox::setScrollTop(LayoutUnit newTop) {
// compositing/overflow/do-not-assert-on-invisible-composited-layers.html
DisableCompositingQueryAsserts disabler;
- if (hasOverflowClip())
- getScrollableArea()->scrollToYOffset(newTop, ScrollOffsetClamped,
- ScrollBehaviorAuto);
+ if (hasOverflowClip()) {
+ PaintLayerScrollableArea* scrollableArea = getScrollableArea();
+ scrollableArea->scrollToOffset(
+ DoubleSize(scrollableArea->adjustedScrollOffset().width(), newTop),
+ ScrollBehaviorAuto);
+ }
}
void LayoutBox::scrollToOffset(const DoubleSize& offset,
@@ -571,8 +583,7 @@ void LayoutBox::scrollToOffset(const DoubleSize& offset,
DisableCompositingQueryAsserts disabler;
if (hasOverflowClip())
- getScrollableArea()->scrollToOffset(offset, ScrollOffsetClamped,
- scrollBehavior);
+ getScrollableArea()->scrollToOffset(offset, scrollBehavior);
}
// Returns true iff we are attempting an autoscroll inside an iframe with
@@ -1093,8 +1104,7 @@ void LayoutBox::middleClickAutoscroll(const IntPoint& sourcePoint) {
scroll(ScrollByPixel, FloatSize(adjustedScrollDelta(delta)));
}
-void LayoutBox::scrollByRecursively(const DoubleSize& delta,
- ScrollOffsetClamping clamp) {
+void LayoutBox::scrollByRecursively(const DoubleSize& delta) {
if (delta.isZero())
return;
@@ -1107,7 +1117,7 @@ void LayoutBox::scrollByRecursively(const DoubleSize& delta,
ASSERT(scrollableArea);
DoubleSize newScrollOffset = scrollableArea->adjustedScrollOffset() + delta;
- scrollableArea->scrollToOffset(newScrollOffset, clamp);
+ scrollableArea->scrollToOffset(newScrollOffset);
// If this layer can't do the scroll we ask the next layer up that can
// scroll to try.
@@ -1115,7 +1125,7 @@ void LayoutBox::scrollByRecursively(const DoubleSize& delta,
newScrollOffset - scrollableArea->adjustedScrollOffset();
if (!remainingScrollOffset.isZero() && parent()) {
if (LayoutBox* scrollableBox = enclosingScrollableBox())
- scrollableBox->scrollByRecursively(remainingScrollOffset, clamp);
+ scrollableBox->scrollByRecursively(remainingScrollOffset);
LocalFrame* frame = this->frame();
if (frame && frame->page())
« no previous file with comments | « third_party/WebKit/Source/core/layout/LayoutBox.h ('k') | third_party/WebKit/Source/core/layout/LayoutBoxModelObjectTest.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698