Changeset 223127 in webkit

Timestamp:

Oct 10, 2017, 3:38:37 AM (8 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:

• ChangeLog (modified) (1 diff)
• accessibility/AccessibilityRenderObject.cpp (modified) (4 diffs)
• rendering/RenderBoxModelObject.cpp (modified) (1 diff)
• rendering/RenderElement.cpp (modified) (1 diff)
• rendering/RenderElement.h (modified) (3 diffs)
• rendering/RenderInline.cpp (modified) (6 diffs)
• rendering/RenderInline.h (modified) (1 diff)
• rendering/RenderObject.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-10-09  Antti Koivisto  <antti@apple.com>
+
+        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.
+
 2017-10-10  Joanmarie Diggs  <jdiggs@igalia.com>
 

trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

@@ -184 +184 @@
     // anonymous parent, because everything has already been linked up via continuation.
     // CSS first-letter selector is an example of this case.
-    if (renderer.isAnonymous() && firstChild && firstChild->isInlineElementContinuation())
+    if (renderer.isAnonymous() && is<RenderElement>(firstChild) && downcast<RenderElement>(*firstChild).isInlineElementContinuation())
         firstChild = nullptr;
     
@@ -249 +249 @@
 static inline RenderInline* startOfContinuations(RenderObject& renderer)
 {
-    if (renderer.isInlineElementContinuation() && is<RenderInline>(renderer.node()->renderer()))
+    if (!is<RenderElement>(renderer))
+        return nullptr;
+    auto& renderElement = downcast<RenderElement>(renderer);
+    if (renderElement.isInlineElementContinuation() && is<RenderInline>(renderElement.element()->renderer()))
         return downcast<RenderInline>(renderer.node()->renderer());
 
     // Blocks with a previous continuation always have a next continuation
-    if (is<RenderBlock>(renderer) && downcast<RenderBlock>(renderer).inlineElementContinuation())
-        return downcast<RenderInline>(downcast<RenderBlock>(renderer).inlineElementContinuation()->element()->renderer());
+    if (is<RenderBlock>(renderElement) && downcast<RenderBlock>(renderElement).inlineElementContinuation())
+        return downcast<RenderInline>(downcast<RenderBlock>(renderElement).inlineElementContinuation()->element()->renderer());
 
     return nullptr;
@@ -308 +311 @@
 {
     RenderObject* child = renderer.firstChild();
-    return child && child->isInlineElementContinuation();
+    return is<RenderElement>(child) && downcast<RenderElement>(*child).isInlineElementContinuation();
 }
 
@@ -1247 +1250 @@
             if (altTextInclusion == IncludeObject)
                 return false;
+            if (downcast<RenderTextFragment>(renderText).firstLetter())
+                return true;
         }
 

trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

@@ -2455 +2455 @@
 void RenderBoxModelObject::setContinuation(RenderBoxModelObject* continuation)
 {
+    ASSERT(!continuation || continuation->isContinuation());
     if (continuation)
         continuationMap().set(this, makeWeakPtr(continuation));

trunk/Source/WebCore/rendering/RenderElement.cpp

@@ -106 +106 @@
     , m_hasCounterNodeMap(false)
     , m_hasContinuation(false)
+    , m_isContinuation(false)
     , m_hasValidCachedFirstLineStyle(false)
     , m_renderBlockHasMarginBeforeQuirk(false)

trunk/Source/WebCore/rendering/RenderElement.h

@@ -205 +205 @@
 
     bool childRequiresTable(const RenderObject& child) const;
-    bool hasContinuation() const { return m_hasContinuation; }
 
 #if ENABLE(TEXT_AUTOSIZING)
@@ -222 +221 @@
     // the child.
     virtual void updateAnonymousChildStyle(const RenderObject&, RenderStyle&) const { };
+
+    bool hasContinuation() const { return m_hasContinuation; }
+    bool isContinuation() const { return m_isContinuation; }
+    void setIsContinuation() { m_isContinuation = true; }
+    bool isElementContinuation() const { return isContinuation() && !isAnonymous(); }
+    bool isInlineElementContinuation() const { return isElementContinuation() && isInline(); }
 
 protected:
@@ -333 +338 @@
     unsigned m_hasCounterNodeMap : 1;
     unsigned m_hasContinuation : 1;
+    unsigned m_isContinuation : 1;
     mutable unsigned m_hasValidCachedFirstLineStyle : 1;
 

trunk/Source/WebCore/rendering/RenderInline.cpp

@@ -343 +343 @@
         auto newBox = createRenderer<RenderBlockFlow>(document(), WTFMove(newStyle));
         newBox->initializeStyle();
+        newBox->setIsContinuation();
         RenderBoxModelObject* oldContinuation = continuation();
         setContinuation(newBox.get());
@@ -355 +356 @@
 }
 
-RenderPtr<RenderInline> RenderInline::clone() const
+RenderPtr<RenderInline> RenderInline::cloneAsContinuation() const
 {
     RenderPtr<RenderInline> cloneInline = createRenderer<RenderInline>(*element(), RenderStyle::clone(style()));
@@ -361 +362 @@
     cloneInline->setFragmentedFlowState(fragmentedFlowState());
     cloneInline->setHasOutlineAutoAncestor(hasOutlineAutoAncestor());
+    cloneInline->setIsContinuation();
     return cloneInline;
 }
@@ -369 +371 @@
 {
     // Create a clone of this inline.
-    RenderPtr<RenderInline> cloneInline = clone();
+    RenderPtr<RenderInline> cloneInline = cloneAsContinuation();
 #if ENABLE(FULLSCREEN_API)
     // If we're splitting the inline containing the fullscreened element,
@@ -436 +438 @@
             // Create a new clone.
             RenderPtr<RenderInline> cloneChild = WTFMove(cloneInline);
-            cloneInline = downcast<RenderInline>(*current).clone();
+            cloneInline = downcast<RenderInline>(*current).cloneAsContinuation();
 
             // Insert our child clone as the first child.
@@ -1376 +1378 @@
     // We have to split the parent flow.
     auto newBox = containingBlock()->createAnonymousBlock();
+    newBox->setIsContinuation();
     RenderBoxModelObject* oldContinuation = continuation();
     setContinuation(newBox.get());

trunk/Source/WebCore/rendering/RenderInline.h

@@ -168 +168 @@
 #endif
     
-    RenderPtr<RenderInline> clone() const;
+    RenderPtr<RenderInline> cloneAsContinuation() const;
 
     void paintOutlineForLine(GraphicsContext&, const LayoutPoint&, const LayoutRect& prevLine, const LayoutRect& thisLine, const LayoutRect& nextLine, const Color&);

trunk/Source/WebCore/rendering/RenderObject.h

@@ -412 +412 @@
             ;
     }
-    bool isElementContinuation() const { return node() && node()->renderer() != this; }
-    bool isInlineElementContinuation() const { return isElementContinuation() && isInline(); }
-    bool isBlockElementContinuation() const { return isElementContinuation() && !isInline(); }
 
     bool isFloating() const { return m_bitfields.floating(); }