Changeset 83221 in webkit


Ignore:
Timestamp:
Apr 7, 2011 3:33:32 PM (13 years ago)
Author:
hyatt@apple.com
Message:

https://bugs.webkit.org/show_bug.cgi?id=57736

Reviewed by Dan Bernstein.

Crash on openstreetmap.org while using the map. Fix a bad interaction between the positioned movement layout
optimization and the simplified layout optimization that could lead to blocks remaining marked as dirty when
layout finished. This would eventually lead to an inability to properly determine the correct layout root and
would cause a deleted root to be used later on.

Added fast/block/positioning/complex-positioned-movement.html.

Source/WebCore:

  • page/FrameView.cpp:

(WebCore::FrameView::scheduleRelayoutOfSubtree):
Add asserts to catch cases in the future where a layout root is set that has a dirty containing block.

  • rendering/RenderBlock.cpp:

(WebCore::RenderBlock::simplifiedLayout):
Change simplified layout so that the positioned movement optimization no longer clears the other layout
flags. This will ensure that we still lay out our descendants if they need it.

(WebCore::RenderBlock::layoutPositionedObjects):
Changed to clear our layout flags now if the positioned movement is successful, since tryLayoutDoingPositionedMovementOnly
no longer does the clear.

  • rendering/RenderBox.h:

(WebCore::RenderBox::tryLayoutDoingPositionedMovementOnly):
tryLayoutDoingPositionedMovementOnly now returns a boolean indicating success or failure. On success it no longer
does setNeedsLayout(false), but instead will let the caller take care of it. This way the caller can still look at
the other flags in case other kinds of layout are still needed.

  • rendering/RenderObject.h:

(WebCore::RenderObject::setNeedsPositionedMovementLayout):
(WebCore::RenderObject::setNeedsSimplifiedNormalFlowLayout):
Changed these methods to only set their respective flags and not to try to be clever about avoiding propagation.

LayoutTests:

  • fast/block/positioning/complex-positioned-movement.html: Added.
  • platform/mac/fast/block/positioning/complex-positioned-movement-expected.checksum: Added.
  • platform/mac/fast/block/positioning/complex-positioned-movement-expected.png: Added.
  • platform/mac/fast/block/positioning/complex-positioned-movement-expected.txt: Added.
