Diff
Modified: branches/safari-600.5-branch/ChangeLog (179883 => 179884)
--- branches/safari-600.5-branch/ChangeLog 2015-02-10 22:35:54 UTC (rev 179883)
+++ branches/safari-600.5-branch/ChangeLog 2015-02-10 22:40:00 UTC (rev 179884)
@@ -1,3 +1,19 @@
+2015-02-10 Lucas Forschler <lforsch...@apple.com>
+
+ Merge r176899
+
+ 2014-12-05 Simon Fraser <simon.fra...@apple.com>
+
+ Programmatic scrolling and content changes are not always synchronized
+ https://bugs.webkit.org/show_bug.cgi?id=139245
+ rdar://problem/18833612
+
+ Reviewed by Anders Carlsson.
+
+ Manual test that tries to sync layout with programmatic scrolling.
+
+ * ManualTests/programmatic-scroll-flicker.html: Added.
+
2015-01-21 Babak Shafiei <bshaf...@apple.com>
Merge r177088.
Copied: branches/safari-600.5-branch/ManualTests/programmatic-scroll-flicker.html (from rev 176899, trunk/ManualTests/programmatic-scroll-flicker.html) (0 => 179884)
--- branches/safari-600.5-branch/ManualTests/programmatic-scroll-flicker.html (rev 0)
+++ branches/safari-600.5-branch/ManualTests/programmatic-scroll-flicker.html 2015-02-10 22:40:00 UTC (rev 179884)
@@ -0,0 +1,120 @@
+<!DOCTYPE html>
+<head>
+<style>
+ #testInner {
+ position: absolute;
+ width: 600px;
+ top: 400px;
+ left: 200px;;
+ padding: 1em;
+ background: yellow;
+ box-shadow: 0 0 0.5em gray;
+ }
+
+ .marker {
+ position: fixed;
+ background: black;
+ color: white;
+ top: 200px;
+ padding: 1em;
+ }
+
+ .spacer {
+ height: 200px;
+ }
+
+ button {
+ position: fixed;
+ top: 100px;
+ left: 200px;
+ padding: 2em;
+ font-size: 16px;
+ font-weight: bold;
+ }
+
+ .trigger #testInner {
+ -webkit-transform: translateY(-200px);
+ }
+</style>
+</head>
+<body>
+<button _onclick_="toggleRunning()">
+ CLICK ME to start / stop test
+</button>
+<div class="marker">Correct top position</div>
+<div id="parent" class="trigger">
+ <div id="testInner">test element</div>
+</div>
+<p class="spacer">.</p>
+<p class="spacer">.</p>
+<p class="spacer">.</p>
+<p class="spacer">.</p>
+<p class="spacer">.</p>
+<p class="spacer">.</p>
+<p class="spacer" id="last">.</p>
+
+<script>
+var INTERVAL_MS = 17;
+var testElement = document.getElementById('testInner');
+var testContainerElement = document.getElementById('parent');
+
+var state = {};
+state.triggerTranslationOnOrOff = false;
+state.running = false;
+state.intervalId = 0;
+
+function updateState()
+{
+ var translated = testContainerElement.classList.contains('trigger');
+ state.scrolled = !translated;
+}
+
+function toggleRunning()
+{
+ state.running = !state.running;
+ if (state.running) {
+ updateState();
+ state.intervalId = setInterval(runSequence, INTERVAL_MS);
+ } else {
+ clearInterval(state.intervalId);
+ }
+}
+
+function doExpensiveContentUpdate()
+{
+ var content = 'I should be stable at the correct position and not flicker above/below';
+ if (state.scrolled)
+ content += '.';
+
+ testElement.innerHTML = content;
+
+ var lastElement = document.getElementById('last');
+ var startTime = Date.now();
+ for (var i = 0; i < 1000; i++) {
+ lastElement.innerHTML = (lastElement.innerHTML + '.');
+ }
+ var domWriteTimeMs = Date.now() - startTime;
+ if (domWriteTimeMs > 2 * INTERVAL_MS)
+ lastElement.innerHTML = '';
+}
+
+function runSequence()
+{
+ doExpensiveContentUpdate();
+
+ var newScrollTop;
+ if (state.scrolled) {
+ testContainerElement.classList.add('trigger');
+ newScrollTop = 0;
+ } else {
+ testContainerElement.classList.remove('trigger');
+ newScrollTop = 200;
+ }
+
+ document.body.scrollTop = newScrollTop;
+ state.scrolled = !state.scrolled;
+}
+
+</script>
+</body>
+
Modified: branches/safari-600.5-branch/Source/WebCore/ChangeLog (179883 => 179884)
--- branches/safari-600.5-branch/Source/WebCore/ChangeLog 2015-02-10 22:35:54 UTC (rev 179883)
+++ branches/safari-600.5-branch/Source/WebCore/ChangeLog 2015-02-10 22:40:00 UTC (rev 179884)
@@ -1,3 +1,42 @@
+2015-02-10 Lucas Forschler <lforsch...@apple.com>
+
+ Merge r176899
+
+ 2014-12-05 Simon Fraser <simon.fra...@apple.com>
+
+ Programmatic scrolling and content changes are not always synchronized
+ https://bugs.webkit.org/show_bug.cgi?id=139245
+ rdar://problem/18833612
+
+ Reviewed by Anders Carlsson.
+
+ For programmatic scrolls, AsyncScrollingCoordinator::requestScrollPositionUpdate()
+ calls updateScrollPositionAfterAsyncScroll(), then dispatches the requested
+ scroll position to the scrolling thread.
+
+ Once the scrolling thread commits, it calls back to the main thread via
+ scheduleUpdateScrollPositionAfterAsyncScroll(), which schedules a second
+ call to updateScrollPositionAfterAsyncScroll() on a timer. That's a problem,
+ because some other scroll may have happened in the meantime; when the timer
+ fires, it can sometimes restore a stale scroll position.
+
+ Fix by bailing early from scheduleUpdateScrollPositionAfterAsyncScroll()
+ for programmatic scrolls, since we know that requestScrollPositionUpdate()
+ already did the updateScrollPositionAfterAsyncScroll().
+
+ Test:
+ ManualTests/programmatic-scroll-flicker.html
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::reset): nullptr.
+ (WebCore::FrameView::setScrollPosition): Ditto.
+ (WebCore::FrameView::setWasScrolledByUser): Ditto.
+ * page/scrolling/AsyncScrollingCoordinator.cpp:
+ (WebCore::AsyncScrollingCoordinator::requestScrollPositionUpdate): Use a local variable for
+ isProgrammaticScroll just to make sure we use the same value for the duration of this function.
+ (WebCore::AsyncScrollingCoordinator::scheduleUpdateScrollPositionAfterAsyncScroll): Do nothing
+ if this is a programmatic scroll.
+
2015-02-06 Babak Shafiei <bshaf...@apple.com>
Merge r179758.
Modified: branches/safari-600.5-branch/Source/WebCore/page/FrameView.cpp (179883 => 179884)
--- branches/safari-600.5-branch/Source/WebCore/page/FrameView.cpp 2015-02-10 22:35:54 UTC (rev 179883)
+++ branches/safari-600.5-branch/Source/WebCore/page/FrameView.cpp 2015-02-10 22:40:00 UTC (rev 179884)
@@ -270,7 +270,7 @@
m_visuallyNonEmptyPixelCount = 0;
m_isVisuallyNonEmpty = false;
m_firstVisuallyNonEmptyLayoutCallbackPending = true;
- m_maintainScrollPositionAnchor = 0;
+ m_maintainScrollPositionAnchor = nullptr;
}
void FrameView::removeFromAXObjectCache()
@@ -2004,7 +2004,7 @@
void FrameView::setScrollPosition(const IntPoint& scrollPoint)
{
TemporaryChange<bool> changeInProgrammaticScroll(m_inProgrammaticScroll, true);
- m_maintainScrollPositionAnchor = 0;
+ m_maintainScrollPositionAnchor = nullptr;
ScrollView::setScrollPosition(scrollPoint);
}
@@ -3599,7 +3599,7 @@
{
if (m_inProgrammaticScroll)
return;
- m_maintainScrollPositionAnchor = 0;
+ m_maintainScrollPositionAnchor = nullptr;
if (m_wasScrolledByUser == wasScrolledByUser)
return;
m_wasScrolledByUser = wasScrolledByUser;
Modified: branches/safari-600.5-branch/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp (179883 => 179884)
--- branches/safari-600.5-branch/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2015-02-10 22:35:54 UTC (rev 179883)
+++ branches/safari-600.5-branch/Source/WebCore/page/scrolling/AsyncScrollingCoordinator.cpp 2015-02-10 22:40:00 UTC (rev 179884)
@@ -153,8 +153,9 @@
if (!coordinatesScrollingForFrameView(frameView))
return false;
- if (frameView->inProgrammaticScroll() || frameView->frame().document()->inPageCache())
- updateScrollPositionAfterAsyncScroll(frameView->scrollLayerID(), scrollPosition, frameView->inProgrammaticScroll(), SetScrollingLayerPosition);
+ bool isProgrammaticScroll = frameView->inProgrammaticScroll();
+ if (isProgrammaticScroll || frameView->frame().document()->inPageCache())
+ updateScrollPositionAfterAsyncScroll(frameView->scrollLayerID(), scrollPosition, isProgrammaticScroll, SetScrollingLayerPosition);
// If this frame view's document is being put into the page cache, we don't want to update our
// main frame scroll position. Just let the FrameView think that we did.
@@ -165,7 +166,7 @@
if (!stateNode)
return false;
- stateNode->setRequestedScrollPosition(scrollPosition, frameView->inProgrammaticScroll());
+ stateNode->setRequestedScrollPosition(scrollPosition, isProgrammaticScroll);
return true;
}
@@ -173,6 +174,10 @@
{
ScheduledScrollUpdate scrollUpdate(nodeID, scrollPosition, programmaticScroll, scrollingLayerPositionAction);
+ // For programmatic scrolls, requestScrollPositionUpdate() has already called updateScrollPositionAfterAsyncScroll().
+ if (programmaticScroll)
+ return;
+
if (m_updateNodeScrollPositionTimer.isActive()) {
if (m_scheduledScrollUpdate.matchesUpdateType(scrollUpdate)) {
m_scheduledScrollUpdate.scrollPosition = scrollPosition;
