REGRESSION(r209865): Crash when navigating back to some pages with compositing layers.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Dec 2016 00:23:37 +0000 (00:23 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Dec 2016 00:23:37 +0000 (00:23 +0000)
<https://webkit.org/b/166469>
<rdar://problem/29109053>

Reviewed by Darin Adler.

Source/WebCore:

Remove the old WK1-era clear/restoreBackingStores optimization from the page cache.
When enabling it on non-iOS platforms, we started hitting lots of assertions,
and none of our memory tests showed any significant improvement anyway.

Test: compositing/page-cache-back-crash.html

* history/CachedFrame.cpp:
(WebCore::CachedFrameBase::CachedFrameBase):
(WebCore::CachedFrameBase::restore):
(WebCore::CachedFrame::CachedFrame):
* history/CachedFrame.h:
* page/FrameView.cpp:
(WebCore::FrameView::restoreBackingStores): Deleted.
* page/FrameView.h:

LayoutTests:

Add a smoke test for the crashes we were seeing. Thanks to Zalán for the reduction.

* compositing/page-cache-back-crash-expected.txt: Added.
* compositing/page-cache-back-crash.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@210142 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/compositing/page-cache-back-crash-expected.txt [new file with mode: 0644]
LayoutTests/compositing/page-cache-back-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/history/CachedFrame.cpp
Source/WebCore/history/CachedFrame.h
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h

index d9ff5b3..1f5fcd3 100644 (file)
@@ -1,3 +1,16 @@
+2016-12-23  Andreas Kling  <akling@apple.com>
+
+        REGRESSION(r209865): Crash when navigating back to some pages with compositing layers.
+        <https://webkit.org/b/166469>
+        <rdar://problem/29109053>
+
+        Reviewed by Darin Adler.
+
+        Add a smoke test for the crashes we were seeing. Thanks to Zalán for the reduction.
+
+        * compositing/page-cache-back-crash-expected.txt: Added.
+        * compositing/page-cache-back-crash.html: Added.
+
 2016-12-22  Sam Weinig  <sam@webkit.org>
 
         [WebIDL] Remove custom bindings for WebSQL code
diff --git a/LayoutTests/compositing/page-cache-back-crash-expected.txt b/LayoutTests/compositing/page-cache-back-crash-expected.txt
new file mode 100644 (file)
index 0000000..2be1195
--- /dev/null
@@ -0,0 +1 @@
+- Test passes if it doesn't crash.
diff --git a/LayoutTests/compositing/page-cache-back-crash.html b/LayoutTests/compositing/page-cache-back-crash.html
new file mode 100644 (file)
index 0000000..511bdbf
--- /dev/null
@@ -0,0 +1,25 @@
+<style>
+.outer { position: fixed; }
+.inner { position: absolute; }
+</style>
+<div class=outer>-<div class=inner></div></div>
+Test passes if it doesn't crash.
+<script>
+if (window.testRunner) {
+    window.testRunner.dumpAsText();
+    window.testRunner.waitUntilDone();
+    window.testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+}
+
+window.addEventListener("pageshow", function(event) {
+    if (event.persisted)
+        testRunner.notifyDone();
+}, false);
+
+window.addEventListener("load", function() {
+    setTimeout(function() {
+        // Navigate to a helper page that will immediately navigate back here after loading.
+        window.location.href = "iframes/resources/page-cache-helper.html";
+    }, 0);
+});
+</script>
index 73ef8b0..d306859 100644 (file)
@@ -1,3 +1,26 @@
+2016-12-23  Andreas Kling  <akling@apple.com>
+
+        REGRESSION(r209865): Crash when navigating back to some pages with compositing layers.
+        <https://webkit.org/b/166469>
+        <rdar://problem/29109053>
+
+        Reviewed by Darin Adler.
+
+        Remove the old WK1-era clear/restoreBackingStores optimization from the page cache.
+        When enabling it on non-iOS platforms, we started hitting lots of assertions,
+        and none of our memory tests showed any significant improvement anyway.
+
+        Test: compositing/page-cache-back-crash.html
+
+        * history/CachedFrame.cpp:
+        (WebCore::CachedFrameBase::CachedFrameBase):
+        (WebCore::CachedFrameBase::restore):
+        (WebCore::CachedFrame::CachedFrame):
+        * history/CachedFrame.h:
+        * page/FrameView.cpp:
+        (WebCore::FrameView::restoreBackingStores): Deleted.
+        * page/FrameView.h:
+
 2016-12-23  Sam Weinig  <sam@webkit.org>
 
         Add missing std::optional to ApplePayPaymentRequest.lineItems
index 21ec527..2946003 100644 (file)
@@ -63,7 +63,6 @@ CachedFrameBase::CachedFrameBase(Frame& frame)
     , m_view(frame.view())
     , m_url(frame.document()->url())
     , m_isMainFrame(!frame.tree().parent())
-    , m_isComposited(frame.view()->hasCompositedContent())
 {
 }
 
@@ -97,9 +96,6 @@ void CachedFrameBase::restore()
     // cached page.
     frame.script().updatePlatformScriptObjects();
 
-    if (m_isComposited)
-        frame.view()->restoreBackingStores();
-
     frame.loader().client().didRestoreFromPageCache();
 
     // Reconstruct the FrameTree. And open the child CachedFrames in their respective FrameLoaders.
@@ -164,9 +160,6 @@ CachedFrame::CachedFrame(Frame& frame)
 
     frame.loader().client().savePlatformDataToCachedFrame(this);
 
-    if (m_isComposited)
-        frame.view()->clearBackingStores();
-
     // documentWillSuspendForPageCache() can set up a layout timer on the FrameView, so clear timers after that.
     frame.clearTimers();
 
index b4d5836..a8f3732 100644 (file)
@@ -60,7 +60,6 @@ protected:
     std::unique_ptr<ScriptCachedFrameData> m_cachedFrameScriptData;
     std::unique_ptr<CachedFramePlatformData> m_cachedFramePlatformData;
     bool m_isMainFrame;
-    bool m_isComposited;
     std::optional<HasInsecureContent> m_hasInsecureContent;
 
     Vector<std::unique_ptr<CachedFrame>> m_childFrames;
index 0ec292b..9605cf0 100644 (file)
@@ -879,17 +879,6 @@ void FrameView::clearBackingStores()
     compositor.clearBackingForAllLayers();
 }
 
-void FrameView::restoreBackingStores()
-{
-    RenderView* renderView = this->renderView();
-    if (!renderView)
-        return;
-
-    RenderLayerCompositor& compositor = renderView->compositor();
-    compositor.enableCompositingMode(true);
-    compositor.updateCompositingLayers(CompositingUpdateAfterLayout);
-}
-
 GraphicsLayer* FrameView::layerForScrolling() const
 {
     RenderView* renderView = this->renderView();
index e0c8d3e..81806ee 100644 (file)
@@ -154,7 +154,6 @@ public:
     void updateCompositingLayersAfterLayout();
 
     void clearBackingStores();
-    void restoreBackingStores();
 
     // Called when changes to the GraphicsLayer hierarchy have to be synchronized with
     // content rendered via the normal painting path.