⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 283365 in webkit


Ignore:
Timestamp:
Oct 1, 2021, 6:41:33 AM (5 years ago)
Author:
Antti Koivisto
Message:

Move InlineWalker to a file of its own and make it use RenderIterator
​https://bugs.webkit.org/show_bug.cgi?id=231069

Reviewed by Alan Bujtas.

It is confusing and overly complicated.

  • WebCore.xcodeproj/project.pbxproj:
  • layout/integration/LayoutIntegrationBoxTree.cpp:

(WebCore::LayoutIntegration::BoxTree::buildTree):

  • layout/integration/LayoutIntegrationCoverage.cpp:

(WebCore::LayoutIntegration::canUseForLineLayoutWithReason):

  • rendering/InlineIterator.h:

(WebCore::bidiNextSkippingEmptyInlines):
(WebCore::bidiNextShared): Deleted.

There is no need to share anything here.

(WebCore::bidiNextIncludingEmptyInlines): Deleted.
(WebCore::bidiFirstIncludingEmptyInlines): Deleted.
(WebCore::InlineWalker::InlineWalker): Deleted.
(WebCore::InlineWalker::root): Deleted.
(WebCore::InlineWalker::current): Deleted.
(WebCore::InlineWalker::atEndOfInline): Deleted.

None of the clients cared about the functionality where the iterator would stop at the end of a RenderInline.

(WebCore::InlineWalker::atEnd const): Deleted.
(WebCore::InlineWalker::advance): Deleted.

  • rendering/InlineWalker.h: Added.

(WebCore::InlineWalker::InlineWalker):
(WebCore::InlineWalker::current):
(WebCore::InlineWalker::atEnd const):
(WebCore::InlineWalker::advance):

  • rendering/LegacyLineLayout.cpp:

(WebCore::LegacyLineLayout::layoutLineBoxes):

  • rendering/RenderBlock.cpp:

(WebCore::RenderBlock::simplifiedNormalFlowLayout):

  • rendering/RenderIterator.h:

(WebCore::RenderIterator::operator bool const):
(WebCore::RenderConstIterator::operator bool const):
(WebCore::RenderIterator<T>::traverseNextSkippingChildren):
(WebCore::RenderConstIterator<T>::traverseNextSkippingChildren):

