Changeset 200220 in webkit

Timestamp:

Apr 28, 2016, 6:11:17 PM (9 years ago)

Author:

Alan Bujtas

Message:

Content disappears on mouse over.
​https://bugs.webkit.org/show_bug.cgi?id=157073
<rdar://problem/24389168>

Reviewed by Simon Fraser.

When a redundant inlinebox is found after constructing the line, we remove it from the tree.
The remove operation marks the ancestor tree dirty (and this newly constructed line is supposed to be clean).
This patch resets this dirty flag on the boxes all the way up to the rootlinebox.
Previously we only cleared the rootinlinebox and we ended up with dirty inlineflowboxes.

Source/WebCore:

Test: fast/text/text-node-remains-dirty-after-calling-surroundContents.html

• rendering/BidiRun.h:

(WebCore::BidiRun::setBox):

• rendering/RenderBlockFlow.h:
• rendering/RenderBlockLineLayout.cpp:

(WebCore::RenderBlockFlow::constructLine):
(WebCore::RenderBlockFlow::removeLineBoxIfNeeded):
(WebCore::RenderBlockFlow::computeBlockDirectionPositionsForLine):

• rendering/RenderBox.cpp:

(WebCore::RenderBox::positionLineBox): Deleted.

• rendering/RenderText.cpp:

(WebCore::RenderText::setText):
(WebCore::RenderText::positionLineBox): Deleted.

LayoutTests:

• fast/text/text-node-remains-dirty-after-calling-surroundContents-expected.html: Added.
• fast/text/text-node-remains-dirty-after-calling-surroundContents.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/text/text-node-remains-dirty-after-calling-surroundContents-expected.html (added)
• LayoutTests/fast/text/text-node-remains-dirty-after-calling-surroundContents.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/BidiRun.h (modified) (1 diff)
• Source/WebCore/rendering/RenderBlockFlow.h (modified) (1 diff)
• Source/WebCore/rendering/RenderBlockLineLayout.cpp (modified) (3 diffs)
• Source/WebCore/rendering/RenderBox.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderText.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-04-28  Zalan Bujtas  <zalan@apple.com>
+
+        Content disappears on mouse over.
+        https://bugs.webkit.org/show_bug.cgi?id=157073
+        <rdar://problem/24389168>
+
+        Reviewed by Simon Fraser.
+
+        When a redundant inlinebox is found after constructing the line, we remove it from the tree.
+        The remove operation marks the ancestor tree dirty (and this newly constructed line is supposed to be clean).
+        This patch resets this dirty flag on the boxes all the way up to the rootlinebox.
+        Previously we only cleared the rootinlinebox and we ended up with dirty inlineflowboxes.
+
+        * fast/text/text-node-remains-dirty-after-calling-surroundContents-expected.html: Added.
+        * fast/text/text-node-remains-dirty-after-calling-surroundContents.html: Added.
+
 2016-04-27  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-04-28  Zalan Bujtas  <zalan@apple.com>
+
+        Content disappears on mouse over.
+        https://bugs.webkit.org/show_bug.cgi?id=157073
+        <rdar://problem/24389168>
+
+        Reviewed by Simon Fraser.
+
+        When a redundant inlinebox is found after constructing the line, we remove it from the tree.
+        The remove operation marks the ancestor tree dirty (and this newly constructed line is supposed to be clean).
+        This patch resets this dirty flag on the boxes all the way up to the rootlinebox.
+        Previously we only cleared the rootinlinebox and we ended up with dirty inlineflowboxes.
+
+        Test: fast/text/text-node-remains-dirty-after-calling-surroundContents.html
+
+        * rendering/BidiRun.h:
+        (WebCore::BidiRun::setBox):
+        * rendering/RenderBlockFlow.h:
+        * rendering/RenderBlockLineLayout.cpp:
+        (WebCore::RenderBlockFlow::constructLine):
+        (WebCore::RenderBlockFlow::removeLineBoxIfNeeded):
+        (WebCore::RenderBlockFlow::computeBlockDirectionPositionsForLine):
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::positionLineBox): Deleted.
+        * rendering/RenderText.cpp:
+        (WebCore::RenderText::setText):
+        (WebCore::RenderText::positionLineBox): Deleted.
+
 2016-04-28  John Wilander  <wilander@apple.com>
 

trunk/Source/WebCore/rendering/BidiRun.h

@@ -42 +42 @@
     RenderObject& renderer() { return m_renderer; }
     InlineBox* box() { return m_box; }
-    void setBox(InlineBox& box) { m_box = &box; }
+    void setBox(InlineBox* box) { m_box = box; }
 
 private:

trunk/Source/WebCore/rendering/RenderBlockFlow.h

@@ -605 +605 @@
     void setSelectionState(SelectionState) final;
 
+    void removeInlineBox(BidiRun&, const RootInlineBox&) const;
+
 public:
     // FIXME-BLOCKFLOW: These can be made protected again once all callers have been moved here.

trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp

