Changeset 192298 in webkit

Timestamp:

Nov 11, 2015 12:39:42 AM (8 years ago)

Author:

Carlos Garcia Campos

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.

Location:

releases/WebKitGTK/webkit-2.10

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dynamic/crash-subtree-layout-when-auto-size-enabled-expected.txt (added)
• LayoutTests/fast/dynamic/crash-subtree-layout-when-auto-size-enabled.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/FrameView.cpp (modified) (10 diffs)
• Source/WebCore/page/FrameView.h (modified) (1 diff)
• Source/WebCore/testing/Internals.cpp (modified) (1 diff)
• Source/WebCore/testing/Internals.h (modified) (1 diff)
• Source/WebCore/testing/Internals.idl (modified) (1 diff)

releases/WebKitGTK/webkit-2.10/LayoutTests/ChangeLog

@@ +1 @@
+2015-11-07  Zalan Bujtas  <zalan@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  <mmaxfield@apple.com>
 

releases/WebKitGTK/webkit-2.10/Source/WebCore/ChangeLog

@@ +1 @@
+2015-11-07  Zalan Bujtas  <zalan@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  <mcatanzaro@igalia.com>
 

releases/WebKitGTK/webkit-2.10/Source/WebCore/page/FrameView.cpp

@@ -1255 +1255 @@
     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);
@@ -1265 +1263 @@
     Document& document = *frame().document();
     ASSERT(!document.inPageCache());
-
-    bool subtree;
-    RenderElement* root;
 
     {
@@ -1298 +1293 @@
         // 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.
@@ -1318 +1303 @@
     }
 
-    RenderLayer* layer;
+    m_layoutPhase = InPreLayout;
+
+    RenderLayer* layer = nullptr;
+    bool subtree = false;
+    RenderElement* root = nullptr;
 
     ++m_nestedLayoutCount;
@@ -1324 +1313 @@
     {
         TemporaryChange<bool> changeSchedulingEnabled(m_layoutSchedulingEnabled, false);
+
+        autoSizeIfEnabled();
+
+        root = m_layoutRoot ? m_layoutRoot : document.renderView();
+        if (!root)
+            return;
+        subtree = m_layoutRoot;
 
         if (!m_layoutRoot) {
@@ -1339 +1335 @@
             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());
@@ -2561 +2555 @@
     }
 }
+void FrameView::convertSubtreeLayoutToFullLayout()
+{
+    ASSERT(m_layoutRoot);
+    m_layoutRoot->markContainingBlocksForLayout(ScheduleRelayout::No);
+    m_layoutRoot = nullptr;
+}
 
 void FrameView::layoutTimerFired()
@@ -2577 +2577 @@
     ASSERT(frame().view() == this);
 
-    if (m_layoutRoot) {
-        m_layoutRoot->markContainingBlocksForLayout(ScheduleRelayout::No);
-        m_layoutRoot = nullptr;
-    }
+    if (m_layoutRoot)
+        convertSubtreeLayoutToFullLayout();
     if (!m_layoutSchedulingEnabled)
         return;
@@ -2669 +2667 @@
 
     // Just do a full relayout.
-    m_layoutRoot->markContainingBlocksForLayout(ScheduleRelayout::No);
-    m_layoutRoot = nullptr;
+    convertSubtreeLayoutToFullLayout();
     newRelayoutRoot.markContainingBlocksForLayout(ScheduleRelayout::No);
     InspectorInstrumentation::didInvalidateLayout(frame());
@@ -3213 +3210 @@
         return;
 
+    if (m_layoutRoot)
+        convertSubtreeLayoutToFullLayout();
     // Start from the minimum size and allow it to grow.
     resize(m_minAutoSize.width(), m_minAutoSize.height());

releases/WebKitGTK/webkit-2.10/Source/WebCore/page/FrameView.h

@@ -677 +677 @@
     void notifyWidgets(WidgetNotification);
 
+    void convertSubtreeLayoutToFullLayout();
+
     RenderElement* viewportRenderer() const;
 

releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.cpp

@@ -2400 +2400 @@
 }
 
+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()

releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.h

@@ -339 +339 @@
     void forceReload(bool endToEnd);
 
+    void enableAutoSizeMode(bool enabled, int minimumWidth, int minimumHeight, int maximumWidth, int maximumHeight);
+
 #if ENABLE(ENCRYPTED_MEDIA_V2)
     void initializeMockCDM();

releases/WebKitGTK/webkit-2.10/Source/WebCore/testing/Internals.idl

@@ -331 +331 @@
     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);