Changeset 230313 in webkit


Ignore:
Timestamp:
Apr 5, 2018 2:14:31 PM (6 years ago)
Author:
Alan Bujtas
Message:

Folding anonymous blocks should not result in deleting content.
​https://bugs.webkit.org/show_bug.cgi?id=184339
<rdar://problem/37327428>

Reviewed by Antti Koivisto.

Source/WebCore:

While folding multiple anonymous blocks (moving the children from next sibling over to previous sibling)
we should ensure that the block we are about to destroy does not gain new descendants.
In case of 4 sibling anonymous blocks (A B C D), while destroying B

  1. we move C's children to A and destroy C.
  2. While destroying C, we notice B and C as sibling anonymous blocks and we move

D's children over to B (even though B is going to be destroyed as we climb back on the stack).

In this patch, B is detached from the tree before we start moving renderers around so that a subsequent folding won't
find B anymore as a candidate.

Test: fast/block/crash-while-folding-anonymous-blocks.html

  • rendering/updating/RenderTreeBuilderBlock.cpp:

(WebCore::RenderTreeBuilder::Block::detach):

LayoutTests:

  • fast/block/crash-when-subtree-is-still-attached-expected.txt: Progressing. This test does not

intend to remove "foobar" text at all.

  • fast/block/crash-while-folding-anonymous-blocks-expected.txt: Added.
  • fast/block/crash-while-folding-anonymous-blocks.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r230304 r230313  
     12018-04-05  Zalan Bujtas  <zalan@apple.com>
     2
     3        Folding anonymous blocks should not result in deleting content.
     4        https://bugs.webkit.org/show_bug.cgi?id=184339
     5        <rdar://problem/37327428>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        * fast/block/crash-when-subtree-is-still-attached-expected.txt: Progressing. This test does not
     10        intend to remove "foobar" text at all.
     11        * fast/block/crash-while-folding-anonymous-blocks-expected.txt: Added.
     12        * fast/block/crash-while-folding-anonymous-blocks.html: Added.
     13
    1142018-03-21  Ryan Haddad  <ryanhaddad@apple.com>
    215

  • trunk/LayoutTests/fast/block/crash-when-subtree-is-still-attached-expected.txt

    r228606 r230313  
    11PASS if
    22no crash.
     3foobar
    34

  • trunk/Source/WebCore/ChangeLog

    r230312 r230313  
     12018-04-05  Zalan Bujtas  <zalan@apple.com>
     2
     3        Folding anonymous blocks should not result in deleting content.
     4        https://bugs.webkit.org/show_bug.cgi?id=184339
     5        <rdar://problem/37327428>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        While folding multiple anonymous blocks (moving the children from next sibling over to previous sibling)
     10        we should ensure that the block we are about to destroy does not gain new descendants.
     11        In case of 4 sibling anonymous blocks (A B C D), while destroying B
     12        1. we move C's children to A and destroy C.
     13        2. While destroying C, we notice B and C as sibling anonymous blocks and we move
     14        D's children over to B (even though B is going to be destroyed as we climb back on the stack).
     15       
     16        In this patch, B is detached from the tree before we start moving renderers around so that a subsequent folding won't
     17        find B anymore as a candidate.
     18
     19        Test: fast/block/crash-while-folding-anonymous-blocks.html
     20
     21        * rendering/updating/RenderTreeBuilderBlock.cpp:
     22        (WebCore::RenderTreeBuilder::Block::detach):
     23
    1242018-04-05  Andy Estes  <aestes@apple.com>
    225

  • trunk/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp

    r229474 r230313  
    281281    // If this child is a block, and if our previous and next siblings are both anonymous blocks
    282282    // with inline content, then we can fold the inline content back together.
    283     RenderObject* prev = oldChild.previousSibling();
    284     RenderObject* next = oldChild.nextSibling();
    285     bool canMergeAnonymousBlocks = canMergeContiguousAnonymousBlocks(oldChild, prev, next);
     283    auto prev = makeWeakPtr(oldChild.previousSibling());
     284    auto next = makeWeakPtr(oldChild.nextSibling());
     285    bool canMergeAnonymousBlocks = canMergeContiguousAnonymousBlocks(oldChild, prev.get(), next.get());
     286
     287    parent.invalidateLineLayoutPath();
     288
     289    auto takenChild = m_builder.detachFromRenderElement(parent, oldChild);
     290
    286291    if (canMergeAnonymousBlocks && prev && next) {
    287292        prev->setNeedsLayoutAndPrefWidthsRecalc();
    … …  
    321326            nextBlock.deleteLines();
    322327            m_builder.destroy(nextBlock);
    323             next = nullptr;
    324         }
    325     }
    326 
    327     parent.invalidateLineLayoutPath();
    328 
    329     auto takenChild = m_builder.detachFromRenderElement(parent, oldChild);
    330 
    331     RenderObject* child = prev ? prev : next;
     328        }
     329    }
     330
     331    RenderObject* child = prev ? prev.get() : next.get();
    332332    if (canMergeAnonymousBlocks && child && !child->previousSibling() && !child->nextSibling() && parent.canDropAnonymousBlockChild()) {
    333333        // The removal has knocked us down to containing only a single anonymous
Note: See TracChangeset for help on using the changeset viewer.