Diff
Modified: trunk/LayoutTests/ChangeLog (276286 => 276287)
--- trunk/LayoutTests/ChangeLog 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/LayoutTests/ChangeLog 2021-04-20 06:33:49 UTC (rev 276287)
@@ -1,3 +1,19 @@
+2021-04-19 Antti Koivisto <[email protected]>
+
+ Render tree updates for Text node content mutations should happen during rendering update
+ https://bugs.webkit.org/show_bug.cgi?id=222406
+ <rdar://problem/74822830>
+
+ Reviewed by Simon Fraser.
+
+ These are progressions.
+
+ * fast/text/splitText-dirty-lines-expected.txt:
+ * fast/text/text-combine-surroundContents-crash-expected.txt:
+ * imported/blink/fast/css/first-letter-range-insert-expected.txt:
+
+ Here we were actually drawing text that didn't exist in DOM anymore.
+
2021-04-19 Diego Pino Garcia <[email protected]>
[GLIB] Unreviewed test gardening. Gardened flaky failures after r224791.
Modified: trunk/LayoutTests/fast/text/splitText-dirty-lines-expected.txt (276286 => 276287)
--- trunk/LayoutTests/fast/text/splitText-dirty-lines-expected.txt 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/LayoutTests/fast/text/splitText-dirty-lines-expected.txt 2021-04-20 06:33:49 UTC (rev 276287)
@@ -15,7 +15,6 @@
text run at (80,120) width 40: "E"
layer at (8,8) size 200x160
RenderBlock (positioned) {DIV} at (0,0) size 200x160 [color=#008000]
- RenderText {#text} at (0,0) size 0x0
RenderText {#text} at (0,0) size 160x160
text run at (0,0) width 160: "AAAA"
text run at (0,40) width 40: "B"
Modified: trunk/LayoutTests/fast/text/text-combine-surroundContents-crash-expected.txt (276286 => 276287)
--- trunk/LayoutTests/fast/text/text-combine-surroundContents-crash-expected.txt 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/LayoutTests/fast/text/text-combine-surroundContents-crash-expected.txt 2021-04-20 06:33:49 UTC (rev 276287)
@@ -1,16 +1,15 @@
layer at (0,0) size 800x600
RenderView at (0,0) size 800x600
-layer at (0,0) size 800x76
- RenderBlock {HTML} at (0,0) size 800x76
- RenderBody {BODY} at (8,8) size 784x60
- RenderBlock {DIV} at (0,0) size 20x40
- RenderCombineText {#text} at (0,0) size 0x0
- RenderInline {SPAN} at (0,0) size 20x0
- RenderCombineText {#text} at (0,20) size 20x20
- text run at (0,20) width 20: "\x{FFFC}"
- RenderBlock (anonymous) at (0,40) size 784x0
+layer at (0,0) size 800x56
+ RenderBlock {HTML} at (0,0) size 800x56
+ RenderBody {BODY} at (8,8) size 784x40
+ RenderBlock {DIV} at (0,0) size 20x20
+ RenderInline {SPAN} at (0,0) size 0x0
+ RenderCombineText {#text} at (0,0) size 20x20
+ text run at (0,0) width 20: "\x{FFFC}"
+ RenderBlock (anonymous) at (0,20) size 784x0
RenderText {#text} at (0,0) size 0x0
RenderText {#text} at (0,0) size 0x0
- RenderBlock {DIV} at (0,40) size 784x20
+ RenderBlock {DIV} at (0,20) size 784x20
RenderText {#text} at (0,0) size 400x20
text run at (0,0) width 400: "PASS, did not crash."
Modified: trunk/LayoutTests/imported/blink/fast/css/first-letter-range-insert-expected.txt (276286 => 276287)
--- trunk/LayoutTests/imported/blink/fast/css/first-letter-range-insert-expected.txt 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/LayoutTests/imported/blink/fast/css/first-letter-range-insert-expected.txt 2021-04-20 06:33:49 UTC (rev 276287)
@@ -1,2 +1 @@
-B
Modified: trunk/Source/WebCore/ChangeLog (276286 => 276287)
--- trunk/Source/WebCore/ChangeLog 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/ChangeLog 2021-04-20 06:33:49 UTC (rev 276287)
@@ -1,3 +1,79 @@
+2021-04-19 Antti Koivisto <[email protected]>
+
+ Render tree updates for Text node content mutations should happen during rendering update
+ https://bugs.webkit.org/show_bug.cgi?id=222406
+ <rdar://problem/74822830>
+
+ Reviewed by Simon Fraser.
+
+ Calls to Text.insertData and similar should not mutate render tree synchronously.
+ Instead render tree should be updated during the next rendering update along with
+ any style changes.
+
+ These updates already go via RenderTreeUpdater. We just need to save the information
+ about which nodes need updating so the next rendering update can pick them up.
+
+ This seems to help with some performance benchmarks.
+
+ * dom/CharacterData.cpp:
+ (WebCore::CharacterData::parserAppendData):
+ (WebCore::CharacterData::setDataAndUpdate):
+
+ Move in-tree check to the updateRendererAfterContentChange and make it use isConnected().
+
+ * dom/Document.cpp:
+ (WebCore::Document::removedLastRef):
+ (WebCore::Document::resolveStyle):
+
+ Include the text update when updating the render tree.
+
+ (WebCore::Document::updateTextRenderer):
+
+ Create a text update that will get flushed during the next rendering update.
+
+ (WebCore::Document::needsStyleRecalc const):
+
+ We need to recalc if there are pending text updates.
+
+ * dom/Document.h:
+ * dom/Text.cpp:
+ (WebCore::Text::splitText):
+
+ Use updateRendererAfterContentChange instead of poking render tree directly.
+
+ (WebCore::Text::updateRendererAfterContentChange):
+ * rendering/updating/RenderTreeUpdater.cpp:
+ * style/StyleTreeResolver.cpp:
+ (WebCore::Style::TreeResolver::TreeResolver):
+ (WebCore::Style::TreeResolver::resolve):
+ * style/StyleTreeResolver.h:
+ (WebCore::Style::TreeResolver::TreeResolver):
+ * style/StyleUpdate.cpp:
+ (WebCore::Style::Update::addText):
+
+ Merge text updates.
+
+ (WebCore::Style::Update::addPossibleRoot):
+ * style/StyleUpdate.h:
+ (WebCore::Style::Update::roots const):
+ (WebCore::Style::Update:: const): Deleted.
+
+ Refcount the nodes since this now has longer lifetime.
+
+ * editing/CompositeEditCommand.cpp:
+ (WebCore::CompositeEditCommand::deleteInsignificantText):
+
+ Ensure we leave deleteInsignificantText with updated render tree. Clients expect that.
+ Do layout (instead of just style update) for consistency, deleteInsignificantText does one anyway in beginning.
+
+ * editing/markup.cpp:
+ (WebCore::replaceChildrenWithFragment):
+
+ Pending text update may ref the node so this refcount assert is not correct.
+
+ * style/StyleUpdate.cpp:
+ (WebCore::Style::Update::addText):
+
2021-04-19 Said Abou-Hallawa <[email protected]>
cachedCGColor() and nsColor() are not thread-safe
Modified: trunk/Source/WebCore/dom/CharacterData.cpp (276286 => 276287)
--- trunk/Source/WebCore/dom/CharacterData.cpp 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/dom/CharacterData.cpp 2021-04-20 06:33:49 UTC (rev 276287)
@@ -102,7 +102,7 @@
m_data.append(string.characters16() + offset, characterLengthLimit);
ASSERT(!renderer() || is<Text>(*this));
- if (is<Text>(*this) && parentNode())
+ if (is<Text>(*this))
downcast<Text>(*this).updateRendererAfterContentChange(oldLength, 0);
notifyParentAfterChange(ContainerNode::ChildChange::Source::Parser);
@@ -182,7 +182,7 @@
document().textInserted(*this, offsetOfReplacedData, newLength);
ASSERT(!renderer() || is<Text>(*this));
- if (is<Text>(*this) && parentNode())
+ if (is<Text>(*this))
downcast<Text>(*this).updateRendererAfterContentChange(offsetOfReplacedData, oldLength);
if (is<ProcessingInstruction>(*this))
Modified: trunk/Source/WebCore/dom/Document.cpp (276286 => 276287)
--- trunk/Source/WebCore/dom/Document.cpp 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/dom/Document.cpp 2021-04-20 06:33:49 UTC (rev 276287)
@@ -809,6 +809,7 @@
m_fullscreenManager->clear();
#endif
m_associatedFormControls.clear();
+ m_pendingRenderTreeTextUpdate = { };
m_fontLoader->stopLoadingAndClearFonts();
@@ -2054,7 +2055,7 @@
documentElement->invalidateStyleForSubtree();
}
- Style::TreeResolver resolver(*this);
+ Style::TreeResolver resolver(*this, WTFMove(m_pendingRenderTreeTextUpdate));
auto styleUpdate = resolver.resolve();
m_lastStyleUpdateSizeForTesting = styleUpdate ? styleUpdate->size() : 0;
@@ -2102,10 +2103,12 @@
void Document::updateTextRenderer(Text& text, unsigned offsetOfReplacedText, unsigned lengthOfReplacedText)
{
- auto textUpdate = makeUnique<Style::Update>(*this);
- textUpdate->addText(text, { offsetOfReplacedText, lengthOfReplacedText, WTF::nullopt });
+ if (!m_pendingRenderTreeTextUpdate)
+ m_pendingRenderTreeTextUpdate = makeUnique<Style::Update>(*this);
- updateRenderTree(WTFMove(textUpdate));
+ m_pendingRenderTreeTextUpdate->addText(text, { offsetOfReplacedText, lengthOfReplacedText, WTF::nullopt });
+
+ scheduleRenderingUpdate({ });
}
bool Document::needsStyleRecalc() const
@@ -2119,6 +2122,9 @@
if (childNeedsStyleRecalc())
return true;
+ if (m_pendingRenderTreeTextUpdate)
+ return true;
+
if (styleScope().hasPendingUpdate())
return true;
Modified: trunk/Source/WebCore/dom/Document.h (276286 => 276287)
--- trunk/Source/WebCore/dom/Document.h 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/dom/Document.h 2021-04-20 06:33:49 UTC (rev 276287)
@@ -1806,6 +1806,8 @@
Timer m_styleRecalcTimer;
+ std::unique_ptr<Style::Update> m_pendingRenderTreeTextUpdate;
+
Element* m_cssTarget { nullptr };
std::unique_ptr<LazyLoadImageObserver> m_lazyLoadImageObserver;
Modified: trunk/Source/WebCore/dom/Text.cpp (276286 => 276287)
--- trunk/Source/WebCore/dom/Text.cpp 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/dom/Text.cpp 2021-04-20 06:33:49 UTC (rev 276287)
@@ -76,8 +76,7 @@
document().textNodeSplit(*this);
- if (renderer())
- renderer()->setTextWithOffset(data(), 0, oldData.length());
+ updateRendererAfterContentChange(0, oldData.length());
return newText;
}
@@ -215,7 +214,9 @@
void Text::updateRendererAfterContentChange(unsigned offsetOfReplacedData, unsigned lengthOfReplacedData)
{
- ASSERT(parentNode());
+ if (!isConnected())
+ return;
+
if (styleValidity() >= Style::Validity::SubtreeAndRenderersInvalid)
return;
Modified: trunk/Source/WebCore/editing/CompositeEditCommand.cpp (276286 => 276287)
--- trunk/Source/WebCore/editing/CompositeEditCommand.cpp 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/editing/CompositeEditCommand.cpp 2021-04-20 06:33:49 UTC (rev 276287)
@@ -1093,6 +1093,10 @@
int endOffset = textNode.ptr() == end.deprecatedNode() ? end.deprecatedEditingOffset() : static_cast<int>(textNode->length());
deleteInsignificantText(textNode, startOffset, endOffset);
}
+ if (!nodes.isEmpty()) {
+ // Callers expect render tree to be in sync.
+ document().updateLayoutIgnorePendingStylesheets();
+ }
}
void CompositeEditCommand::deleteInsignificantTextDownstream(const Position& position)
Modified: trunk/Source/WebCore/editing/markup.cpp (276286 => 276287)
--- trunk/Source/WebCore/editing/markup.cpp 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/editing/markup.cpp 2021-04-20 06:33:49 UTC (rev 276287)
@@ -1387,7 +1387,6 @@
auto* containerChild = containerNode->firstChild();
if (containerChild && !containerChild->nextSibling()) {
if (is<Text>(*containerChild) && hasOneTextChild(fragment) && canUseSetDataOptimization(downcast<Text>(*containerChild), mutation)) {
- ASSERT(!fragment->firstChild()->refCount());
downcast<Text>(*containerChild).setData(downcast<Text>(*fragment->firstChild()).data());
return { };
}
Modified: trunk/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp (276286 => 276287)
--- trunk/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/rendering/updating/RenderTreeUpdater.cpp 2021-04-20 06:33:49 UTC (rev 276287)
@@ -102,7 +102,7 @@
static ListHashSet<ContainerNode*> findRenderingRoots(const Style::Update& update)
{
ListHashSet<ContainerNode*> renderingRoots;
- for (auto* root : update.roots()) {
+ for (auto& root : update.roots()) {
auto* renderingRoot = findRenderingRoot(*root);
if (!renderingRoot)
continue;
Modified: trunk/Source/WebCore/style/StyleTreeResolver.cpp (276286 => 276287)
--- trunk/Source/WebCore/style/StyleTreeResolver.cpp 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/style/StyleTreeResolver.cpp 2021-04-20 06:33:49 UTC (rev 276287)
@@ -63,8 +63,9 @@
DEFINE_ALLOCATOR_WITH_HEAP_IDENTIFIER(TreeResolverScope);
-TreeResolver::TreeResolver(Document& document)
+TreeResolver::TreeResolver(Document& document, std::unique_ptr<Update> update)
: m_document(document)
+ , m_update(WTFMove(update))
{
}
@@ -600,11 +601,12 @@
return nullptr;
}
if (!documentElement->childNeedsStyleRecalc() && !documentElement->needsStyleRecalc())
- return nullptr;
+ return WTFMove(m_update);
m_didSeePendingStylesheet = m_document.styleScope().hasPendingSheetsBeforeBody();
- m_update = makeUnique<Update>(m_document);
+ if (!m_update)
+ m_update = makeUnique<Update>(m_document);
m_scopeStack.append(adoptRef(*new Scope(m_document)));
m_parentStack.append(Parent(m_document));
Modified: trunk/Source/WebCore/style/StyleTreeResolver.h (276286 => 276287)
--- trunk/Source/WebCore/style/StyleTreeResolver.h 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/style/StyleTreeResolver.h 2021-04-20 06:33:49 UTC (rev 276287)
@@ -49,7 +49,7 @@
DECLARE_ALLOCATOR_WITH_HEAP_IDENTIFIER(TreeResolverScope);
class TreeResolver {
public:
- TreeResolver(Document&);
+ TreeResolver(Document&, std::unique_ptr<Update> = { });
~TreeResolver();
std::unique_ptr<Update> resolve();
Modified: trunk/Source/WebCore/style/StyleUpdate.cpp (276286 => 276287)
--- trunk/Source/WebCore/style/StyleUpdate.cpp 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/style/StyleUpdate.cpp 2021-04-20 06:33:49 UTC (rev 276287)
@@ -98,12 +98,23 @@
void Update::addText(Text& text, Element* parent, TextUpdate&& textUpdate)
{
- ASSERT(!m_texts.contains(&text));
ASSERT(composedTreeAncestors(text).first() == parent);
addPossibleRoot(parent);
-
- m_texts.add(&text, WTFMove(textUpdate));
+
+ auto result = m_texts.add(&text, WTFMove(textUpdate));
+
+ if (!result.isNewEntry) {
+ auto& entry = result.iterator->value;
+ auto startOffset = std::min(entry.offset, textUpdate.offset);
+ auto endOffset = std::max(entry.offset + entry.length, textUpdate.offset + textUpdate.length);
+ entry.offset = startOffset;
+ entry.length = endOffset - startOffset;
+
+ ASSERT(!entry.inheritedDisplayContentsStyle || !textUpdate.inheritedDisplayContentsStyle);
+ if (!entry.inheritedDisplayContentsStyle)
+ entry.inheritedDisplayContentsStyle = WTFMove(textUpdate.inheritedDisplayContentsStyle);
+ }
}
void Update::addText(Text& text, TextUpdate&& textUpdate)
@@ -114,7 +125,7 @@
void Update::addPossibleRoot(Element* element)
{
if (!element) {
- m_roots.add(&m_document);
+ m_roots.add(m_document.ptr());
return;
}
if (m_elements.contains(element))
Modified: trunk/Source/WebCore/style/StyleUpdate.h (276286 => 276287)
--- trunk/Source/WebCore/style/StyleUpdate.h 2021-04-20 05:19:31 UTC (rev 276286)
+++ trunk/Source/WebCore/style/StyleUpdate.h 2021-04-20 06:33:49 UTC (rev 276287)
@@ -69,7 +69,7 @@
public:
Update(Document&);
- const ListHashSet<ContainerNode*>& roots() const { return m_roots; }
+ const ListHashSet<RefPtr<ContainerNode>>& roots() const { return m_roots; }
const ElementUpdates* elementUpdates(const Element&) const;
ElementUpdates* elementUpdates(const Element&);
@@ -90,10 +90,10 @@
private:
void addPossibleRoot(Element*);
- Document& m_document;
- ListHashSet<ContainerNode*> m_roots;
- HashMap<const Element*, ElementUpdates> m_elements;
- HashMap<const Text*, TextUpdate> m_texts;
+ Ref<Document> m_document;
+ ListHashSet<RefPtr<ContainerNode>> m_roots;
+ HashMap<RefPtr<const Element>, ElementUpdates> m_elements;
+ HashMap<RefPtr<const Text>, TextUpdate> m_texts;
};
}
