Title: [210329] trunk
- Revision
- 210329
- Author
- akl...@apple.com
- Date
- 2017-01-05 08:02:44 -0800 (Thu, 05 Jan 2017)
Log Message
REGRESSION(r210226): overflow:scroll scroll position not restored on back navigation
<https://webkit.org/b/166724>
Reviewed by Antti Koivisto.
Source/WebCore:
Before r210226, the render tree being torn down and the document being destroyed
were roughly the same thing, since they would always happen together, from the
render tree's perspective.
Changing this caused us to skip over the code that saves the scroll position
for an element's RenderLayer when going into the page cache. Navigating back to
that page would then scroll the layer to (0,0) instead of the previous position.
The fix is simply to remove the check for documentBeingDestroyed() in ~RenderLayer().
Note that two checks are being removed, there was also a weird "optimization"
to avoid nulling out EventHandler's m_resizeLayer if it points to this layer.
That pointer would eventually get nulled out in EventHandler::clear() anyway,
but it feels better to not let that pointer dangle.
Test: fast/scrolling/page-cache-back-overflow-scroll-restore.html
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::~RenderLayer):
LayoutTests:
Add a test that navigates back to a page with a scrolled overflow:scroll element.
The test verifies that the scroll position is restored.
* fast/scrolling/page-cache-back-overflow-scroll-restore-expected.txt: Added.
* fast/scrolling/page-cache-back-overflow-scroll-restore.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (210328 => 210329)
--- trunk/LayoutTests/ChangeLog 2017-01-05 15:51:39 UTC (rev 210328)
+++ trunk/LayoutTests/ChangeLog 2017-01-05 16:02:44 UTC (rev 210329)
@@ -1,3 +1,16 @@
+2017-01-05 Andreas Kling <akl...@apple.com>
+
+ REGRESSION(r210226): overflow:scroll scroll position not restored on back navigation
+ <https://webkit.org/b/166724>
+
+ Reviewed by Antti Koivisto.
+
+ Add a test that navigates back to a page with a scrolled overflow:scroll element.
+ The test verifies that the scroll position is restored.
+
+ * fast/scrolling/page-cache-back-overflow-scroll-restore-expected.txt: Added.
+ * fast/scrolling/page-cache-back-overflow-scroll-restore.html: Added.
+
2017-01-05 Per Arne Vollan <pvol...@apple.com>
Unreviewed test gardening.
Added: trunk/LayoutTests/fast/scrolling/page-cache-back-overflow-scroll-restore-expected.txt (0 => 210329)
--- trunk/LayoutTests/fast/scrolling/page-cache-back-overflow-scroll-restore-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/scrolling/page-cache-back-overflow-scroll-restore-expected.txt 2017-01-05 16:02:44 UTC (rev 210329)
@@ -0,0 +1,18 @@
+ALERT: Going back.
+
+
+
+
+
+
+
+layer at (0,0) size 100x126 backgroundClip at (0,0) size 100x100 clip at (0,0) size 85x85 scrollY 41 scrollHeight 126
+ RenderBlock (positioned) {DIV} at (0,0) size 100x100 [bgcolor=#CCCCCC]
+ RenderBR {BR} at (0,0) size 0x18
+ RenderBR {BR} at (0,18) size 0x18
+ RenderBR {BR} at (0,36) size 0x18
+ RenderBR {BR} at (0,54) size 0x18
+ RenderBR {BR} at (0,72) size 0x18
+ RenderBR {BR} at (0,90) size 0x18
+ RenderBR {BR} at (0,108) size 0x18
+
Added: trunk/LayoutTests/fast/scrolling/page-cache-back-overflow-scroll-restore.html (0 => 210329)
--- trunk/LayoutTests/fast/scrolling/page-cache-back-overflow-scroll-restore.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/page-cache-back-overflow-scroll-restore.html 2017-01-05 16:02:44 UTC (rev 210329)
@@ -0,0 +1,43 @@
+<style>
+#puffin {
+ font: Ahem 10px;
+ position: absolute;
+ top: 0;
+ left: 0;
+ overflow: scroll;
+ width: 100px;
+ height: 100px;
+ background-color: #ccc;
+}
+</style>
+<div id="puffin">
+<br>
+<br>
+<br>
+<br>
+<br>
+<br>
+<br>
+</div>
+<pre id="tree"></pre>
+<script>
+testRunner.dumpAsText();
+testRunner.waitUntilDone();
+testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+
+window.addEventListener("pageshow", function(event) {
+ if (event.persisted) {
+ document.getElementById("tree").innerText = window.internals.elementRenderTreeAsText(document.getElementById("puffin"));
+ testRunner.notifyDone();
+ }
+}, false);
+
+window.addEventListener("load", function() {
+ setTimeout(function() {
+ eventSender.mouseMoveTo(10, 10);
+ eventSender.mouseScrollBy(0, -100);
+
+ setTimeout(function() {location.href = '' + 'script>';}, 0);
+ }, 0);
+});
+</script>
Modified: trunk/Source/WebCore/ChangeLog (210328 => 210329)
--- trunk/Source/WebCore/ChangeLog 2017-01-05 15:51:39 UTC (rev 210328)
+++ trunk/Source/WebCore/ChangeLog 2017-01-05 16:02:44 UTC (rev 210329)
@@ -1,3 +1,29 @@
+2017-01-05 Andreas Kling <akl...@apple.com>
+
+ REGRESSION(r210226): overflow:scroll scroll position not restored on back navigation
+ <https://webkit.org/b/166724>
+
+ Reviewed by Antti Koivisto.
+
+ Before r210226, the render tree being torn down and the document being destroyed
+ were roughly the same thing, since they would always happen together, from the
+ render tree's perspective.
+
+ Changing this caused us to skip over the code that saves the scroll position
+ for an element's RenderLayer when going into the page cache. Navigating back to
+ that page would then scroll the layer to (0,0) instead of the previous position.
+
+ The fix is simply to remove the check for documentBeingDestroyed() in ~RenderLayer().
+ Note that two checks are being removed, there was also a weird "optimization"
+ to avoid nulling out EventHandler's m_resizeLayer if it points to this layer.
+ That pointer would eventually get nulled out in EventHandler::clear() anyway,
+ but it feels better to not let that pointer dangle.
+
+ Test: fast/scrolling/page-cache-back-overflow-scroll-restore.html
+
+ * rendering/RenderLayer.cpp:
+ (WebCore::RenderLayer::~RenderLayer):
+
2017-01-05 Wenson Hsieh <wenson_hs...@apple.com>
Disable smooth playhead animation for main content media in the Touch Bar
Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (210328 => 210329)
--- trunk/Source/WebCore/rendering/RenderLayer.cpp 2017-01-05 15:51:39 UTC (rev 210328)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp 2017-01-05 16:02:44 UTC (rev 210329)
@@ -348,7 +348,7 @@
RenderLayer::~RenderLayer()
{
- if (inResizeMode() && !renderer().documentBeingDestroyed())
+ if (inResizeMode())
renderer().frame().eventHandler().resizeLayerDestroyed();
ASSERT(m_registeredScrollableArea == renderer().view().frameView().containsScrollableArea(this));
@@ -356,13 +356,11 @@
if (m_registeredScrollableArea)
renderer().view().frameView().removeScrollableArea(this);
- if (!renderer().documentBeingDestroyed()) {
#if ENABLE(IOS_TOUCH_EVENTS)
- unregisterAsTouchEventListenerForScrolling();
+ unregisterAsTouchEventListenerForScrolling();
#endif
- if (Element* element = renderer().element())
- element->setSavedLayerScrollPosition(m_scrollPosition);
- }
+ if (Element* element = renderer().element())
+ element->setSavedLayerScrollPosition(m_scrollPosition);
destroyScrollbar(HorizontalScrollbar);
destroyScrollbar(VerticalScrollbar);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