Location:
trunk
Files:
4 added
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r83216 r83221  
     12011-04-07  David Hyatt  <hyatt@apple.com>
     2
     3        Reviewed by Dan Bernstein.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=57736
     6       
     7        Crash on openstreetmap.org while using the map. Fix a bad interaction between the positioned movement layout
     8        optimization and the simplified layout optimization that could lead to blocks remaining marked as dirty when
     9        layout finished. This would eventually lead to an inability to properly determine the correct layout root and
     10        would cause a deleted root to be used later on.
     11
     12        Added fast/block/positioning/complex-positioned-movement.html.
     13
     14        * fast/block/positioning/complex-positioned-movement.html: Added.
     15        * platform/mac/fast/block/positioning/complex-positioned-movement-expected.checksum: Added.
     16        * platform/mac/fast/block/positioning/complex-positioned-movement-expected.png: Added.
     17        * platform/mac/fast/block/positioning/complex-positioned-movement-expected.txt: Added.
     18
    1192011-04-07  Dan Bernstein  <mitz@apple.com>
    220
  • trunk/Source/WebCore/ChangeLog

    r83220 r83221  
     12011-04-07  David Hyatt  <hyatt@apple.com>
     2
     3        Reviewed by Dan Bernstein.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=57736
     6       
     7        Crash on openstreetmap.org while using the map. Fix a bad interaction between the positioned movement layout
     8        optimization and the simplified layout optimization that could lead to blocks remaining marked as dirty when
     9        layout finished. This would eventually lead to an inability to properly determine the correct layout root and
     10        would cause a deleted root to be used later on.
     11
     12        Added fast/block/positioning/complex-positioned-movement.html.
     13
     14        * page/FrameView.cpp:
     15        (WebCore::FrameView::scheduleRelayoutOfSubtree):
     16        Add asserts to catch cases in the future where a layout root is set that has a dirty containing block.
     17   
     18        * rendering/RenderBlock.cpp:
     19        (WebCore::RenderBlock::simplifiedLayout):
     20        Change simplified layout so that the positioned movement optimization no longer clears the other layout
     21        flags. This will ensure that we still lay out our descendants if they need it.
     22       
     23        (WebCore::RenderBlock::layoutPositionedObjects):
     24        Changed to clear our layout flags now if the positioned movement is successful, since tryLayoutDoingPositionedMovementOnly
     25        no longer does the clear.
     26   
     27        * rendering/RenderBox.h:
     28        (WebCore::RenderBox::tryLayoutDoingPositionedMovementOnly):
     29        tryLayoutDoingPositionedMovementOnly now returns a boolean indicating success or failure.  On success it no longer
     30        does setNeedsLayout(false), but instead will let the caller take care of it. This way the caller can still look at
     31        the other flags in case other kinds of layout are still needed.
     32   
     33        * rendering/RenderObject.h:
     34        (WebCore::RenderObject::setNeedsPositionedMovementLayout):
     35        (WebCore::RenderObject::setNeedsSimplifiedNormalFlowLayout):
     36        Changed these methods to only set their respective flags and not to try to be clever about avoiding propagation.
     37
    1382011-04-07  Anna Cavender  <annacc@chromium.org>
    239
  • trunk/Source/WebCore/page/FrameView.cpp

    r83022 r83221  
    16731673                // Keep the current root
    16741674                relayoutRoot->markContainingBlocksForLayout(false, m_layoutRoot);
     1675                ASSERT(!m_layoutRoot->container() || !m_layoutRoot->container()->needsLayout());
    16751676            } else if (m_layoutRoot && isObjectAncestorContainerOf(relayoutRoot, m_layoutRoot)) {
    16761677                // Re-root at relayoutRoot
    16771678                m_layoutRoot->markContainingBlocksForLayout(false, relayoutRoot);
    16781679                m_layoutRoot = relayoutRoot;
     1680                ASSERT(!m_layoutRoot->container() || !m_layoutRoot->container()->needsLayout());
    16791681            } else {
    16801682                // Just do a full relayout
     
    16881690        int delay = m_frame->document()->minimumLayoutDelay();
    16891691        m_layoutRoot = relayoutRoot;
     1692        ASSERT(!m_layoutRoot->container() || !m_layoutRoot->container()->needsLayout());
    16901693        m_delayedLayout = delay != 0;
    16911694        m_layoutTimer.startOneShot(delay * 0.001);
  • trunk/Source/WebCore/rendering/RenderBlock.cpp

    r83099 r83221  
    21152115    LayoutStateMaintainer statePusher(view(), this, IntSize(x(), y()), hasColumns() || hasTransform() || hasReflection() || style()->isFlippedBlocksWritingMode());
    21162116   
    2117     if (needsPositionedMovementLayout()) {
    2118         tryLayoutDoingPositionedMovementOnly();
    2119         if (needsLayout())
    2120             return false;
    2121     }
     2117    if (needsPositionedMovementLayout() && !tryLayoutDoingPositionedMovementOnly())
     2118        return false;
    21222119
    21232120    // Lay out positioned descendants or objects that just need to recompute overflow.
     
    21752172        // We don't have to do a full layout.  We just have to update our position. Try that first. If we have shrink-to-fit width
    21762173        // and we hit the available width constraint, the layoutIfNeeded() will catch it and do a full layout.
    2177         if (r->needsPositionedMovementLayoutOnly())
    2178             r->tryLayoutDoingPositionedMovementOnly();
     2174        if (r->needsPositionedMovementLayoutOnly() && r->tryLayoutDoingPositionedMovementOnly())
     2175            r->setNeedsLayout(false);
    21792176        r->layoutIfNeeded();
    21802177    }
  • trunk/Source/WebCore/rendering/RenderBox.h

    r82611 r83221  
    351351    // Called when a positioned object moves but doesn't necessarily change size.  A simplified layout is attempted
    352352    // that just updates the object's position. If the size does change, the object remains dirty.
    353     void tryLayoutDoingPositionedMovementOnly()
     353    bool tryLayoutDoingPositionedMovementOnly()
    354354    {
    355355        int oldWidth = width();
     
    357357        // If we shrink to fit our width may have changed, so we still need full layout.
    358358        if (oldWidth != width())
    359             return;
     359            return false;
    360360        computeLogicalHeight();
    361         setNeedsLayout(false);
     361        return true;
    362362    }
    363363
  • trunk/Source/WebCore/rendering/RenderObject.h

    r83038 r83221  
    960960inline void RenderObject::setNeedsPositionedMovementLayout()
    961961{
    962     bool alreadyNeededLayout = needsLayout();
     962    bool alreadyNeededLayout = m_needsPositionedMovementLayout;
    963963    m_needsPositionedMovementLayout = true;
     964    ASSERT(!isSetNeedsLayoutForbidden());
    964965    if (!alreadyNeededLayout) {
    965966        markContainingBlocksForLayout();
     
    971972inline void RenderObject::setNeedsSimplifiedNormalFlowLayout()
    972973{
    973     bool alreadyNeededLayout = needsLayout();
     974    bool alreadyNeededLayout = m_needsSimplifiedNormalFlowLayout;
    974975    m_needsSimplifiedNormalFlowLayout = true;
     976    ASSERT(!isSetNeedsLayoutForbidden());
    975977    if (!alreadyNeededLayout) {
    976978        markContainingBlocksForLayout();
Note: See TracChangeset for help on using the changeset viewer.