Location:
trunk/Source/WebCore
Files:
1 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r283362 r283365  
     12021-10-01  Antti Koivisto  <antti@apple.com>
     2
     3        Move InlineWalker to a file of its own and make it use RenderIterator
     4        https://bugs.webkit.org/show_bug.cgi?id=231069
     5
     6        Reviewed by Alan Bujtas.
     7
     8        It is confusing and overly complicated.
     9
     10        * WebCore.xcodeproj/project.pbxproj:
     11        * layout/integration/LayoutIntegrationBoxTree.cpp:
     12        (WebCore::LayoutIntegration::BoxTree::buildTree):
     13        * layout/integration/LayoutIntegrationCoverage.cpp:
     14        (WebCore::LayoutIntegration::canUseForLineLayoutWithReason):
     15        * rendering/InlineIterator.h:
     16        (WebCore::bidiNextSkippingEmptyInlines):
     17        (WebCore::bidiNextShared): Deleted.
     18
     19        There is no need to share anything here.
     20
     21        (WebCore::bidiNextIncludingEmptyInlines): Deleted.
     22        (WebCore::bidiFirstIncludingEmptyInlines): Deleted.
     23        (WebCore::InlineWalker::InlineWalker): Deleted.
     24        (WebCore::InlineWalker::root): Deleted.
     25        (WebCore::InlineWalker::current): Deleted.
     26        (WebCore::InlineWalker::atEndOfInline): Deleted.
     27
     28        None of the clients cared about the functionality where the iterator would stop at the end of a RenderInline.
     29
     30        (WebCore::InlineWalker::atEnd const): Deleted.
     31        (WebCore::InlineWalker::advance): Deleted.
     32        * rendering/InlineWalker.h: Added.
     33        (WebCore::InlineWalker::InlineWalker):
     34        (WebCore::InlineWalker::current):
     35        (WebCore::InlineWalker::atEnd const):
     36        (WebCore::InlineWalker::advance):
     37        * rendering/LegacyLineLayout.cpp:
     38        (WebCore::LegacyLineLayout::layoutLineBoxes):
     39        * rendering/RenderBlock.cpp:
     40        (WebCore::RenderBlock::simplifiedNormalFlowLayout):
     41        * rendering/RenderIterator.h:
     42        (WebCore::RenderIterator::operator bool const):
     43        (WebCore::RenderConstIterator::operator bool const):
     44        (WebCore::RenderIterator<T>::traverseNextSkippingChildren):
     45        (WebCore::RenderConstIterator<T>::traverseNextSkippingChildren):
     46
    1472021-10-01  Kimmo Kinnunen  <kkinnunen@apple.com>
    248

  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r283362 r283365  
    52695269                E47C39321FE6E10500BBBC6B /* RenderTreeBuilderList.h in Headers */ = {isa = PBXBuildFile; fileRef = E47C39281FE6E0DD00BBBC6B /* RenderTreeBuilderList.h */; };
    52705270                E47C39331FE6E10800BBBC6B /* RenderTreeBuilderMultiColumn.h in Headers */ = {isa = PBXBuildFile; fileRef = E47C39271FE6E0DC00BBBC6B /* RenderTreeBuilderMultiColumn.h */; };
     5271                E47CEBC92706EF5200B8D8F5 /* InlineWalker.h in Headers */ = {isa = PBXBuildFile; fileRef = E47CEBC72706EF5100B8D8F5 /* InlineWalker.h */; };
    52715272                E47E276516036ED200EE2AFB /* ExtensionStyleSheets.h in Headers */ = {isa = PBXBuildFile; fileRef = E47E276416036ED200EE2AFB /* ExtensionStyleSheets.h */; settings = {ATTRIBUTES = (Private, ); }; };
    52725273                E47FC8A325B84D72005495FC /* LoadSchedulingMode.h in Headers */ = {isa = PBXBuildFile; fileRef = E47FC8A125B84D71005495FC /* LoadSchedulingMode.h */; settings = {ATTRIBUTES = (Private, ); }; };
    … …  
    1651616517                E47C392B1FE6E0DF00BBBC6B /* RenderTreeBuilderList.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RenderTreeBuilderList.cpp; sourceTree = "<group>"; };
    1651716518                E47C392C1FE6E0DF00BBBC6B /* RenderTreeUpdater.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RenderTreeUpdater.h; sourceTree = "<group>"; };
     16519                E47CEBC72706EF5100B8D8F5 /* InlineWalker.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = InlineWalker.h; sourceTree = "<group>"; };
    1651816520                E47E276416036ED200EE2AFB /* ExtensionStyleSheets.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ExtensionStyleSheets.h; sourceTree = "<group>"; };
    1651916521                E47E276716036EDC00EE2AFB /* ExtensionStyleSheets.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ExtensionStyleSheets.cpp; sourceTree = "<group>"; };
    … …  
    2996629968                                930C90DC19CF965300D6C21A /* InlineIterator.cpp */,
    2996729969                                BCE789151120D6080060ECE5 /* InlineIterator.h */,
     29970                                E47CEBC72706EF5100B8D8F5 /* InlineWalker.h */,
    2996829971                                0FEC120B22BF2CC7004E9D35 /* LayerAncestorClippingStack.cpp */,
    2996929972                                0FEC120922BF2CC7004E9D35 /* LayerAncestorClippingStack.h */,
    … …  
    3299132994                                1C010701192594DF008A4201 /* InlineTextBoxStyle.h in Headers */,
    3299232995                                6F1CC1DE225F8B4900720AD2 /* InlineTextItem.h in Headers */,
     32996                                E47CEBC92706EF5200B8D8F5 /* InlineWalker.h in Headers */,
    3299332997                                1CE8864126105BF2000C816C /* InMemoryDisplayList.h in Headers */,
    3299432998                                2EB767571DA19BDF003E23B5 /* InputEvent.h in Headers */,

  • trunk/Source/WebCore/layout/integration/LayoutIntegrationBoxTree.cpp

    r283234 r283365  
    2929#if ENABLE(LAYOUT_FORMATTING_CONTEXT)
    3030
    31 #include "InlineIterator.h"
     31#include "InlineWalker.h"
    3232#include "LayoutContainerBox.h"
    3333#include "LayoutInlineTextBox.h"
    … …  
    145145
    146146    for (auto walker = InlineWalker(m_flow); !walker.atEnd(); walker.advance()) {
    147         if (walker.atEndOfInline())
    148             continue;
    149147        auto& childRenderer = *walker.current();
    150148        auto childBox = createChildBox(childRenderer);

  • trunk/Source/WebCore/layout/integration/LayoutIntegrationCoverage.cpp

    r283312 r283365  
    648648    // This currently covers <blockflow>#text</blockflow>, <blockflow>#text<br></blockflow> and mutiple (sibling) RenderText cases.
    649649    // The <blockflow><inline>#text</inline></blockflow> case is also popular and should be relatively easy to cover.
    650     for (auto walker = InlineWalker(const_cast<RenderBlockFlow&>(flow)); !walker.atEnd(); walker.advance()) {
     650    for (auto walker = InlineWalker(flow); !walker.atEnd(); walker.advance()) {
    651651        if (auto childReasons = canUseForChild(flow, *walker.current(), includeReasons))
    652652            ADD_REASONS_AND_RETURN_IF_NEEDED(childReasons, reasons, includeReasons);

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

    r281731 r283365  
    200200}
    201201
    202 // This enum is only used for bidiNextShared()
    203 enum EmptyInlineBehavior {
    204     SkipEmptyInlines,
    205     IncludeEmptyInlines,
    206 };
    207 
    208202static bool isEmptyInline(const RenderInline& renderer)
    209203{
    … …  
    222216}
    223217
    224 // FIXME: This function is misleadingly named. It has little to do with bidi.
    225 // This function will iterate over inlines within a block, optionally notifying
    226 // a bidi resolver as it enters/exits inlines (so it can push/pop embedding levels).
    227218template <class Observer>
    228 static inline RenderObject* bidiNextShared(RenderElement& root, RenderObject* current, Observer* observer = nullptr, EmptyInlineBehavior emptyInlineBehavior = SkipEmptyInlines, bool* endOfInlinePtr = nullptr)
     219static inline RenderObject* bidiNextSkippingEmptyInlines(RenderElement& root, RenderObject* current, Observer* observer)
    229220{
    230221    RenderObject* next = nullptr;
    231     // oldEndOfInline denotes if when we last stopped iterating if we were at the end of an inline.
    232     bool oldEndOfInline = endOfInlinePtr ? *endOfInlinePtr : false;
    233     bool endOfInline = false;
    234222
    235223    while (current) {
    236224        next = nullptr;
    237         if (!oldEndOfInline && !isIteratorTarget(current)) {
     225        if (!isIteratorTarget(current)) {
    238226            next = downcast<RenderElement>(*current).firstChild();
    239227            notifyObserverEnteredObject(observer, next);
    240228        }
    241229
    242         // We hit this when either current has no children, or when current is not a renderer we care about.
    243230        if (!next) {
    244             // If it is a renderer we care about, and we're doing our inline-walk, return it.
    245             if (emptyInlineBehavior == IncludeEmptyInlines && !oldEndOfInline && is<RenderInline>(*current)) {
    246                 next = current;
    247                 endOfInline = true;
    248                 break;
    249             }
    250 
    251231            while (current && current != &root) {
    252232                notifyObserverWillExitObject(observer, current);
    … …  
    259239
    260240                current = current->parent();
    261                 if (emptyInlineBehavior == IncludeEmptyInlines && current && current != &root && is<RenderInline>(*current)) {
    262                     next = current;
    263                     endOfInline = true;
    264                     break;
    265                 }
    266241            }
    267242        }
    … …  
    270245            break;
    271246
    272         if (isIteratorTarget(next)
    273             || (is<RenderInline>(*next) && (emptyInlineBehavior == IncludeEmptyInlines || isEmptyInline(downcast<RenderInline>(*next)))))
     247        if (isIteratorTarget(next) || (is<RenderInline>(*next) && isEmptyInline(downcast<RenderInline>(*next))))
    274248            break;
    275249        current = next;
    276250    }
    277251
    278     if (endOfInlinePtr)
    279         *endOfInlinePtr = endOfInline;
    280 
    281252    return next;
    282 }
    283 
    284 template <class Observer>
    285 static inline RenderObject* bidiNextSkippingEmptyInlines(RenderElement& root, RenderObject* current, Observer* observer)
    286 {
    287     // The SkipEmptyInlines callers never care about endOfInlinePtr.
    288     return bidiNextShared(root, current, observer, SkipEmptyInlines);
    289253}
    290254
    … …  
    294258    InlineBidiResolver* observer = nullptr;
    295259    return bidiNextSkippingEmptyInlines(root, current, observer);
    296 }
    297 
    298 static inline RenderObject* bidiNextIncludingEmptyInlines(RenderElement& root, RenderObject* current, bool* endOfInlinePtr = nullptr)
    299 {
    300     InlineBidiResolver* observer = nullptr; // Callers who include empty inlines, never use an observer.
    301     return bidiNextShared(root, current, observer, IncludeEmptyInlines, endOfInlinePtr);
    302260}
    303261
    … …  
    329287}
    330288
    331 // FIXME: This method needs to be renamed when bidiNext finds a good name.
    332 static inline RenderObject* bidiFirstIncludingEmptyInlines(RenderElement& root)
    333 {
    334     RenderObject* o = root.firstChild();
    335     // If either there are no children to walk, or the first one is correct
    336     // then just return it.
    337     if (!o || o->isRenderInline() || isIteratorTarget(o))
    338         return o;
    339 
    340     return bidiNextIncludingEmptyInlines(root, o);
    341 }
    342 
    343289inline void InlineIterator::fastIncrementInTextNode()
    344290{
    … …  
    374320}
    375321
    376 // FIXME: This is used by RenderBlock for simplified layout, and has nothing to do with bidi
    377 // it shouldn't use functions called bidiFirst and bidiNext.
    378 class InlineWalker {
    379 public:
    380     InlineWalker(RenderElement& root)
    381         : m_root(root)
    382         , m_current(nullptr)
    383         , m_atEndOfInline(false)
    384     {
    385         // FIXME: This class should be taught how to do the SkipEmptyInlines codepath as well.
    386         m_current = bidiFirstIncludingEmptyInlines(m_root);
    387     }
    388 
    389     RenderElement& root() { return m_root; }
    390     RenderObject* current() { return m_current; }
    391 
    392     bool atEndOfInline() { return m_atEndOfInline; }
    393     bool atEnd() const { return !m_current; }
    394 
    395     RenderObject* advance()
    396     {
    397         // FIXME: Support SkipEmptyInlines and observer parameters.
    398         m_current = bidiNextIncludingEmptyInlines(m_root, m_current, &m_atEndOfInline);
    399         return m_current;
    400     }
    401 private:
    402     RenderElement& m_root;
    403     RenderObject* m_current;
    404     bool m_atEndOfInline;
    405 };
    406322
    407323inline void InlineIterator::increment(InlineBidiResolver* resolver)

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

    r282242 r283365  
    3333#include "InlineIterator.h"
    3434#include "InlineTextBoxStyle.h"
     35#include "InlineWalker.h"
    3536#include "LayoutIntegrationRunIterator.h"
    3637#include "LegacyInlineElementBox.h"
    … …  
    17681769                        box.layoutIfNeeded();
    17691770                }
    1770             } else if (o.isTextOrLineBreak() || (is<RenderInline>(o) && !walker.atEndOfInline())) {
     1771            } else if (o.isTextOrLineBreak() || is<RenderInline>(o)) {
    17711772                if (layoutState.isFullLayout() || o.selfNeedsLayout())
    17721773                    dirtyLineBoxesForRenderer(o, layoutState.isFullLayout());

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

    r283269 r283365  
    4141#include "ImageBuffer.h"
    4242#include "InlineIterator.h"
     43#include "InlineWalker.h"
    4344#include "LayoutRepainter.h"
    4445#include "LegacyInlineElementBox.h"
    … …  
    875876    if (childrenInline()) {
    876877        ListHashSet<LegacyRootInlineBox*> lineBoxes;
    877         for (InlineWalker walker(*this); !walker.atEnd(); walker.advance()) {
     878        for (InlineWalker walker(downcast<RenderBlockFlow>(*this)); !walker.atEnd(); walker.advance()) {
    878879            RenderObject& renderer = *walker.current();
    879880            if (!renderer.isOutOfFlowPositioned() && (renderer.isReplaced() || renderer.isFloating())) {
    … …  
    882883                if (box.inlineBoxWrapper())
    883884                    lineBoxes.add(&box.inlineBoxWrapper()->root());
    884             } else if (is<RenderText>(renderer) || (is<RenderInline>(renderer) && !walker.atEndOfInline()))
     885            } else if (is<RenderText>(renderer) || is<RenderInline>(renderer))
    885886                renderer.clearNeedsLayout();
    886887        }

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

    r217893 r283365  
    4141    T* operator->();
    4242
     43    operator bool() const { return m_current; }
     44
    4345    bool operator==(const RenderIterator& other) const;
    4446    bool operator!=(const RenderIterator& other) const;
    … …  
    4648    RenderIterator& traverseNext();
    4749    RenderIterator& traverseNextSibling();
     50    RenderIterator& traverseNextSkippingChildren();
    4851    RenderIterator& traversePreviousSibling();
    4952    RenderIterator& traverseAncestor();
    … …  
    6366    const T* operator->() const;
    6467
     68    operator bool() const { return m_current; }
     69
    6570    bool operator==(const RenderConstIterator& other) const;
    6671    bool operator!=(const RenderConstIterator& other) const;
    … …  
    6873    RenderConstIterator& traverseNext();
    6974    RenderConstIterator& traverseNextSibling();
     75    RenderConstIterator& traverseNextSkippingChildren();
    7076    RenderConstIterator& traversePreviousSibling();
    7177    RenderConstIterator& traverseAncestor();
    … …  
    240246
    241247template <typename T>
     248inline RenderIterator<T>& RenderIterator<T>::traverseNextSkippingChildren()
     249{
     250    ASSERT(m_current);
     251    m_current = RenderObjectTraversal::nextSkippingChildren(*m_current, m_root);
     252    return *this;
     253}
     254
     255template <typename T>
    242256inline RenderIterator<T>& RenderIterator<T>::traversePreviousSibling()
    243257{
    … …  
    316330
    317331template <typename T>
     332inline RenderConstIterator<T>& RenderConstIterator<T>::traverseNextSkippingChildren()
     333{
     334    ASSERT(m_current);
     335    m_current = RenderObjectTraversal::nextSkippingChildren(*m_current, m_root);
     336    return *this;
     337}
     338
     339template <typename T>
    318340inline RenderConstIterator<T>& RenderConstIterator<T>::traversePreviousSibling()
    319341{
Note: See TracChangeset for help on using the changeset viewer.