Title: [192298] releases/WebKitGTK/webkit-2.10
Revision
192298
Author
carlo...@webkit.org
Date
2015-11-11 00:39:42 -0800 (Wed, 11 Nov 2015)

Log Message

Merge r192133 - Crash when subtree layout is set on FrameView while auto size mode is enabled.
https://bugs.webkit.org/show_bug.cgi?id=150995
rdar://problem/22785262

Reviewed by Beth Dakin.

Autosizing initiates multiple synchronous layouts to calculate preferred view width for current content.
FrameView::autoSizeIfEnabled() is called from FrameView::layout() while we are in InPreLayout state.
It is safe to do during full layout.
However, since we setup the subtree state just before the autoSizeIfEnabled() call, reentering it with
a newly issued layout confuses SubtreeLayoutStateMaintainer.

This patch reverses the order of autoSizeIfEnabled() call and the subtree layout state setup.
It also ensures that the first layout requested by autoSizeIfEnabled() always runs on the whole tree.

Source/WebCore:

Test: fast/dynamic/crash-subtree-layout-when-auto-size-enabled.html

* page/FrameView.cpp:
(WebCore::FrameView::layout):
(WebCore::FrameView::convertSubtreeLayoutToFullLayout):
(WebCore::FrameView::scheduleRelayout):
(WebCore::FrameView::scheduleRelayoutOfSubtree):
(WebCore::FrameView::autoSizeIfEnabled):
* page/FrameView.h:
* testing/Internals.cpp:
(WebCore::Internals::enableAutoSizeMode):
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

* fast/dynamic/crash-subtree-layout-when-auto-size-enabled-expected.txt: Added.
* fast/dynamic/crash-subtree-layout-when-auto-size-enabled.html: Added.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog (192297 => 192298)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog	2015-11-11 08:26:24 UTC (rev 192297)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog	2015-11-11 08:39:42 UTC (rev 192298)
@@ -1,3 +1,23 @@
+2015-11-07  Zalan Bujtas  <za...@apple.com>
+
+        Crash when subtree layout is set on FrameView while auto size mode is enabled.
+        https://bugs.webkit.org/show_bug.cgi?id=150995
+        rdar://problem/22785262
+
+        Reviewed by Beth Dakin.
+
+        Autosizing initiates multiple synchronous layouts to calculate preferred view width for current content.
+        FrameView::autoSizeIfEnabled() is called from FrameView::layout() while we are in InPreLayout state.
+        It is safe to do during full layout.
+        However, since we setup the subtree state just before the autoSizeIfEnabled() call, reentering it with
+        a newly issued layout confuses SubtreeLayoutStateMaintainer.
+
+        This patch reverses the order of autoSizeIfEnabled() call and the subtree layout state setup.
+        It also ensures that the first layout requested by autoSizeIfEnabled() always runs on the whole tree.  
+
+        * fast/dynamic/crash-subtree-layout-when-auto-size-enabled-expected.txt: Added.
+        * fast/dynamic/crash-subtree-layout-when-auto-size-enabled.html: Added.
+
 2015-11-06  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         REGRESSION(r182286): Tatechuyoko following ruby is drawn too far to the right

Added: releases/WebKitGTK/webkit-2.10/LayoutTests/fast/dynamic/crash-subtree-layout-when-auto-size-enabled-expected.txt (0 => 192298)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/fast/dynamic/crash-subtree-layout-when-auto-size-enabled-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/fast/dynamic/crash-subtree-layout-when-auto-size-enabled-expected.txt	2015-11-11 08:39:42 UTC (rev 192298)
@@ -0,0 +1 @@
+Pass if no crash or assert.

Added: releases/WebKitGTK/webkit-2.10/LayoutTests/fast/dynamic/crash-subtree-layout-when-auto-size-enabled.html (0 => 192298)


