Title: [224034] trunk/Source/WebCore
- Revision
- 224034
- Author
- [email protected]
- Date
- 2017-10-26 11:51:33 -0700 (Thu, 26 Oct 2017)
Log Message
Remove unnecessary whitespace invalidation logic from RenderTreeUpdater
https://bugs.webkit.org/show_bug.cgi?id=178786
Reviewed by Zalan Bujtas.
RenderTreeUpdater::invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded is a somewhat complex
and confusing function for figuring out if some whitespace-only text node might need to have its
rendering status recomputed. However actually computing if a text renderer is needed is not expensive.
We can simply do it for all whitespace nodes after a sibling mutation.
This also removes a set that could have stale renderer pointers in it (they were never dereferenced).
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::addChildIgnoringContinuation):
Fix a display:contents bug exposed by this change. With display:contents a text renderer may have an anonymous
inline wrapper and we need to take it into account when the text renderer is the beforeChild.
Tested by imported/w3c/web-platform-tests/css/css-display-3/display-contents-state-change-001.html
* style/RenderTreeUpdater.cpp:
(WebCore::RenderTreeUpdater::updateRenderTree):
Call updateTextRenderer() for all whitespace-only text nodes after a change in siblings.
In normal update case it just figures out quickly (by calling textRendererIsNeeded)
that there are no changes and bails out.
(WebCore::RenderTreeUpdater::updateElementRenderer):
(WebCore::RenderTreeUpdater::updateTextRenderer):
(WebCore::RenderTreeUpdater::invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded): Deleted.
No longer needed. Just mark that there have been changes to siblings instead.
* style/RenderTreeUpdater.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (224033 => 224034)
--- trunk/Source/WebCore/ChangeLog 2017-10-26 18:46:24 UTC (rev 224033)
+++ trunk/Source/WebCore/ChangeLog 2017-10-26 18:51:33 UTC (rev 224034)
@@ -1,3 +1,40 @@
+2017-10-26 Antti Koivisto <[email protected]>
+
+ Remove unnecessary whitespace invalidation logic from RenderTreeUpdater
+ https://bugs.webkit.org/show_bug.cgi?id=178786
+
+ Reviewed by Zalan Bujtas.
+
+ RenderTreeUpdater::invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded is a somewhat complex
+ and confusing function for figuring out if some whitespace-only text node might need to have its
+ rendering status recomputed. However actually computing if a text renderer is needed is not expensive.
+ We can simply do it for all whitespace nodes after a sibling mutation.
+
+ This also removes a set that could have stale renderer pointers in it (they were never dereferenced).
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::addChildIgnoringContinuation):
+
+ Fix a display:contents bug exposed by this change. With display:contents a text renderer may have an anonymous
+ inline wrapper and we need to take it into account when the text renderer is the beforeChild.
+
+ Tested by imported/w3c/web-platform-tests/css/css-display-3/display-contents-state-change-001.html
+
+ * style/RenderTreeUpdater.cpp:
+ (WebCore::RenderTreeUpdater::updateRenderTree):
+
+ Call updateTextRenderer() for all whitespace-only text nodes after a change in siblings.
+ In normal update case it just figures out quickly (by calling textRendererIsNeeded)
+ that there are no changes and bails out.
+
+ (WebCore::RenderTreeUpdater::updateElementRenderer):
+ (WebCore::RenderTreeUpdater::updateTextRenderer):
+ (WebCore::RenderTreeUpdater::invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded): Deleted.
+
+ No longer needed. Just mark that there have been changes to siblings instead.
+
+ * style/RenderTreeUpdater.h:
+
2017-10-26 Myles C. Maxfield <[email protected]>
Mark font palettes as in development
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (224033 => 224034)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2017-10-26 18:46:24 UTC (rev 224033)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2017-10-26 18:51:33 UTC (rev 224034)
@@ -554,6 +554,11 @@
ASSERT(beforeChildContainer);
if (beforeChildContainer->isAnonymous()) {
+ if (beforeChildContainer->isInline()) {
+ ASSERT(RenderText::findByDisplayContentsInlineWrapperCandidate(*beforeChildContainer) == beforeChild);
+ addChild(WTFMove(newChild), beforeChildContainer);
+ return;
+ }
// If the requested beforeChild is not one of our children, then this is because
// there is an anonymous container within this object that contains the beforeChild.
RenderElement* beforeChildAnonymousContainer = beforeChildContainer;
Modified: trunk/Source/WebCore/style/RenderTreeUpdater.cpp (224033 => 224034)
--- trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-10-26 18:46:24 UTC (rev 224033)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-10-26 18:51:33 UTC (rev 224034)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -174,7 +174,9 @@
if (is<Text>(node)) {
auto& text = downcast<Text>(node);
auto* textUpdate = m_styleUpdate->textUpdate(text);
- if ((parent().updates && parent().updates->update.change == Style::Detach) || textUpdate || m_invalidatedWhitespaceOnlyTextSiblings.contains(&text))
+ bool didCreateParent = parent().updates && parent().updates->update.change == Style::Detach;
+ bool mayNeedUpdateWhitespaceOnlyRenderer = parent().didCreateOrDestroyChildRenderer && text.containsOnlyWhitespace();
+ if (didCreateParent || textUpdate || mayNeedUpdateWhitespaceOnlyRenderer)
updateTextRenderer(text, textUpdate);
storePreviousRenderer(text);
@@ -211,8 +213,6 @@
}
popParentsToDepth(0);
-
- m_invalidatedWhitespaceOnlyTextSiblings.clear();
}
RenderTreePosition& RenderTreeUpdater::renderTreePosition()
@@ -313,6 +313,8 @@
// display:none cancels animations.
auto teardownType = update.style->display() == NONE ? TeardownType::RendererUpdateCancelingAnimations : TeardownType::RendererUpdate;
tearDownRenderers(element, teardownType);
+
+ parent().didCreateOrDestroyChildRenderer = true;
}
bool hasDisplayContents = update.style->display() == CONTENTS;
@@ -326,7 +328,8 @@
if (element.hasCustomStyleResolveCallbacks())
element.willAttachRenderers();
createRenderer(element, RenderStyle::clone(*update.style));
- invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded(element);
+
+ parent().didCreateOrDestroyChildRenderer = true;
return;
}
@@ -480,41 +483,15 @@
return;
}
tearDownRenderer(text);
- invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded(text);
+ parent().didCreateOrDestroyChildRenderer = true;
return;
}
if (!needsRenderer)
return;
createTextRenderer(text, renderTreePosition(), textUpdate);
- invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded(text);
+ parent().didCreateOrDestroyChildRenderer = true;
}
-void RenderTreeUpdater::invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded(Node& current)
-{
- // FIXME: This needs to traverse in composed tree order.
-
- // This function finds sibling text renderers where the results of textRendererIsNeeded may have changed as a result of
- // the current node gaining or losing the renderer. This can only affect white space text nodes.
- for (Node* sibling = current.nextSibling(); sibling; sibling = sibling->nextSibling()) {
- if (is<Element>(*sibling)) {
- if (m_styleUpdate->elementUpdates(downcast<Element>(*sibling)))
- return;
- // Text renderers beyond rendered elements can't be affected.
- if (sibling->renderer())
- return;
- continue;
- }
- if (!is<Text>(*sibling))
- continue;
- Text& textSibling = downcast<Text>(*sibling);
- if (m_styleUpdate->textUpdate(textSibling))
- return;
- if (!textSibling.containsOnlyWhitespace())
- continue;
- m_invalidatedWhitespaceOnlyTextSiblings.add(&textSibling);
- }
-}
-
void RenderTreeUpdater::storePreviousRenderer(Node& node)
{
auto* renderer = node.renderer();
Modified: trunk/Source/WebCore/style/RenderTreeUpdater.h (224033 => 224034)
--- trunk/Source/WebCore/style/RenderTreeUpdater.h 2017-10-26 18:46:24 UTC (rev 224033)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.h 2017-10-26 18:51:33 UTC (rev 224034)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -61,7 +61,6 @@
void updateTextRenderer(Text&, const Style::TextUpdate*);
void updateElementRenderer(Element&, const Style::ElementUpdate&);
void createRenderer(Element&, RenderStyle&&);
- void invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded(Node&);
void updateBeforeDescendants(Element&, const Style::ElementUpdates*);
void updateAfterDescendants(Element&, const Style::ElementUpdates*);
bool textRendererIsNeeded(const Text& textNode);
@@ -71,6 +70,8 @@
Element* element { nullptr };
const Style::ElementUpdates* updates { nullptr };
std::optional<RenderTreePosition> renderTreePosition;
+
+ bool didCreateOrDestroyChildRenderer { false };
RenderObject* previousChildRenderer { nullptr };
Parent(ContainerNode& root);
@@ -95,8 +96,6 @@
Vector<Parent> m_parentStack;
- HashSet<Text*> m_invalidatedWhitespaceOnlyTextSiblings;
-
std::unique_ptr<GeneratedContent> m_generatedContent;
};
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
