- Revision
- 223848
- Author
- an...@apple.com
- Date
- 2017-10-23 11:59:06 -0700 (Mon, 23 Oct 2017)
Log Message
Remember previous child renderer during render tree update
https://bugs.webkit.org/show_bug.cgi?id=178659
Reviewed by Zalan Bujtas.
Source/WebCore:
We shouldn't need to recompute the previous renderer, we know it already.
* style/RenderTreePosition.cpp:
(WebCore::RenderTreePosition::previousSiblingRenderer const): Deleted.
No longer needed. This was also subtly wrong as doesn't take display:contents into account.
* style/RenderTreePosition.h:
* style/RenderTreeUpdater.cpp:
(WebCore::RenderTreeUpdater::updateRenderTree):
(WebCore::RenderTreeUpdater::textRendererIsNeeded):
Use the saved previous renderer.
(WebCore::RenderTreeUpdater::updateTextRenderer):
(WebCore::RenderTreeUpdater::storePreviousRenderer):
Save the previous renderere as we walk the tree.
(WebCore::textRendererIsNeeded): Deleted.
* style/RenderTreeUpdater.h:
LayoutTests:
* fast/block/float/float-not-removed-from-pre-block-expected.txt:
* platform/mac/fast/css-generated-content/details-summary-before-after-expected.txt:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (223847 => 223848)
--- trunk/LayoutTests/ChangeLog 2017-10-23 18:57:50 UTC (rev 223847)
+++ trunk/LayoutTests/ChangeLog 2017-10-23 18:59:06 UTC (rev 223848)
@@ -1,3 +1,13 @@
+2017-10-23 Antti Koivisto <an...@apple.com>
+
+ Remember previous child renderer during render tree update
+ https://bugs.webkit.org/show_bug.cgi?id=178659
+
+ Reviewed by Zalan Bujtas.
+
+ * fast/block/float/float-not-removed-from-pre-block-expected.txt:
+ * platform/mac/fast/css-generated-content/details-summary-before-after-expected.txt:
+
2017-10-23 Daniel Bates <daba...@apple.com>
Add tests to ensure spelling error dots are drawn in the correct place in bottom-to-top
Modified: trunk/LayoutTests/fast/block/float/float-not-removed-from-pre-block-expected.txt (223847 => 223848)
--- trunk/LayoutTests/fast/block/float/float-not-removed-from-pre-block-expected.txt 2017-10-23 18:57:50 UTC (rev 223847)
+++ trunk/LayoutTests/fast/block/float/float-not-removed-from-pre-block-expected.txt 2017-10-23 18:59:06 UTC (rev 223848)
@@ -1,3 +1,3 @@
Bug 101970: Heap-use-after-free in WebCore::RenderLayerModelObject::hasSelfPaintingLayer
Test passes if it does not crash.
-
+
Modified: trunk/LayoutTests/platform/mac/fast/css-generated-content/details-summary-before-after-expected.txt (223847 => 223848)
--- trunk/LayoutTests/platform/mac/fast/css-generated-content/details-summary-before-after-expected.txt 2017-10-23 18:57:50 UTC (rev 223847)
+++ trunk/LayoutTests/platform/mac/fast/css-generated-content/details-summary-before-after-expected.txt 2017-10-23 18:59:06 UTC (rev 223848)
@@ -24,7 +24,6 @@
RenderText at (0,0) size 30x18
text run at (0,0) width 30: "after"
RenderBlock (anonymous) at (1,77) size 782x18
- RenderText {#text} at (0,0) size 0x0
RenderText {#text} at (0,0) size 50x18
text run at (0,0) width 50: "Details "
RenderInline (generated) at (0,0) size 30x18
Modified: trunk/Source/WebCore/ChangeLog (223847 => 223848)
--- trunk/Source/WebCore/ChangeLog 2017-10-23 18:57:50 UTC (rev 223847)
+++ trunk/Source/WebCore/ChangeLog 2017-10-23 18:59:06 UTC (rev 223848)
@@ -1,3 +1,32 @@
+2017-10-23 Antti Koivisto <an...@apple.com>
+
+ Remember previous child renderer during render tree update
+ https://bugs.webkit.org/show_bug.cgi?id=178659
+
+ Reviewed by Zalan Bujtas.
+
+ We shouldn't need to recompute the previous renderer, we know it already.
+
+ * style/RenderTreePosition.cpp:
+ (WebCore::RenderTreePosition::previousSiblingRenderer const): Deleted.
+
+ No longer needed. This was also subtly wrong as doesn't take display:contents into account.
+
+ * style/RenderTreePosition.h:
+ * style/RenderTreeUpdater.cpp:
+ (WebCore::RenderTreeUpdater::updateRenderTree):
+ (WebCore::RenderTreeUpdater::textRendererIsNeeded):
+
+ Use the saved previous renderer.
+
+ (WebCore::RenderTreeUpdater::updateTextRenderer):
+ (WebCore::RenderTreeUpdater::storePreviousRenderer):
+
+ Save the previous renderere as we walk the tree.
+
+ (WebCore::textRendererIsNeeded): Deleted.
+ * style/RenderTreeUpdater.h:
+
2017-10-23 Keith Miller <keith_mil...@apple.com>
Add Shared Modules files to the unified source build.
Modified: trunk/Source/WebCore/style/RenderTreePosition.cpp (223847 => 223848)
--- trunk/Source/WebCore/style/RenderTreePosition.cpp 2017-10-23 18:57:50 UTC (rev 223847)
+++ trunk/Source/WebCore/style/RenderTreePosition.cpp 2017-10-23 18:59:06 UTC (rev 223848)
@@ -56,23 +56,6 @@
m_hasValidNextSibling = false;
}
-RenderObject* RenderTreePosition::previousSiblingRenderer(const Text& textNode) const
-{
- if (textNode.renderer())
- return textNode.renderer()->previousSibling();
-
- auto* parentElement = m_parent.element();
-
- auto composedChildren = composedTreeChildren(*parentElement);
- for (auto it = composedChildren.at(textNode), end = composedChildren.end(); it != end; --it) {
- if (auto* renderer = it->renderer())
- return renderer;
- }
- if (auto* before = parentElement->beforePseudoElement())
- return before->renderer();
- return nullptr;
-}
-
RenderObject* RenderTreePosition::nextSiblingRenderer(const Node& node) const
{
ASSERT(!node.renderer());
Modified: trunk/Source/WebCore/style/RenderTreePosition.h (223847 => 223848)
--- trunk/Source/WebCore/style/RenderTreePosition.h 2017-10-23 18:57:50 UTC (rev 223847)
+++ trunk/Source/WebCore/style/RenderTreePosition.h 2017-10-23 18:59:06 UTC (rev 223848)
@@ -49,7 +49,6 @@
void invalidateNextSibling() { m_hasValidNextSibling = false; }
void invalidateNextSibling(const RenderObject&);
- RenderObject* previousSiblingRenderer(const Text&) const;
RenderObject* nextSiblingRenderer(const Node&) const;
private:
Modified: trunk/Source/WebCore/style/RenderTreeUpdater.cpp (223847 => 223848)
--- trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-10-23 18:57:50 UTC (rev 223847)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-10-23 18:59:06 UTC (rev 223848)
@@ -177,6 +177,7 @@
if ((parent().updates && parent().updates->update.change == Style::Detach) || textUpdate || m_invalidatedWhitespaceOnlyTextSiblings.contains(&text))
updateTextRenderer(text, textUpdate);
+ storePreviousRenderer(text);
it.traverseNextSkippingChildren();
continue;
}
@@ -188,6 +189,7 @@
// We hop through display: contents elements in findRenderingRoot, so
// there may be other updates down the tree.
if (!elementUpdates && !element.hasDisplayContents()) {
+ storePreviousRenderer(element);
it.traverseNextSkippingChildren();
continue;
}
@@ -195,6 +197,8 @@
if (elementUpdates)
updateElementRenderer(element, elementUpdates->update);
+ storePreviousRenderer(element);
+
bool mayHaveRenderedDescendants = element.renderer() || (element.hasDisplayContents() && shouldCreateRenderer(element, renderTreePosition().parent()));
if (!mayHaveRenderedDescendants) {
it.traverseNextSkippingChildren();
@@ -384,9 +388,9 @@
cache->updateCacheAfterNodeIsAttached(&element);
}
-static bool textRendererIsNeeded(const Text& textNode, const RenderTreePosition& renderTreePosition)
+bool RenderTreeUpdater::textRendererIsNeeded(const Text& textNode)
{
- const RenderElement& parentRenderer = renderTreePosition.parent();
+ const RenderElement& parentRenderer = renderTreePosition().parent();
if (!parentRenderer.canHaveChildren())
return false;
if (parentRenderer.element() && !parentRenderer.element()->childShouldCreateRenderer(textNode))
@@ -403,7 +407,7 @@
if (parentRenderer.style().preserveNewline()) // pre/pre-wrap/pre-line always make renderers.
return true;
- RenderObject* previousRenderer = renderTreePosition.previousSiblingRenderer(textNode);
+ auto* previousRenderer = parent().previousChildRenderer;
if (previousRenderer && previousRenderer->isBR()) // <span><br/> <br/></span>
return false;
@@ -418,7 +422,7 @@
RenderObject* first = parentRenderer.firstChild();
while (first && first->isFloatingOrOutOfFlowPositioned())
first = first->nextSibling();
- RenderObject* nextRenderer = textNode.renderer() ? textNode.renderer() : renderTreePosition.nextSiblingRenderer(textNode);
+ RenderObject* nextRenderer = textNode.renderer() ? textNode.renderer() : renderTreePosition().nextSiblingRenderer(textNode);
if (!first || nextRenderer == first) {
// Whitespace at the start of a block just goes away. Don't even make a render object for this text.
return false;
@@ -459,7 +463,7 @@
void RenderTreeUpdater::updateTextRenderer(Text& text, const Style::TextUpdate* textUpdate)
{
auto* existingRenderer = text.renderer();
- bool needsRenderer = textRendererIsNeeded(text, renderTreePosition());
+ bool needsRenderer = textRendererIsNeeded(text);
if (existingRenderer && textUpdate && textUpdate->inheritedDisplayContentsStyle) {
if (existingRenderer->inlineWrapperForDisplayContents() || *textUpdate->inheritedDisplayContentsStyle) {
@@ -511,6 +515,15 @@
}
}
+void RenderTreeUpdater::storePreviousRenderer(Node& node)
+{
+ auto* renderer = node.renderer();
+ if (!renderer)
+ return;
+ ASSERT(parent().previousChildRenderer != renderer);
+ parent().previousChildRenderer = renderer;
+}
+
void RenderTreeUpdater::tearDownRenderers(Element& root)
{
tearDownRenderers(root, TeardownType::Full);
Modified: trunk/Source/WebCore/style/RenderTreeUpdater.h (223847 => 223848)
--- trunk/Source/WebCore/style/RenderTreeUpdater.h 2017-10-23 18:57:50 UTC (rev 223847)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.h 2017-10-23 18:59:06 UTC (rev 223848)
@@ -64,11 +64,14 @@
void invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded(Node&);
void updateBeforeDescendants(Element&, const Style::ElementUpdates*);
void updateAfterDescendants(Element&, const Style::ElementUpdates*);
+ bool textRendererIsNeeded(const Text& textNode);
+ void storePreviousRenderer(Node&);
struct Parent {
Element* element { nullptr };
const Style::ElementUpdates* updates { nullptr };
std::optional<RenderTreePosition> renderTreePosition;
+ RenderObject* previousChildRenderer { nullptr };
Parent(ContainerNode& root);
Parent(Element&, const Style::ElementUpdates*);