--- releases/WebKitGTK/webkit-2.10/LayoutTests/fast/dynamic/crash-subtree-layout-when-auto-size-enabled.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.10/LayoutTests/fast/dynamic/crash-subtree-layout-when-auto-size-enabled.html	2015-11-11 08:39:42 UTC (rev 192298)
@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<title>This tests that we don't crash in autosize mode with subtree layout.</title>
+<style>
+    div {
+        position: absolute;
+        overflow: hidden;
+        width: 10px;
+        height: 10px;
+    }
+</style>
+</head>
+<body>
+<div>Pass if no crash or assert.<div id=resizeThis></div></div>
+<script>
+if (window.internals)
+    internals.enableAutoSizeMode(true, 10, 10, 1000, 1000);
+
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+}
+
+setTimeout(function() {
+    document.getElementById("resizeThis").style.width = "20px";
+    if (window.testRunner)
+        testRunner.notifyDone();
+    }, 0);
+</script>
+</body>
+</html>

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog (192297 => 192298)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog	2015-11-11 08:26:24 UTC (rev 192297)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog	2015-11-11 08:39:42 UTC (rev 192298)
@@ -1,3 +1,34 @@
+2015-11-07  Zalan Bujtas  <za...@apple.com>
+
+        Crash when subtree layout is set on FrameView while auto size mode is enabled.
+        https://bugs.webkit.org/show_bug.cgi?id=150995
+        rdar://problem/22785262
+
+        Reviewed by Beth Dakin.
+
+        Autosizing initiates multiple synchronous layouts to calculate preferred view width for current content.
+        FrameView::autoSizeIfEnabled() is called from FrameView::layout() while we are in InPreLayout state.
+        It is safe to do during full layout.
+        However, since we setup the subtree state just before the autoSizeIfEnabled() call, reentering it with
+        a newly issued layout confuses SubtreeLayoutStateMaintainer.
+
+        This patch reverses the order of autoSizeIfEnabled() call and the subtree layout state setup.
+        It also ensures that the first layout requested by autoSizeIfEnabled() always runs on the whole tree.  
+
+        Test: fast/dynamic/crash-subtree-layout-when-auto-size-enabled.html
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::layout):
+        (WebCore::FrameView::convertSubtreeLayoutToFullLayout):
+        (WebCore::FrameView::scheduleRelayout):
+        (WebCore::FrameView::scheduleRelayoutOfSubtree):
+        (WebCore::FrameView::autoSizeIfEnabled):
+        * page/FrameView.h:
+        * testing/Internals.cpp:
+        (WebCore::Internals::enableAutoSizeMode):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2015-11-07  Michael Catanzaro  <mcatanz...@igalia.com>
 
         Node.h:392:12: warning: 'this' pointer cannot be null in well-defined C++ code

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/page/FrameView.cpp (192297 => 192298)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/page/FrameView.cpp	2015-11-11 08:26:24 UTC (rev 192297)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/page/FrameView.cpp	2015-11-11 08:39:42 UTC (rev 192298)
@@ -1254,10 +1254,8 @@
     InspectorInstrumentationCookie cookie = InspectorInstrumentation::willLayout(frame());
     AnimationUpdateBlock animationUpdateBlock(&frame().animation());
     
-    if (!allowSubtree && m_layoutRoot) {
-        m_layoutRoot->markContainingBlocksForLayout(ScheduleRelayout::No);
-        m_layoutRoot = nullptr;
-    }
+    if (!allowSubtree && m_layoutRoot)
+        convertSubtreeLayoutToFullLayout();
 
     ASSERT(frame().view() == this);
     ASSERT(frame().document());
@@ -1265,9 +1263,6 @@
     Document& document = *frame().document();
     ASSERT(!document.inPageCache());
 
-    bool subtree;
-    RenderElement* root;
-
     {
         TemporaryChange<bool> changeSchedulingEnabled(m_layoutSchedulingEnabled, false);
 
@@ -1297,33 +1292,34 @@
         // Always ensure our style info is up-to-date. This can happen in situations where
         // the layout beats any sort of style recalc update that needs to occur.
         document.updateStyleIfNeeded();
-        m_layoutPhase = InPreLayout;
-
-        subtree = m_layoutRoot;
-
-        // If there is only one ref to this view left, then its going to be destroyed as soon as we exit, 
+        // If there is only one ref to this view left, then its going to be destroyed as soon as we exit,
         // so there's no point to continuing to layout
         if (hasOneRef())
             return;
 
-        root = subtree ? m_layoutRoot : document.renderView();
-        if (!root) {
-            // FIXME: Do we need to set m_size here?
-            return;
-        }
-
         // Close block here so we can set up the font cache purge preventer, which we will still
         // want in scope even after we want m_layoutSchedulingEnabled to be restored again.
         // The next block sets m_layoutSchedulingEnabled back to false once again.
     }
 
-    RenderLayer* layer;
+    m_layoutPhase = InPreLayout;
 
