Changeset 220646 in webkit


Ignore:
Timestamp:
Aug 14, 2017 12:10:17 AM (7 years ago)
Author:
Antti Koivisto
Message:

[Render Tree Mutation] First letter should not mutate the render tree while in layout.
​https://bugs.webkit.org/show_bug.cgi?id=163848
Source/WebCore:

Reviewed by Zalan Bujtas.

RenderBlock::updateFirstLetter shouldn't be called during layout. Instead it should
be invoked by the RenderTreeUpdater.

With this future patches can move updateFirstLetter() and the related functions
completely out of the render tree.

  • rendering/RenderBlock.cpp:

(WebCore::RenderBlock::layout):

No more updateFirstLetter calls during layout...

(WebCore::RenderBlock::computePreferredLogicalWidths):

...or preferred width computation.

(WebCore::RenderBlock::updateFirstLetter):

  • rendering/RenderBlock.h:
  • rendering/RenderRubyRun.cpp:

(WebCore::RenderRubyRun::updateFirstLetter):

  • rendering/RenderRubyRun.h:
  • rendering/RenderTable.cpp:

(WebCore::RenderTable::updateFirstLetter):

  • rendering/RenderTable.h:
  • rendering/svg/RenderSVGText.cpp:

(WebCore::RenderSVGText::updateFirstLetter):

  • rendering/svg/RenderSVGText.h:
  • style/RenderTreeUpdater.cpp:

(WebCore::RenderTreeUpdater::popParent):

Call updateFirstLetter when closing the element. All of of descedant renderers are known here
so this can be resolved correctly.

LayoutTests:

<rdar://problem/33402718>

Reviewed by Zalan Bujtas.

  • fast/text-autosizing/ios/first-letter-expected.html: Added.

Turn into reftest for easier debugging and robustness.

  • imported/blink/fast/css/first-letter-range-insert-expected.txt:

This is crash-or-assert test and the output change here doesn't matter.

  • platform/ios/fast/text-autosizing/ios/first-letter-expected.txt: Removed.
  • platform/mac/fast/text-autosizing/ios/first-letter-expected.txt: Removed.
