Reviewers: ymalik
CL:
https://codereview.chromium.org/2118683002/Description:
Limit ScrollAnchor to 20 adjustments between user scrolls.
This somewhat mitigates the impact of pathological feedback loops with scroll
event handlers.
BUG=601906
Base URL:
https://chromium.googlesource.com/chromium/src.git@masterAffected files (+62, -1 lines):
A third_party/WebKit/LayoutTests/fast/layout/scroll-anchoring/feedback-loop.html
M third_party/WebKit/Source/core/layout/ScrollAnchor.h
M third_party/WebKit/Source/core/layout/ScrollAnchor.cpp
Index: third_party/WebKit/LayoutTests/fast/layout/scroll-anchoring/feedback-loop.html
diff --git a/third_party/WebKit/LayoutTests/fast/layout/scroll-anchoring/feedback-loop.html b/third_party/WebKit/LayoutTests/fast/layout/scroll-anchoring/feedback-loop.html
new file mode 100644
index 0000000000000000000000000000000000000000..2bfe126d2c264769398928f19f913f217f593c61
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/layout/scroll-anchoring/feedback-loop.html
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<script src="../../../resources/testharness.js"></script>
+<script src="../../../resources/testharnessreport.js"></script>
+<style>
+
+body { margin: 0; height: 5000px; }
+#evil { height: 200px; width: 200px; background-color: #fcc; }
+
+</style>
+<div id="evil"></div>
+<script>
+
+internals.runtimeFlags.scrollAnchoringEnabled = true;
+
+var kMaxAdjustments = 20; // Keep in sync with kMaxAdjustments in ScrollAnchor.cpp
+
+var evil = document.querySelector('#evil');
+onscroll = () => { evil.style.marginTop = scrollY + "px"; };
+
+var frame = () => new Promise((resolve) => { requestAnimationFrame(resolve); });
+
+var waitFor = function(condition, failmsg, deadline) {
+ if (!deadline) deadline = Date.now() + 1000;
+ if (condition()) return Promise.resolve();
+ else if (Date.now() > deadline) return Promise.reject(failmsg);
+ else return frame().then(() => waitFor(condition, failmsg, deadline));
+};
+
+var waitFrames = function(n, condition, failmsg) {
+ var p = Promise.resolve();
+ var check = () => (!condition || condition() ?
+ Promise.resolve() : Promise.reject(failmsg));
+ while (n--)
+ p = p.then(check).then(frame);
+ return p.then(check);
+};
+
+var scrollSettlesAt = function(expectedY) {
+ return waitFor(() => (scrollY == expectedY),
+ "scroll did not reach " + expectedY)
+ .then(() => waitFrames(3))
+ .then(() => waitFrames(3, () => (scrollY == expectedY),
+ "scroll did not stay at " + expectedY));
+};
+
+promise_test(function() {
+ var y = 10;
+ return Promise.resolve()
+ .then(() => { scrollTo(0, y); })
+ .then(() => scrollSettlesAt(y + kMaxAdjustments * y));
+}, "Scroll anchoring with scroll event handler feedback loop");
+
+</script>
Index: third_party/WebKit/Source/core/layout/ScrollAnchor.cpp
diff --git a/third_party/WebKit/Source/core/layout/ScrollAnchor.cpp b/third_party/WebKit/Source/core/layout/ScrollAnchor.cpp
index 8d1de54353f6f1b6df13ce4e08afc9fb9725ba54..48b648de6111c3ad0c8eb8f2742b2dce9cf49b3d 100644
--- a/third_party/WebKit/Source/core/layout/ScrollAnchor.cpp
+++ b/third_party/WebKit/Source/core/layout/ScrollAnchor.cpp
@@ -14,9 +14,12 @@ namespace blink {
using Corner = ScrollAnchor::Corner;
+static const int kMaxAdjustments = 20;
+
ScrollAnchor::ScrollAnchor(ScrollableArea* scroller)
: m_scroller(scroller)
, m_hasBounced(false)
+ , m_adjustmentCount(0)
{
ASSERT(m_scroller);
ASSERT(m_scroller->isFrameView() || m_scroller->isPaintLayerScrollableArea());
@@ -251,7 +254,8 @@ void ScrollAnchor::restore()
m_lastAdjusted.clear();
return;
}
- adjust(adjustment);
+ if (++m_adjustmentCount <= kMaxAdjustments)
+ adjust(adjustment);
}
void ScrollAnchor::adjust(IntSize adjustment)
@@ -274,6 +278,7 @@ void ScrollAnchor::adjust(IntSize adjustment)
void ScrollAnchor::clear()
{
+ m_adjustmentCount = 0;
m_current.clear();
}
Index: third_party/WebKit/Source/core/layout/ScrollAnchor.h
diff --git a/third_party/WebKit/Source/core/layout/ScrollAnchor.h b/third_party/WebKit/Source/core/layout/ScrollAnchor.h
index 0847701788399f3293dd5eeee4c35101e104469f..df5d4c45e3a8151374c8bae0c36cbc4fae4201ca 100644
--- a/third_party/WebKit/Source/core/layout/ScrollAnchor.h
+++ b/third_party/WebKit/Source/core/layout/ScrollAnchor.h
@@ -121,6 +121,9 @@ private:
// True iff the last adjustment was the exact opposite of the one before it.
// A bounce suggests a circular interaction with a scroll event handler.
bool m_hasBounced;
+
+ // Number of adjustments made since the last clear().
+ int m_adjustmentCount;
};
} // namespace blink