<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[210329] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/210329">210329</a></dd>
<dt>Author</dt> <dd>akling@apple.com</dd>
<dt>Date</dt> <dd>2017-01-05 08:02:44 -0800 (Thu, 05 Jan 2017)</dd>
</dl>

<h3>Log Message</h3>
<pre>REGRESSION(<a href="http://trac.webkit.org/projects/webkit/changeset/210226">r210226</a>): overflow:scroll scroll position not restored on back navigation
&lt;https://webkit.org/b/166724&gt;

Reviewed by Antti Koivisto.

Source/WebCore:

Before <a href="http://trac.webkit.org/projects/webkit/changeset/210226">r210226</a>, 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 &quot;optimization&quot;
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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderLayercpp">trunk/Source/WebCore/rendering/RenderLayer.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastscrollingpagecachebackoverflowscrollrestoreexpectedtxt">trunk/LayoutTests/fast/scrolling/page-cache-back-overflow-scroll-restore-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastscrollingpagecachebackoverflowscrollrestorehtml">trunk/LayoutTests/fast/scrolling/page-cache-back-overflow-scroll-restore.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (210328 => 210329)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2017-01-05 15:51:39 UTC (rev 210328)
+++ trunk/LayoutTests/ChangeLog        2017-01-05 16:02:44 UTC (rev 210329)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2017-01-05  Andreas Kling  &lt;akling@apple.com&gt;
+
+        REGRESSION(r210226): overflow:scroll scroll position not restored on back navigation
+        &lt;https://webkit.org/b/166724&gt;
+
+        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.
+
</ins><span class="cx"> 2017-01-05  Per Arne Vollan  &lt;pvollan@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Unreviewed test gardening.
</span></span></pre></div>
<a id="trunkLayoutTestsfastscrollingpagecachebackoverflowscrollrestoreexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/scrolling/page-cache-back-overflow-scroll-restore-expected.txt (0 => 210329)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,18 @@
</span><ins>+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
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastscrollingpagecachebackoverflowscrollrestorehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/scrolling/page-cache-back-overflow-scroll-restore.html (0 => 210329)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,43 @@
</span><ins>+&lt;style&gt;
+#puffin {
+    font: Ahem 10px;
+    position: absolute;
+    top: 0;
+    left: 0;
+    overflow: scroll;
+    width: 100px;
+    height: 100px;
+    background-color: #ccc;
+}
+&lt;/style&gt;
+&lt;div id=&quot;puffin&quot;&gt;
+&lt;br&gt;
+&lt;br&gt;
+&lt;br&gt;
+&lt;br&gt;
+&lt;br&gt;
+&lt;br&gt;
+&lt;br&gt;
+&lt;/div&gt;
+&lt;pre id=&quot;tree&quot;&gt;&lt;/pre&gt;
+&lt;script&gt;
+testRunner.dumpAsText();
+testRunner.waitUntilDone();
+testRunner.overridePreference(&quot;WebKitUsesPageCachePreferenceKey&quot;, 1);
+
+window.addEventListener(&quot;pageshow&quot;, function(event) {
+    if (event.persisted) {
+        document.getElementById(&quot;tree&quot;).innerText = window.internals.elementRenderTreeAsText(document.getElementById(&quot;puffin&quot;));
+        testRunner.notifyDone();
+    }
+}, false);
+
+window.addEventListener(&quot;load&quot;, function() {
+    setTimeout(function() {
+        eventSender.mouseMoveTo(10, 10);
+        eventSender.mouseScrollBy(0, -100);
+
+        setTimeout(function() {location.href = 'data:text/html,&lt;script&gt;alert(&quot;Going back.&quot;);history.back();&lt;/' + 'script&gt;';}, 0);
+    }, 0);
+});
+&lt;/script&gt;
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (210328 => 210329)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,29 @@
</span><ins>+2017-01-05  Andreas Kling  &lt;akling@apple.com&gt;
+
+        REGRESSION(r210226): overflow:scroll scroll position not restored on back navigation
+        &lt;https://webkit.org/b/166724&gt;
+
+        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 &quot;optimization&quot;
+        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):
+
</ins><span class="cx"> 2017-01-05  Wenson Hsieh  &lt;wenson_hsieh@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Disable smooth playhead animation for main content media in the Touch Bar
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderLayercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (210328 => 210329)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -348,7 +348,7 @@
</span><span class="cx"> 
</span><span class="cx"> RenderLayer::~RenderLayer()
</span><span class="cx"> {
</span><del>-    if (inResizeMode() &amp;&amp; !renderer().documentBeingDestroyed())
</del><ins>+    if (inResizeMode())
</ins><span class="cx">         renderer().frame().eventHandler().resizeLayerDestroyed();
</span><span class="cx"> 
</span><span class="cx">     ASSERT(m_registeredScrollableArea == renderer().view().frameView().containsScrollableArea(this));
</span><span class="lines">@@ -356,13 +356,11 @@
</span><span class="cx">     if (m_registeredScrollableArea)
</span><span class="cx">         renderer().view().frameView().removeScrollableArea(this);
</span><span class="cx"> 
</span><del>-    if (!renderer().documentBeingDestroyed()) {
</del><span class="cx"> #if ENABLE(IOS_TOUCH_EVENTS)
</span><del>-        unregisterAsTouchEventListenerForScrolling();
</del><ins>+    unregisterAsTouchEventListenerForScrolling();
</ins><span class="cx"> #endif
</span><del>-        if (Element* element = renderer().element())
-            element-&gt;setSavedLayerScrollPosition(m_scrollPosition);
-    }
</del><ins>+    if (Element* element = renderer().element())
+        element-&gt;setSavedLayerScrollPosition(m_scrollPosition);
</ins><span class="cx"> 
</span><span class="cx">     destroyScrollbar(HorizontalScrollbar);
</span><span class="cx">     destroyScrollbar(VerticalScrollbar);
</span></span></pre>
</div>
</div>

</body>
</html>