Location:
trunk
Files:
1 added
2 deleted
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r220639 r220646  
     12017-08-14  Antti Koivisto  <antti@apple.com>
     2
     3        [Render Tree Mutation] First letter should not mutate the render tree while in layout.
     4        https://bugs.webkit.org/show_bug.cgi?id=163848
     5        <rdar://problem/33402718>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        * fast/text-autosizing/ios/first-letter-expected.html: Added.
     10
     11            Turn into reftest for easier debugging and robustness.
     12
     13        * imported/blink/fast/css/first-letter-range-insert-expected.txt:
     14
     15            This is crash-or-assert test and the output change here doesn't matter.
     16
     17        * platform/ios/fast/text-autosizing/ios/first-letter-expected.txt: Removed.
     18        * platform/mac/fast/text-autosizing/ios/first-letter-expected.txt: Removed.
     19
    1202017-08-13  Manuel Rego Casasnovas  <rego@igalia.com>
    221

  • trunk/LayoutTests/imported/blink/fast/css/first-letter-range-insert-expected.txt

    r190629 r220646  
     1B
    12

  • trunk/Source/WebCore/ChangeLog

    r220639 r220646  
     12017-08-14  Antti Koivisto  <antti@apple.com>
     2
     3        [Render Tree Mutation] First letter should not mutate the render tree while in layout.
     4        https://bugs.webkit.org/show_bug.cgi?id=163848
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        RenderBlock::updateFirstLetter shouldn't be called during layout. Instead it should
     9        be invoked by the RenderTreeUpdater.
     10
     11        With this future patches can move updateFirstLetter() and the related functions
     12        completely out of the render tree.
     13
     14        * rendering/RenderBlock.cpp:
     15        (WebCore::RenderBlock::layout):
     16
     17            No more updateFirstLetter calls during layout...
     18
     19        (WebCore::RenderBlock::computePreferredLogicalWidths):
     20
     21            ...or preferred width computation.
     22
     23        (WebCore::RenderBlock::updateFirstLetter):
     24        * rendering/RenderBlock.h:
     25        * rendering/RenderRubyRun.cpp:
     26        (WebCore::RenderRubyRun::updateFirstLetter):
     27        * rendering/RenderRubyRun.h:
     28        * rendering/RenderTable.cpp:
     29        (WebCore::RenderTable::updateFirstLetter):
     30        * rendering/RenderTable.h:
     31        * rendering/svg/RenderSVGText.cpp:
     32        (WebCore::RenderSVGText::updateFirstLetter):
     33        * rendering/svg/RenderSVGText.h:
     34        * style/RenderTreeUpdater.cpp:
     35        (WebCore::RenderTreeUpdater::popParent):
     36
     37            Call updateFirstLetter when closing the element. All of of descedant renderers are known here
     38            so this can be resolved correctly.
     39
    1402017-08-13  Manuel Rego Casasnovas  <rego@igalia.com>
    241

  • trunk/Source/WebCore/rendering/RenderBlock.cpp

    r219961 r220646  
    10611061    OverflowEventDispatcher dispatcher(this);
    10621062
    1063     // Update our first letter info now.
    1064     updateFirstLetter();
    1065 
    10661063    // Table cells call layoutBlock directly, so don't add any logic here.  Put code into
    10671064    // layoutBlock().
    … …  
    27432740{
    27442741    ASSERT(preferredLogicalWidthsDirty());
    2745 
    2746     // FIXME: Do not even try to reshuffle first letter renderers when we are not in layout
    2747     // until after webkit.org/b/163848 is fixed.
    2748     updateFirstLetter(view().frameView().isInRenderTreeLayout() ? RenderTreeMutationIsAllowed::Yes : RenderTreeMutationIsAllowed::No);
    27492742
    27502743    m_minPreferredLogicalWidth = 0;
    … …  
    33433336}
    33443337
    3345 void RenderBlock::updateFirstLetter(RenderTreeMutationIsAllowed mutationAllowedOrNot)
    3346 {
     3338void RenderBlock::updateFirstLetter()
     3339{
     3340    ASSERT_WITH_SECURITY_IMPLICATION(!view().layoutState());
     3341
    33473342    RenderObject* firstLetterObj;
    33483343    RenderElement* firstLetterContainer;
    … …  
    33633358    if (!is<RenderText>(*firstLetterObj))
    33643359        return;
    3365 
    3366     if (mutationAllowedOrNot != RenderTreeMutationIsAllowed::Yes)
    3367         return;
    3368     // Our layout state is not valid for the repaints we are going to trigger by
    3369     // adding and removing children of firstLetterContainer.
    3370     LayoutStateDisabler layoutStateDisabler(view());
    33713360
    33723361    createFirstLetterRenderer(firstLetterContainer, downcast<RenderText>(firstLetterObj));

  • trunk/Source/WebCore/rendering/RenderBlock.h

    r216549 r220646  
    264264    LayoutUnit collapsedMarginAfterForChild(const RenderBox& child) const;
    265265
    266     enum class RenderTreeMutationIsAllowed { Yes, No };
    267     virtual void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes);
     266    virtual void updateFirstLetter();
    268267    void getFirstLetter(RenderObject*& firstLetter, RenderElement*& firstLetterContainer, RenderObject* skipObject = nullptr);
    269268

  • trunk/Source/WebCore/rendering/RenderRubyRun.cpp

    r219544 r220646  
    102102}
    103103
    104 void RenderRubyRun::updateFirstLetter(RenderTreeMutationIsAllowed)
     104void RenderRubyRun::updateFirstLetter()
    105105{
    106106}

  • trunk/Source/WebCore/rendering/RenderRubyRun.h

    r213455 r220646  
    6161
    6262    RenderBlock* firstLineBlock() const override;
    63     void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes) override;
     63    void updateFirstLetter() override;
    6464
    6565    void getOverhang(bool firstLine, RenderObject* startRenderer, RenderObject* endRenderer, float& startOverhang, float& endOverhang) const;

  • trunk/Source/WebCore/rendering/RenderTable.cpp

    r219961 r220646  
    14861486}
    14871487
    1488 void RenderTable::updateFirstLetter(RenderTreeMutationIsAllowed)
     1488void RenderTable::updateFirstLetter()
    14891489{
    14901490}

  • trunk/Source/WebCore/rendering/RenderTable.h

    r219394 r220646  
    303303
    304304    RenderBlock* firstLineBlock() const final;
    305     void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes) final;
     305    void updateFirstLetter() final;
    306306   
    307307    void updateLogicalWidth() final;

  • trunk/Source/WebCore/rendering/RenderTableCell.cpp

    r213455 r220646  
    261261{
    262262    StackStats::LayoutCheckPoint layoutCheckPoint;
    263     updateFirstLetter();
    264263
    265264    int oldCellBaseline = cellBaselinePosition();

  • trunk/Source/WebCore/rendering/RenderTextFragment.cpp

    r207631 r220646  
    6969    RenderText::styleDidChange(diff, oldStyle);
    7070
    71     if (RenderBlock* block = blockForAccompanyingFirstLetter()) {
     71    if (RenderBlock* block = blockForAccompanyingFirstLetter())
    7272        block->mutableStyle().removeCachedPseudoStyle(FIRST_LETTER);
    73         block->updateFirstLetter();
    74     }
    7573}
    7674

  • trunk/Source/WebCore/rendering/TextAutoSizing.cpp

    r219665 r220646  
    3333#include "FontCascade.h"
    3434#include "Logging.h"
     35#include "RenderBlock.h"
    3536#include "RenderListMarker.h"
    3637#include "RenderText.h"
     38#include "RenderTextFragment.h"
    3739#include "StyleResolver.h"
    3840
    … …  
    154156        newParentStyle.fontCascade().update(&node->document().fontSelector());
    155157        parentRenderer->setStyle(WTFMove(newParentStyle));
     158    }
     159
     160    // FIXME: All render tree mutations should be done via RenderTreeUpdater.
     161    for (auto& node : m_autoSizedNodes) {
     162        auto& textRenderer = *node->renderer();
     163        if (!is<RenderTextFragment>(textRenderer))
     164            continue;
     165        auto* block = downcast<RenderTextFragment>(textRenderer).blockForAccompanyingFirstLetter();
     166        if (!block)
     167            continue;
     168        block->updateFirstLetter();
    156169    }
    157170

  • trunk/Source/WebCore/rendering/svg/RenderSVGText.cpp

    r214082 r220646  
    545545// Fix for <rdar://problem/8048875>. We should not render :first-letter CSS Style
    546546// in a SVG text element context.
    547 void RenderSVGText::updateFirstLetter(RenderTreeMutationIsAllowed)
    548 {
    549 }
    550 
    551 }
     547void RenderSVGText::updateFirstLetter()
     548{
     549}
     550
     551}

  • trunk/Source/WebCore/rendering/svg/RenderSVGText.h

    r208668 r220646  
    9292
    9393    RenderBlock* firstLineBlock() const override;
    94     void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes) override;
     94    void updateFirstLetter() override;
    9595
    9696    bool shouldHandleSubtreeMutations() const;

  • trunk/Source/WebCore/style/RenderTreeUpdater.cpp

    r220594 r220646  
    233233        updateBeforeOrAfterPseudoElement(*parent.element, AFTER);
    234234
    235         if (parent.element->hasCustomStyleResolveCallbacks() && parent.styleChange == Style::Detach && parent.element->renderer())
     235        auto* renderer = parent.element->renderer();
     236        if (is<RenderBlock>(renderer))
     237            downcast<RenderBlock>(*renderer).updateFirstLetter();
     238
     239        if (parent.element->hasCustomStyleResolveCallbacks() && parent.styleChange == Style::Detach && renderer)
    236240            parent.element->didAttachRenderers();
    237241    }
Note: See TracChangeset for help on using the changeset viewer.