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

Unified Diff: third_party/WebKit/Source/core/frame/FrameView.cpp

Issue 2404393003: Tie scroll anchoring adjustments to frame lifecycle instead of layout. (Closed)
Patch Set: add DCHECK 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/frame/FrameView.cpp
diff --git a/third_party/WebKit/Source/core/frame/FrameView.cpp b/third_party/WebKit/Source/core/frame/FrameView.cpp
index a3537839378bfed23940dbbef65496c9484447bf..560585ef7d64731755c8a1e1b82690d18c24a9b2 100644
--- a/third_party/WebKit/Source/core/frame/FrameView.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameView.cpp
@@ -223,6 +223,7 @@ DEFINE_TRACE(FrameView) {
visitor->trace(m_children);
visitor->trace(m_viewportScrollableArea);
visitor->trace(m_scrollAnchor);
+ visitor->trace(m_anchoringAdjustmentQueue);
visitor->trace(m_scrollbarManager);
Widget::trace(visitor);
ScrollableArea::trace(visitor);
@@ -909,7 +910,7 @@ void FrameView::performPreLayoutTasks() {
lifecycle().advanceTo(DocumentLifecycle::StyleClean);
if (shouldPerformScrollAnchoring())
- m_scrollAnchor.save();
+ m_scrollAnchor.notifyBeforeLayout();
}
bool FrameView::shouldPerformScrollAnchoring() const {
@@ -2286,11 +2287,6 @@ void FrameView::performPostLayoutTasks() {
scrollingCoordinator->notifyGeometryChanged();
scrollToFragmentAnchor();
- // TODO(skobes): Figure out interactions between scroll anchor, fragment
- // anchor, and history restoration.
- if (shouldPerformScrollAnchoring())
- m_scrollAnchor.restore();
-
sendResizeEventIfNeeded();
}
@@ -2717,6 +2713,10 @@ void FrameView::updateLifecyclePhasesInternal(
return;
}
+ forAllNonThrottledFrameViews([](FrameView& frameView) {
+ frameView.performScrollAnchoringAdjustments();
+ });
+
if (targetState == DocumentLifecycle::PaintClean) {
forAllNonThrottledFrameViews(
[](FrameView& frameView) { frameView.notifyResizeObservers(); });
@@ -2784,6 +2784,21 @@ void FrameView::updateLifecyclePhasesInternal(
updateViewportIntersectionsForSubtree(targetState);
}
+void FrameView::enqueueScrollAnchoringAdjustment(
+ ScrollableArea* scrollableArea) {
+ m_anchoringAdjustmentQueue.add(scrollableArea);
+}
+
+void FrameView::performScrollAnchoringAdjustments() {
+ for (WeakMember<ScrollableArea>& scroller : m_anchoringAdjustmentQueue) {
+ if (scroller) {
+ DCHECK(scroller->scrollAnchor());
+ scroller->scrollAnchor()->adjust();
+ }
+ }
+ m_anchoringAdjustmentQueue.clear();
+}
+
void FrameView::updatePaintProperties() {
TRACE_EVENT0("blink", "FrameView::updatePaintProperties");
@@ -3630,7 +3645,7 @@ void FrameView::updateScrollOffset(const ScrollOffset& offset,
documentLoader->initialScrollState().wasScrolledByUser = true;
}
- if (scrollType != AnchoringScroll)
+ if (scrollType != AnchoringScroll && scrollType != ClampingScroll)
clearScrollAnchor();
}
@@ -3858,15 +3873,8 @@ void FrameView::updateScrollbars() {
void FrameView::adjustScrollOffsetFromUpdateScrollbars() {
ScrollOffset clamped = clampScrollOffset(scrollOffset());
- // Restore before clamping because clamping clears the scroll anchor.
- // TODO(ymalik): This same logic exists in PaintLayerScrollableArea.
- // Remove when root-layer-scrolls is enabled.
- if (clamped != scrollOffset() && shouldPerformScrollAnchoring()) {
- m_scrollAnchor.restore();
- clamped = clampScrollOffset(scrollOffset());
- }
if (clamped != scrollOffset() || scrollOriginChanged()) {
- ScrollableArea::setScrollOffset(clamped, ProgrammaticScroll);
+ ScrollableArea::setScrollOffset(clamped, ClampingScroll);
resetScrollOriginChanged();
}
}
« no previous file with comments | « third_party/WebKit/Source/core/frame/FrameView.h ('k') | third_party/WebKit/Source/core/layout/LayoutBlock.cpp » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698