@@ -294 +294 @@
 
         InlineBox* box = createInlineBoxForRenderer(&r->renderer(), false, isOnlyRun);
-        r->setBox(*box);
+        r->setBox(box);
 
         if (!rootHasSelectedChildren && box->renderer().selectionState() != RenderObject::SelectionNone)
@@ -951 +951 @@
 }
 
+void RenderBlockFlow::removeInlineBox(BidiRun& run, const RootInlineBox& rootLineBox) const
+{
+    auto* inlineBox = run.box();
+#if !ASSERT_DISABLED
+    auto* inlineParent = inlineBox->parent();
+    while (inlineParent && inlineParent != &rootLineBox) {
+        ASSERT(!inlineParent->isDirty());
+        inlineParent = inlineParent->parent();
+    }
+    ASSERT(!rootLineBox.isDirty());
+#endif
+    auto* parent = inlineBox->parent();
+    inlineBox->removeFromParent();
+
+    auto& renderer = run.renderer();
+    if (is<RenderText>(renderer))
+        downcast<RenderText>(renderer).removeTextBox(downcast<InlineTextBox>(*inlineBox));
+    delete inlineBox;
+    run.setBox(nullptr);
+    // removeFromParent() unnecessarily dirties the ancestor subtree.
+    auto* ancestor = parent;
+    while (ancestor) {
+        ancestor->markDirty(false);
+        if (ancestor == &rootLineBox)
+            break;
+        ancestor = ancestor->parent();
+    }
+}
+
 void RenderBlockFlow::computeBlockDirectionPositionsForLine(RootInlineBox* lineBox, BidiRun* firstRun, GlyphOverflowAndFallbackFontsMap& textBoxDataMap,
                                                         VerticalPositionCache& verticalPositionCache)
@@ -957 +986 @@
 
     // Now make sure we place replaced render objects correctly.
-    for (BidiRun* run = firstRun; run; run = run->next()) {
+    for (auto* run = firstRun; run; run = run->next()) {
         ASSERT(run->box());
         if (!run->box())
             continue; // Skip runs with no line boxes.
 
-        InlineBox& box = *run->box();
-
         // Align positioned boxes with the top of the line box.  This is
         // a reasonable approximation of an appropriate y position.
-        if (run->renderer().isOutOfFlowPositioned())
-            box.setLogicalTop(logicalHeight());
+        auto& renderer = run->renderer();
+        if (renderer.isOutOfFlowPositioned())
+            run->box()->setLogicalTop(logicalHeight());
 
         // Position is used to properly position both replaced elements and
         // to update the static normal flow x/y of positioned elements.
-        if (is<RenderText>(run->renderer()))
-            downcast<RenderText>(run->renderer()).positionLineBox(downcast<InlineTextBox>(box));
-        else if (is<RenderBox>(run->renderer()))
-            downcast<RenderBox>(run->renderer()).positionLineBox(downcast<InlineElementBox>(box));
-        else if (is<RenderLineBreak>(run->renderer()))
-            downcast<RenderLineBreak>(run->renderer()).replaceInlineBoxWrapper(downcast<InlineElementBox>(box));
-    }
-    // Positioned objects and zero-length text nodes destroy their boxes in
-    // position(), which unnecessarily dirties the line.
-    lineBox->markDirty(false);
+        bool inlineBoxIsRedundant = false;
+        if (is<RenderText>(renderer)) {
+            auto& inlineTextBox = downcast<InlineTextBox>(*run->box());
+            downcast<RenderText>(renderer).positionLineBox(inlineTextBox);
+            inlineBoxIsRedundant = !inlineTextBox.len();
+        } else if (is<RenderBox>(renderer)) {
+            downcast<RenderBox>(renderer).positionLineBox(downcast<InlineElementBox>(*run->box()));
+            inlineBoxIsRedundant = renderer.isOutOfFlowPositioned();
+        } else if (is<RenderLineBreak>(renderer))
+            downcast<RenderLineBreak>(renderer).replaceInlineBoxWrapper(downcast<InlineElementBox>(*run->box()));
+        // Check if we need to keep this box on the line at all.
+        if (inlineBoxIsRedundant)
+            removeInlineBox(*run, *lineBox);
+    }
 }
 

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -2158 +2158 @@
                 setChildNeedsLayout(MarkOnlyThis); // Just mark the positioned object as needing layout, so it will update its position properly.
         }
-
-        // Nuke the box.
-        box.removeFromParent();
-        delete &box;
         return;
     }

trunk/Source/WebCore/rendering/RenderText.cpp

@@ -1269 +1269 @@
 void RenderText::positionLineBox(InlineTextBox& textBox)
 {
-    // FIXME: should not be needed!!!
-    if (!textBox.len()) {
-        // We want the box to be destroyed.
-        textBox.removeFromParent();
-        m_lineBoxes.remove(textBox);
-        delete &textBox;
+    if (!textBox.len())
         return;
-    }
-
     m_containsReversedText |= !textBox.isLeftToRightDirection();
 }