Changeset 223127 in webkit


Ignore:
Timestamp:
Oct 10, 2017 3:38:37 AM (6 years ago)
Author:
Antti Koivisto
Message:

Add isContinuation bit
https://bugs.webkit.org/show_bug.cgi?id=178084

Reviewed by Zalan Bujtas.

Currently continuations are identified indirectly by comparing renderer pointer with the element renderer pointer.
This is bug prone and fails to cover anonymous continuations.

  • accessibility/AccessibilityRenderObject.cpp:

(WebCore::firstChildConsideringContinuation):
(WebCore::startOfContinuations):
(WebCore::firstChildIsInlineContinuation):
(WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored const):

Ignore first-letter fragment. This worked before because first-letter renderers
were mistakenly considered inline element continuations (see below).

  • rendering/RenderBoxModelObject.cpp:

(WebCore::RenderBoxModelObject::setContinuation):

  • rendering/RenderElement.cpp:

(WebCore::RenderElement::RenderElement):

  • rendering/RenderElement.h:

(WebCore::RenderElement::hasContinuation const):
(WebCore::RenderElement::isContinuation const):
(WebCore::RenderElement::setIsContinuation):

The new bit.

(WebCore::RenderElement::isElementContinuation const):
(WebCore::RenderElement::isInlineElementContinuation const):

  • rendering/RenderInline.cpp:

(WebCore::RenderInline::addChildIgnoringContinuation):
(WebCore::RenderInline::cloneAsContinuation const):
(WebCore::RenderInline::splitInlines):
(WebCore::RenderInline::childBecameNonInline):
(WebCore::RenderInline::clone const): Deleted.

  • rendering/RenderInline.h:
  • rendering/RenderObject.h:

(WebCore::RenderObject::isAnonymousBlock const):
(WebCore::RenderObject::isElementContinuation const): Deleted.

The old continuation test was 'node() && node()->renderer() != this'
This was fragile as nulling the renderer will make it fail.
It was also wrong for first-letter renderers (isElementContinuation was true for them).

(WebCore::RenderObject::isInlineElementContinuation const): Deleted.

Move to RenderElement.

(WebCore::RenderObject::isBlockElementContinuation const): Deleted.

