Changeset 143322 in webkit

Timestamp:

Feb 19, 2013 5:44:42 AM (11 years ago)

Author:

abucur@adobe.com

Message:

[CSS Regions] Assertion in RenderFlowThread::removeRenderBoxRegionInfo
​https://bugs.webkit.org/show_bug.cgi?id=109914

Reviewed by David Hyatt.

Source/WebCore:

This patch moves a part of the invalidation operations inside the RenderFlowThread::invalidateRegions call. The maps
are cleared anyway at layout time but doing this earlier makes sure the flow thread is in a more consistent state
(the RenderFlowThread object has both the region chain invalidated and the regions information cleared).

RenderFlowThread::removeRenderBoxRegionInfo will check if the region chain is invalidated. If true, it means the
flow thread has a layout scheduled and the regions information is not yet reliable. In this case we just return from the
function and wait for the layout to cleanup the box information.

Test: fast/regions/remove-box-info-assert.html

• rendering/RenderFlowThread.cpp:

(WebCore::RenderFlowThread::removeRegionFromThread):
(WebCore::RenderFlowThread::invalidateRegions):
(WebCore):
(WebCore::RenderFlowThread::layout):
(WebCore::RenderFlowThread::removeRenderBoxRegionInfo):

• rendering/RenderFlowThread.h:
• rendering/RenderNamedFlowThread.cpp:

(WebCore::RenderNamedFlowThread::removeRegionFromThread):

LayoutTests:

The test creates a flow thread with two regions and a content node. We remove the region and the node from the flow thread
at the same time. When the style is updated, the region is first removed from the chain and range information for boxes
is cleared from the flow thread. When the node is removed from the flow thread it tries to delete its region box information.
The range information on the flow thread is gone so the range for the node's box is zero - nothing gets deleted. Afterwards,
an ASSERT is triggered because there's leftover box information inside the region chain.

• fast/regions/remove-box-info-assert-expected.txt: Added.
• fast/regions/remove-box-info-assert.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/regions/remove-box-info-assert-expected.txt (added)
• LayoutTests/fast/regions/remove-box-info-assert.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderFlowThread.cpp (modified) (4 diffs)
• Source/WebCore/rendering/RenderFlowThread.h (modified) (1 diff)
• Source/WebCore/rendering/RenderNamedFlowThread.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-02-19  Andrei Bucur  <abucur@adobe.com>
+
+        [CSS Regions] Assertion in RenderFlowThread::removeRenderBoxRegionInfo
+        https://bugs.webkit.org/show_bug.cgi?id=109914
+
+        Reviewed by David Hyatt.
+
+        The test creates a flow thread with two regions and a content node. We remove the region and the node from the flow thread
+        at the same time. When the style is updated, the region is first removed from the chain and range information for boxes
+        is cleared from the flow thread. When the node is removed from the flow thread it tries to delete its region box information.
+        The range information on the flow thread is gone so the range for the node's box is zero - nothing gets deleted. Afterwards,
+        an ASSERT is triggered because there's leftover box information inside the region chain.
+
+        * fast/regions/remove-box-info-assert-expected.txt: Added.
+        * fast/regions/remove-box-info-assert.html: Added.
+
 2013-02-19  Stephen White  <senorblanco@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-02-19  Andrei Bucur  <abucur@adobe.com>
+
+        [CSS Regions] Assertion in RenderFlowThread::removeRenderBoxRegionInfo
+        https://bugs.webkit.org/show_bug.cgi?id=109914
+
+        Reviewed by David Hyatt.
+
+        This patch moves a part of the invalidation operations inside the RenderFlowThread::invalidateRegions call. The maps
+        are cleared anyway at layout time but doing this earlier makes sure the flow thread is in a more consistent state
+        (the RenderFlowThread object has both the region chain invalidated and the regions information cleared).
+
+        RenderFlowThread::removeRenderBoxRegionInfo will check if the region chain is invalidated. If true, it means the
+        flow thread has a layout scheduled and the regions information is not yet reliable. In this case we just return from the
+        function and wait for the layout to cleanup the box information.
+
+        Test: fast/regions/remove-box-info-assert.html
+
+        * rendering/RenderFlowThread.cpp:
+        (WebCore::RenderFlowThread::removeRegionFromThread):
+        (WebCore::RenderFlowThread::invalidateRegions):
+        (WebCore):
+        (WebCore::RenderFlowThread::layout):
+        (WebCore::RenderFlowThread::removeRenderBoxRegionInfo):
+        * rendering/RenderFlowThread.h:
+        * rendering/RenderNamedFlowThread.cpp:
+        (WebCore::RenderNamedFlowThread::removeRegionFromThread):
+
 2013-02-19  Alberto Garcia  <agarcia@igalia.com>
 

trunk/Source/WebCore/rendering/RenderFlowThread.cpp

@@ -106 +106 @@
 {
     ASSERT(renderRegion);
-    m_regionRangeMap.clear();
     m_regionList.remove(renderRegion);
     invalidateRegions();
     checkRegionsWithStyling();
+}
+
+void RenderFlowThread::invalidateRegions()
+{
+    if (m_regionsInvalidated) {
+        ASSERT(selfNeedsLayout());
+        return;
+    }
+
+    m_regionRangeMap.clear();
+    m_breakBeforeToRegionMap.clear();
+    m_breakAfterToRegionMap.clear();
+    setNeedsLayout(true);
+
+    m_regionsInvalidated = true;
 }
 
@@ -142 +156 @@
         m_regionsHaveUniformLogicalWidth = true;
         m_regionsHaveUniformLogicalHeight = true;
-        m_regionRangeMap.clear();
-        m_breakBeforeToRegionMap.clear();
-        m_breakAfterToRegionMap.clear();
 
         LayoutUnit previousRegionLogicalWidth = 0;
@@ -153 +164 @@
                 RenderRegion* region = *iter;
                 ASSERT(!region->needsLayout());
-                
+
                 region->deleteAllRenderBoxRegionInfo();
 
@@ -436 +447 @@
     if (!hasRegions())
         return;
+
+    // If the region chain was invalidated the next layout will clear the box information from all the regions.
+    if (m_regionsInvalidated) {
+        ASSERT(selfNeedsLayout());
+        return;
+    }
 
     RenderRegion* startRegion;

trunk/Source/WebCore/rendering/RenderFlowThread.h

@@ -85 +85 @@
     void checkRegionsWithStyling();
 
-    void invalidateRegions() { m_regionsInvalidated = true; setNeedsLayout(true); }
+    void invalidateRegions();
     bool hasValidRegionInfo() const { return !m_regionsInvalidated && !m_regionList.isEmpty(); }
 

trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp

@@ -251 +251 @@
 {
     ASSERT(renderRegion);
-    m_regionRangeMap.clear();
 
     if (renderRegion->parentNamedFlowThread()) {