+    RenderLayer* layer = nullptr;
+    bool subtree = false;
+    RenderElement* root = nullptr;
+
     ++m_nestedLayoutCount;
 
     {
         TemporaryChange<bool> changeSchedulingEnabled(m_layoutSchedulingEnabled, false);
 
+        autoSizeIfEnabled();
+
+        root = m_layoutRoot ? m_layoutRoot : document.renderView();
+        if (!root)
+            return;
+        subtree = m_layoutRoot;
+
         if (!m_layoutRoot) {
             auto* body = document.bodyOrFrameset();
             if (body && body->renderer()) {
@@ -1338,11 +1334,9 @@
 #ifdef INSTRUMENT_LAYOUT_SCHEDULING
             if (m_firstLayout && !frame().ownerElement())
                 printf("Elapsed time before first layout: %lld\n", document.elapsedTime().count());
-#endif        
+#endif
         }
 
-        autoSizeIfEnabled();
-
         m_needsFullRepaint = !subtree && (m_firstLayout || downcast<RenderView>(*root).printing());
 
         if (!subtree) {
@@ -2560,6 +2554,12 @@
         adjustTiledBackingCoverage();
     }
 }
+void FrameView::convertSubtreeLayoutToFullLayout()
+{
+    ASSERT(m_layoutRoot);
+    m_layoutRoot->markContainingBlocksForLayout(ScheduleRelayout::No);
+    m_layoutRoot = nullptr;
+}
 
 void FrameView::layoutTimerFired()
 {
@@ -2576,10 +2576,8 @@
     // too many false assertions.  See <rdar://problem/7218118>.
     ASSERT(frame().view() == this);
 
-    if (m_layoutRoot) {
-        m_layoutRoot->markContainingBlocksForLayout(ScheduleRelayout::No);
-        m_layoutRoot = nullptr;
-    }
+    if (m_layoutRoot)
+        convertSubtreeLayoutToFullLayout();
     if (!m_layoutSchedulingEnabled)
         return;
     if (!needsLayout())
@@ -2668,8 +2666,7 @@
     }
 
     // Just do a full relayout.
-    m_layoutRoot->markContainingBlocksForLayout(ScheduleRelayout::No);
-    m_layoutRoot = nullptr;
+    convertSubtreeLayoutToFullLayout();
     newRelayoutRoot.markContainingBlocksForLayout(ScheduleRelayout::No);
     InspectorInstrumentation::didInvalidateLayout(frame());
 }
@@ -3212,6 +3209,8 @@
     if (!documentView || !documentElement)
         return;
 
+    if (m_layoutRoot)
+        convertSubtreeLayoutToFullLayout();
     // Start from the minimum size and allow it to grow.
     resize(m_minAutoSize.width(), m_minAutoSize.height());
 

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/page/FrameView.h (192297 => 192298)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/page/FrameView.h	2015-11-11 08:26:24 UTC (rev 192297)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/page/FrameView.h	2015-11-11 08:39:42 UTC (rev 192298)
@@ -676,6 +676,8 @@
     void removeFromAXObjectCache();
     void notifyWidgets(WidgetNotification);
 
+    void convertSubtreeLayoutToFullLayout();
+
     RenderElement* viewportRenderer() const;
 
     HashSet<Widget*> m_widgetsInRenderTree;

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.cpp (192297 => 192298)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.cpp	2015-11-11 08:26:24 UTC (rev 192297)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.cpp	2015-11-11 08:39:42 UTC (rev 192298)
@@ -2399,6 +2399,14 @@
     frame()->loader().reload(endToEnd);
 }
 
+void Internals::enableAutoSizeMode(bool enabled, int minimumWidth, int minimumHeight, int maximumWidth, int maximumHeight)
+{
+    Document* document = contextDocument();
+    if (!document || !document->view())
+        return;
+    document->view()->enableAutoSizeMode(enabled, IntSize(minimumWidth, minimumHeight), IntSize(maximumWidth, maximumHeight));
+}
+
 #if ENABLE(ENCRYPTED_MEDIA_V2)
 void Internals::initializeMockCDM()
 {

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.h (192297 => 192298)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.h	2015-11-11 08:26:24 UTC (rev 192297)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.h	2015-11-11 08:39:42 UTC (rev 192298)
@@ -338,6 +338,8 @@
 
     void forceReload(bool endToEnd);
 
+    void enableAutoSizeMode(bool enabled, int minimumWidth, int minimumHeight, int maximumWidth, int maximumHeight);
+
 #if ENABLE(ENCRYPTED_MEDIA_V2)
     void initializeMockCDM();
 #endif

Modified: releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.idl (192297 => 192298)


--- releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.idl	2015-11-11 08:26:24 UTC (rev 192297)
+++ releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.idl	2015-11-11 08:39:42 UTC (rev 192298)
@@ -330,6 +330,8 @@
 
     void forceReload(boolean endToEnd);
 
+    void enableAutoSizeMode(boolean enabled, long minimumWidth, long minimumHeight, long maximumWidth, long maximumHeight);
+
     [Conditional=VIDEO] void simulateAudioInterruption(Node node);
     [Conditional=VIDEO, RaisesException] boolean mediaElementHasCharacteristic(Node node, DOMString characteristic);
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to