Location:
trunk/Source/WebCore
Files:
8 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r223126 r223127  
     12017-10-09  Antti Koivisto  <antti@apple.com>
     2
     3        Add isContinuation bit
     4        https://bugs.webkit.org/show_bug.cgi?id=178084
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        Currently continuations are identified indirectly by comparing renderer pointer with the element renderer pointer.
     9        This is bug prone and fails to cover anonymous continuations.
     10
     11        * accessibility/AccessibilityRenderObject.cpp:
     12        (WebCore::firstChildConsideringContinuation):
     13        (WebCore::startOfContinuations):
     14        (WebCore::firstChildIsInlineContinuation):
     15        (WebCore::AccessibilityRenderObject::computeAccessibilityIsIgnored const):
     16
     17            Ignore first-letter fragment. This worked before because first-letter renderers
     18            were mistakenly considered inline element continuations (see below).
     19
     20        * rendering/RenderBoxModelObject.cpp:
     21        (WebCore::RenderBoxModelObject::setContinuation):
     22        * rendering/RenderElement.cpp:
     23        (WebCore::RenderElement::RenderElement):
     24        * rendering/RenderElement.h:
     25        (WebCore::RenderElement::hasContinuation const):
     26        (WebCore::RenderElement::isContinuation const):
     27        (WebCore::RenderElement::setIsContinuation):
     28
     29            The new bit.
     30
     31        (WebCore::RenderElement::isElementContinuation const):
     32        (WebCore::RenderElement::isInlineElementContinuation const):
     33        * rendering/RenderInline.cpp:
     34        (WebCore::RenderInline::addChildIgnoringContinuation):
     35        (WebCore::RenderInline::cloneAsContinuation const):
     36        (WebCore::RenderInline::splitInlines):
     37        (WebCore::RenderInline::childBecameNonInline):
     38        (WebCore::RenderInline::clone const): Deleted.
     39        * rendering/RenderInline.h:
     40        * rendering/RenderObject.h:
     41        (WebCore::RenderObject::isAnonymousBlock const):
     42        (WebCore::RenderObject::isElementContinuation const): Deleted.
     43
     44            The old continuation test was 'node() && node()->renderer() != this'
     45            This was fragile as nulling the renderer will make it fail.
     46            It was also wrong for first-letter renderers (isElementContinuation was true for them).
     47
     48        (WebCore::RenderObject::isInlineElementContinuation const): Deleted.
     49
     50            Move to RenderElement.
     51
     52        (WebCore::RenderObject::isBlockElementContinuation const): Deleted.
     53
    1542017-10-10  Joanmarie Diggs  <jdiggs@igalia.com>
    255
  • trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

    r222422 r223127  
    184184    // anonymous parent, because everything has already been linked up via continuation.
    185185    // CSS first-letter selector is an example of this case.
    186     if (renderer.isAnonymous() && firstChild && firstChild->isInlineElementContinuation())
     186    if (renderer.isAnonymous() && is<RenderElement>(firstChild) && downcast<RenderElement>(*firstChild).isInlineElementContinuation())
    187187        firstChild = nullptr;
    188188   
     
    249249static inline RenderInline* startOfContinuations(RenderObject& renderer)
    250250{
    251     if (renderer.isInlineElementContinuation() && is<RenderInline>(renderer.node()->renderer()))
     251    if (!is<RenderElement>(renderer))
     252        return nullptr;
     253    auto& renderElement = downcast<RenderElement>(renderer);
     254    if (renderElement.isInlineElementContinuation() && is<RenderInline>(renderElement.element()->renderer()))
    252255        return downcast<RenderInline>(renderer.node()->renderer());
    253256
    254257    // Blocks with a previous continuation always have a next continuation
    255     if (is<RenderBlock>(renderer) && downcast<RenderBlock>(renderer).inlineElementContinuation())
    256         return downcast<RenderInline>(downcast<RenderBlock>(renderer).inlineElementContinuation()->element()->renderer());
     258    if (is<RenderBlock>(renderElement) && downcast<RenderBlock>(renderElement).inlineElementContinuation())
     259        return downcast<RenderInline>(downcast<RenderBlock>(renderElement).inlineElementContinuation()->element()->renderer());
    257260
    258261    return nullptr;
     
    308311{
    309312    RenderObject* child = renderer.firstChild();
    310     return child && child->isInlineElementContinuation();
     313    return is<RenderElement>(child) && downcast<RenderElement>(*child).isInlineElementContinuation();
    311314}
    312315
     
    12471250            if (altTextInclusion == IncludeObject)
    12481251                return false;
     1252            if (downcast<RenderTextFragment>(renderText).firstLetter())
     1253                return true;
    12491254        }
    12501255
  • trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

    r223003 r223127  
    24552455void RenderBoxModelObject::setContinuation(RenderBoxModelObject* continuation)
    24562456{
     2457    ASSERT(!continuation || continuation->isContinuation());
    24572458    if (continuation)
    24582459        continuationMap().set(this, makeWeakPtr(continuation));
  • trunk/Source/WebCore/rendering/RenderElement.cpp

    r222740 r223127  
    106106    , m_hasCounterNodeMap(false)
    107107    , m_hasContinuation(false)
     108    , m_isContinuation(false)
    108109    , m_hasValidCachedFirstLineStyle(false)
    109110    , m_renderBlockHasMarginBeforeQuirk(false)
  • trunk/Source/WebCore/rendering/RenderElement.h

    r222679 r223127  
    205205
    206206    bool childRequiresTable(const RenderObject& child) const;
    207     bool hasContinuation() const { return m_hasContinuation; }
    208207
    209208#if ENABLE(TEXT_AUTOSIZING)
     
    222221    // the child.
    223222    virtual void updateAnonymousChildStyle(const RenderObject&, RenderStyle&) const { };
     223
     224    bool hasContinuation() const { return m_hasContinuation; }
     225    bool isContinuation() const { return m_isContinuation; }
     226    void setIsContinuation() { m_isContinuation = true; }
     227    bool isElementContinuation() const { return isContinuation() && !isAnonymous(); }
     228    bool isInlineElementContinuation() const { return isElementContinuation() && isInline(); }
    224229
    225230protected:
     
    333338    unsigned m_hasCounterNodeMap : 1;
    334339    unsigned m_hasContinuation : 1;
     340    unsigned m_isContinuation : 1;
    335341    mutable unsigned m_hasValidCachedFirstLineStyle : 1;
    336342
  • trunk/Source/WebCore/rendering/RenderInline.cpp

    r222679 r223127  
    343343        auto newBox = createRenderer<RenderBlockFlow>(document(), WTFMove(newStyle));
    344344        newBox->initializeStyle();
     345        newBox->setIsContinuation();
    345346        RenderBoxModelObject* oldContinuation = continuation();
    346347        setContinuation(newBox.get());
     
    355356}
    356357
    357 RenderPtr<RenderInline> RenderInline::clone() const
     358RenderPtr<RenderInline> RenderInline::cloneAsContinuation() const
    358359{
    359360    RenderPtr<RenderInline> cloneInline = createRenderer<RenderInline>(*element(), RenderStyle::clone(style()));
     
    361362    cloneInline->setFragmentedFlowState(fragmentedFlowState());
    362363    cloneInline->setHasOutlineAutoAncestor(hasOutlineAutoAncestor());
     364    cloneInline->setIsContinuation();
    363365    return cloneInline;
    364366}
     
    369371{
    370372    // Create a clone of this inline.
    371     RenderPtr<RenderInline> cloneInline = clone();
     373    RenderPtr<RenderInline> cloneInline = cloneAsContinuation();
    372374#if ENABLE(FULLSCREEN_API)
    373375    // If we're splitting the inline containing the fullscreened element,
     
    436438            // Create a new clone.
    437439            RenderPtr<RenderInline> cloneChild = WTFMove(cloneInline);
    438             cloneInline = downcast<RenderInline>(*current).clone();
     440            cloneInline = downcast<RenderInline>(*current).cloneAsContinuation();
    439441
    440442            // Insert our child clone as the first child.
     
    13761378    // We have to split the parent flow.
    13771379    auto newBox = containingBlock()->createAnonymousBlock();
     1380    newBox->setIsContinuation();
    13781381    RenderBoxModelObject* oldContinuation = continuation();
    13791382    setContinuation(newBox.get());
  • trunk/Source/WebCore/rendering/RenderInline.h

    r223072 r223127  
    168168#endif
    169169   
    170     RenderPtr<RenderInline> clone() const;
     170    RenderPtr<RenderInline> cloneAsContinuation() const;
    171171
    172172    void paintOutlineForLine(GraphicsContext&, const LayoutPoint&, const LayoutRect& prevLine, const LayoutRect& thisLine, const LayoutRect& nextLine, const Color&);
  • trunk/Source/WebCore/rendering/RenderObject.h

    r223072 r223127  
    412412            ;
    413413    }
    414     bool isElementContinuation() const { return node() && node()->renderer() != this; }
    415     bool isInlineElementContinuation() const { return isElementContinuation() && isInline(); }
    416     bool isBlockElementContinuation() const { return isElementContinuation() && !isInline(); }
    417414
    418415    bool isFloating() const { return m_bitfields.floating(); }
Note: See TracChangeset for help on using the changeset viewer.