Diff
Modified: trunk/Source/WebCore/ChangeLog (220446 => 220447)
--- trunk/Source/WebCore/ChangeLog 2017-08-09 07:16:56 UTC (rev 220446)
+++ trunk/Source/WebCore/ChangeLog 2017-08-09 07:36:45 UTC (rev 220447)
@@ -1,3 +1,70 @@
+2017-08-09 Antti Koivisto <[email protected]>
+
+ RenderQuote should not mutate render tree
+ https://bugs.webkit.org/show_bug.cgi?id=175328
+
+ Reviewed by Zalan Bujtas.
+
+ RenderQuote text renderers are currently created and deleted in a quirky fashion using a linked list.
+ This patch moves to a simpler model that guarantees the mutations are always done in controlled fashion
+ during render tree update.
+
+ * dom/Document.cpp:
+ (WebCore::Document::updateTextRenderer):
+
+ Move text renderer updating to Document so we can set the inRenderTreeUpdate bit for it too.
+
+ * dom/Document.h:
+ * dom/Text.cpp:
+ (WebCore::Text::updateRendererAfterContentChange):
+ * rendering/RenderDescendantIterator.h:
+ (WebCore::RenderDescendantIteratorAdapter<T>::at):
+ (WebCore::RenderDescendantConstIteratorAdapter<T>::at const):
+
+ Add at() function for starting iteration from a specified renderer.
+
+ * rendering/RenderQuote.cpp:
+ (WebCore::RenderQuote::insertedIntoTree):
+ (WebCore::RenderQuote::willBeRemovedFromTree):
+
+ Register and unregister quotes to RenderView.
+ Don't do any mutations.
+
+ (WebCore::RenderQuote::styleDidChange):
+
+ Invalidate the text renderer but don't mutate it.
+
+ (WebCore::RenderQuote::updateTextRenderer):
+ (WebCore::RenderQuote::computeText const):
+ (WebCore::RenderQuote::updateRenderers):
+
+ Compute depth of all render quotes and update the text renderer as needed.
+
+ (WebCore::RenderQuote::willBeDestroyed): Deleted.
+ (WebCore::RenderQuote::attachQuote): Deleted.
+ (WebCore::RenderQuote::detachQuote): Deleted.
+ (WebCore::RenderQuote::updateDepth): Deleted.
+
+ Get rid of the linked list.
+
+ * rendering/RenderQuote.h:
+ * rendering/RenderView.cpp:
+ (WebCore::RenderView::registerQuote):
+ (WebCore::RenderView::unregisterQuote):
+
+ Maintain a render tree order ListHashSet of RenderQuotes.
+
+ (WebCore::RenderView::updateSpecialRenderers):
+
+ Add a function for making additional render tree mutations at the end of a render tree update.
+ Currently this just invokes RenderQuote::updateRenderers.
+
+ * rendering/RenderView.h:
+ * style/RenderTreeUpdater.cpp:
+ (WebCore::RenderTreeUpdater::commit):
+
+ Call RenderView::updateSpecialRenderers after committing all other changes.
+
2017-08-09 Zan Dobersek <[email protected]>
[Soup] Incorrect conversion in msToSoupDate()
Modified: trunk/Source/WebCore/dom/Document.cpp (220446 => 220447)
--- trunk/Source/WebCore/dom/Document.cpp 2017-08-09 07:16:56 UTC (rev 220446)
+++ trunk/Source/WebCore/dom/Document.cpp 2017-08-09 07:36:45 UTC (rev 220447)
@@ -1868,6 +1868,18 @@
// FIXME: Ideally we would ASSERT(!needsStyleRecalc()) here but we have some cases where it is not true.
}
+void Document::updateTextRenderer(Text& text)
+{
+ ASSERT(!m_inRenderTreeUpdate);
+ SetForScope<bool> inRenderTreeUpdate(m_inRenderTreeUpdate, true);
+
+ auto textUpdate = std::make_unique<Style::Update>(*this);
+ textUpdate->addText(text);
+
+ RenderTreeUpdater renderTreeUpdater(*this);
+ renderTreeUpdater.commit(WTFMove(textUpdate));
+}
+
bool Document::needsStyleRecalc() const
{
if (pageCacheState() != NotInPageCache)
Modified: trunk/Source/WebCore/dom/Document.h (220446 => 220447)
--- trunk/Source/WebCore/dom/Document.h 2017-08-09 07:16:56 UTC (rev 220446)
+++ trunk/Source/WebCore/dom/Document.h 2017-08-09 07:36:45 UTC (rev 220447)
@@ -1237,6 +1237,8 @@
bool inStyleRecalc() const { return m_inStyleRecalc; }
bool inRenderTreeUpdate() const { return m_inRenderTreeUpdate; }
+ void updateTextRenderer(Text&);
+
// Return a Locale for the default locale if the argument is null or empty.
Locale& getCachedLocale(const AtomicString& locale = nullAtom());
Modified: trunk/Source/WebCore/dom/Text.cpp (220446 => 220447)
--- trunk/Source/WebCore/dom/Text.cpp 2017-08-09 07:16:56 UTC (rev 220446)
+++ trunk/Source/WebCore/dom/Text.cpp 2017-08-09 07:36:45 UTC (rev 220447)
@@ -26,7 +26,6 @@
#include "RenderCombineText.h"
#include "RenderSVGInlineText.h"
#include "RenderText.h"
-#include "RenderTreeUpdater.h"
#include "SVGElement.h"
#include "SVGNames.h"
#include "ScopedEventQueue.h"
@@ -218,12 +217,8 @@
if (styleValidity() >= Style::Validity::SubtreeAndRenderersInvalid)
return;
- auto textUpdate = std::make_unique<Style::Update>(document());
- textUpdate->addText(*this);
+ document().updateTextRenderer(*this);
- RenderTreeUpdater renderTreeUpdater(document());
- renderTreeUpdater.commit(WTFMove(textUpdate));
-
if (auto* renderer = this->renderer())
renderer->setTextWithOffset(data(), offsetOfReplacedData, lengthOfReplacedData);
}
Modified: trunk/Source/WebCore/rendering/RenderDescendantIterator.h (220446 => 220447)
--- trunk/Source/WebCore/rendering/RenderDescendantIterator.h 2017-08-09 07:16:56 UTC (rev 220446)
+++ trunk/Source/WebCore/rendering/RenderDescendantIterator.h 2017-08-09 07:36:45 UTC (rev 220447)
@@ -51,6 +51,7 @@
RenderDescendantIteratorAdapter(RenderElement& root);
RenderDescendantIterator<T> begin();
RenderDescendantIterator<T> end();
+ RenderDescendantIterator<T> at(T&);
private:
RenderElement& m_root;
@@ -62,6 +63,7 @@
RenderDescendantConstIteratorAdapter(const RenderElement& root);
RenderDescendantConstIterator<T> begin() const;
RenderDescendantConstIterator<T> end() const;
+ RenderDescendantConstIterator<T> at(const T&) const;
private:
const RenderElement& m_root;
@@ -130,6 +132,12 @@
return RenderDescendantIterator<T>(m_root);
}
+template <typename T>
+inline RenderDescendantIterator<T> RenderDescendantIteratorAdapter<T>::at(T& current)
+{
+ return RenderDescendantIterator<T>(m_root, ¤t);
+}
+
// RenderDescendantConstIteratorAdapter
template <typename T>
@@ -150,6 +158,12 @@
return RenderDescendantConstIterator<T>(m_root);
}
+template <typename T>
+inline RenderDescendantConstIterator<T> RenderDescendantConstIteratorAdapter<T>::at(const T& current) const
+{
+ return RenderDescendantConstIterator<T>(m_root, ¤t);
+}
+
// Standalone functions
template <typename T>
Modified: trunk/Source/WebCore/rendering/RenderQuote.cpp (220446 => 220447)
--- trunk/Source/WebCore/rendering/RenderQuote.cpp 2017-08-09 07:16:56 UTC (rev 220446)
+++ trunk/Source/WebCore/rendering/RenderQuote.cpp 2017-08-09 07:36:45 UTC (rev 220447)
@@ -1,7 +1,7 @@
/*
* Copyright (C) 2011 Nokia Inc. All rights reserved.
* Copyright (C) 2012 Google Inc. All rights reserved.
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2017 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -44,33 +44,25 @@
// Do not add any code here. Add it to willBeDestroyed() instead.
}
-void RenderQuote::willBeDestroyed()
-{
- detachQuote();
-
- ASSERT(!m_isAttached);
- ASSERT(!m_next);
- ASSERT(!m_previous);
-
- RenderInline::willBeDestroyed();
-}
-
void RenderQuote::insertedIntoTree()
{
RenderInline::insertedIntoTree();
- attachQuote();
+ view().registerQuote(*this);
}
void RenderQuote::willBeRemovedFromTree()
{
+ view().unregisterQuote(*this);
RenderInline::willBeRemovedFromTree();
- detachQuote();
}
void RenderQuote::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
{
RenderInline::styleDidChange(diff, oldStyle);
- updateText();
+ if (diff >= StyleDifferenceLayout) {
+ m_needsTextUpdate = true;
+ view().setHasSpecialRendererNeedingUpdate();
+ }
}
const unsigned maxDistinctQuoteCharacters = 16;
@@ -355,14 +347,15 @@
return downcast<RenderTextFragment>(lastChild);
}
-void RenderQuote::updateText()
+void RenderQuote::updateTextRenderer()
{
+ ASSERT_WITH_SECURITY_IMPLICATION(document().inRenderTreeUpdate());
+ ASSERT_WITH_SECURITY_IMPLICATION(!view().renderTreeIsBeingMutatedInternally());
+
String text = computeText();
if (m_text == text)
return;
m_text = text;
- // Start from the end of the child list because, if we've had a first-letter
- // renderer inserted then the remaining text will be at the end.
if (auto* renderText = quoteTextRenderer(lastChild())) {
renderText->setContentString(m_text);
renderText->dirtyLineBoxes(false);
@@ -395,105 +388,25 @@
return emptyString();
}
-void RenderQuote::attachQuote()
+void RenderQuote::updateRenderers(const RenderView& view)
{
- if (view().renderTreeIsBeingMutatedInternally())
- return;
+ int depth = -1;
+ for (auto* quote : view.quotes()) {
+ bool isOpen = quote->m_type == OPEN_QUOTE || quote->m_type == NO_OPEN_QUOTE;
+ if (!isOpen)
+ --depth;
+ else if (depth < 0)
+ depth = 0;
- ASSERT(!m_isAttached);
- ASSERT(!m_next);
- ASSERT(!m_previous);
- ASSERT(isRooted());
-
- // Optimize case where this is the first quote in a RenderView by not searching for predecessors in that case.
- if (view().renderQuoteHead()) {
- for (RenderObject* predecessor = previousInPreOrder(); predecessor; predecessor = predecessor->previousInPreOrder()) {
- // Skip unattached predecessors to avoid having stale m_previous pointers
- // if the previous node is never attached and is then destroyed.
- if (!is<RenderQuote>(*predecessor) || !downcast<RenderQuote>(*predecessor).m_isAttached)
- continue;
- m_previous = downcast<RenderQuote>(predecessor);
- m_next = m_previous->m_next;
- m_previous->m_next = this;
- if (m_next)
- m_next->m_previous = this;
- break;
+ if (quote->m_depth != depth || quote->m_needsTextUpdate) {
+ quote->m_depth = depth;
+ quote->m_needsTextUpdate = false;
+ quote->updateTextRenderer();
}
- }
- if (!m_previous) {
- m_next = view().renderQuoteHead();
- view().setRenderQuoteHead(this);
- if (m_next)
- m_next->m_previous = this;
+ if (isOpen)
+ ++depth;
}
-
- m_isAttached = true;
-
- for (RenderQuote* quote = this; quote; quote = quote->m_next)
- quote->updateDepth();
-
- ASSERT(!m_next || m_next->m_isAttached);
- ASSERT(!m_next || m_next->m_previous == this);
- ASSERT(!m_previous || m_previous->m_isAttached);
- ASSERT(!m_previous || m_previous->m_next == this);
}
-void RenderQuote::detachQuote()
-{
- if (view().renderTreeIsBeingMutatedInternally())
- return;
-
- ASSERT(!m_next || m_next->m_isAttached);
- ASSERT(!m_previous || m_previous->m_isAttached);
- if (!m_isAttached)
- return;
- if (m_previous)
- m_previous->m_next = m_next;
- else
- view().setRenderQuoteHead(m_next);
- if (m_next)
- m_next->m_previous = m_previous;
- if (!renderTreeBeingDestroyed()) {
- for (RenderQuote* quote = m_next; quote; quote = quote->m_next)
- quote->updateDepth();
- }
- m_isAttached = false;
- m_next = 0;
- m_previous = 0;
-}
-
-void RenderQuote::updateDepth()
-{
- ASSERT(m_isAttached);
- int depth = 0;
- if (m_previous) {
- depth = m_previous->m_depth;
- if (depth < 0)
- depth = 0;
- switch (m_previous->m_type) {
- case OPEN_QUOTE:
- case NO_OPEN_QUOTE:
- depth++;
- break;
- case CLOSE_QUOTE:
- case NO_CLOSE_QUOTE:
- break;
- }
- }
- switch (m_type) {
- case OPEN_QUOTE:
- case NO_OPEN_QUOTE:
- break;
- case CLOSE_QUOTE:
- case NO_CLOSE_QUOTE:
- depth--;
- break;
- }
- if (m_depth == depth)
- return;
- m_depth = depth;
- updateText();
-}
-
} // namespace WebCore
Modified: trunk/Source/WebCore/rendering/RenderQuote.h (220446 => 220447)
--- trunk/Source/WebCore/rendering/RenderQuote.h 2017-08-09 07:16:56 UTC (rev 220446)
+++ trunk/Source/WebCore/rendering/RenderQuote.h 2017-08-09 07:36:45 UTC (rev 220447)
@@ -31,12 +31,9 @@
RenderQuote(Document&, RenderStyle&&, QuoteType);
virtual ~RenderQuote();
- void attachQuote();
+ static void updateRenderers(const RenderView&);
private:
- void willBeDestroyed() override;
- void detachQuote();
-
const char* renderName() const override { return "RenderQuote"; }
bool isQuote() const override { return true; }
void styleDidChange(StyleDifference, const RenderStyle*) override;
@@ -44,15 +41,13 @@
void willBeRemovedFromTree() override;
String computeText() const;
- void updateText();
- void updateDepth();
+ void updateTextRenderer();
const QuoteType m_type;
int m_depth { -1 };
- RenderQuote* m_next { nullptr };
- RenderQuote* m_previous { nullptr };
- bool m_isAttached { false };
String m_text;
+
+ bool m_needsTextUpdate { false };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/rendering/RenderView.cpp (220446 => 220447)
--- trunk/Source/WebCore/rendering/RenderView.cpp 2017-08-09 07:16:56 UTC (rev 220446)
+++ trunk/Source/WebCore/rendering/RenderView.cpp 2017-08-09 07:36:45 UTC (rev 220447)
@@ -38,6 +38,7 @@
#include "ImageQualityController.h"
#include "NodeTraversal.h"
#include "Page.h"
+#include "RenderDescendantIterator.h"
#include "RenderGeometryMap.h"
#include "RenderIterator.h"
#include "RenderLayer.h"
@@ -47,6 +48,7 @@
#include "RenderMultiColumnSet.h"
#include "RenderMultiColumnSpannerPlaceholder.h"
#include "RenderNamedFlowThread.h"
+#include "RenderQuote.h"
#include "RenderSelectionInfo.h"
#include "RenderWidget.h"
#include "ScrollbarTheme.h"
@@ -1522,6 +1524,48 @@
return 0;
}
+void RenderView::registerQuote(RenderQuote& quote)
+{
+ ASSERT(!m_quotes.contains("e));
+
+ setHasSpecialRendererNeedingUpdate();
+
+ if (m_quotes.isEmpty()) {
+ m_quotes.add("e);
+ return;
+ }
+ auto quoteRenderers = descendantsOfType<RenderQuote>(*this);
+ auto it = quoteRenderers.at(quote);
+ if (++it == quoteRenderers.end()) {
+ m_quotes.add("e);
+ return;
+ }
+ auto& nextQuote = *it;
+ ASSERT(m_quotes.contains(&nextQuote));
+ m_quotes.insertBefore(&nextQuote, "e);
+}
+
+void RenderView::unregisterQuote(RenderQuote& quote)
+{
+ ASSERT(m_quotes.contains("e));
+
+ setHasSpecialRendererNeedingUpdate();
+
+ m_quotes.remove("e);
+}
+
+void RenderView::updateSpecialRenderers()
+{
+ ASSERT_WITH_SECURITY_IMPLICATION(document().inRenderTreeUpdate());
+ ASSERT_WITH_SECURITY_IMPLICATION(!renderTreeIsBeingMutatedInternally());
+
+ if (!m_hasSpecialRendererNeedingUpdate)
+ return;
+ m_hasSpecialRendererNeedingUpdate = false;
+
+ RenderQuote::updateRenderers(*this);
+}
+
#if ENABLE(CSS_SCROLL_SNAP)
void RenderView::registerBoxWithScrollSnapPositions(const RenderBox& box)
{
Modified: trunk/Source/WebCore/rendering/RenderView.h (220446 => 220447)
--- trunk/Source/WebCore/rendering/RenderView.h 2017-08-09 07:16:56 UTC (rev 220446)
+++ trunk/Source/WebCore/rendering/RenderView.h 2017-08-09 07:36:45 UTC (rev 220447)
@@ -29,6 +29,7 @@
#include "SelectionSubtreeRoot.h"
#include <memory>
#include <wtf/HashSet.h>
+#include <wtf/ListHashSet.h>
#if ENABLE(SERVICE_CONTROLS)
#include "SelectionRectGatherer.h"
@@ -195,9 +196,13 @@
IntSize viewportSizeForCSSViewportUnits() const;
- void setRenderQuoteHead(RenderQuote* head) { m_renderQuoteHead = head; }
- RenderQuote* renderQuoteHead() const { return m_renderQuoteHead; }
-
+ void registerQuote(RenderQuote&);
+ void unregisterQuote(RenderQuote&);
+ const ListHashSet<RenderQuote*>& quotes() const { return m_quotes; }
+
+ void setHasSpecialRendererNeedingUpdate() { m_hasSpecialRendererNeedingUpdate = true; }
+ void updateSpecialRenderers();
+
// FIXME: see class RenderTreeInternalMutation below.
bool renderTreeIsBeingMutatedInternally() const { return !!m_renderTreeInternalMutationCounter; }
@@ -371,7 +376,9 @@
std::unique_ptr<RenderLayerCompositor> m_compositor;
std::unique_ptr<FlowThreadController> m_flowThreadController;
- RenderQuote* m_renderQuoteHead { nullptr };
+ ListHashSet<RenderQuote*> m_quotes;
+ bool m_hasSpecialRendererNeedingUpdate { false };
+
unsigned m_renderCounterCount { 0 };
unsigned m_renderTreeInternalMutationCounter { 0 };
Modified: trunk/Source/WebCore/style/RenderTreeUpdater.cpp (220446 => 220447)
--- trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-08-09 07:16:56 UTC (rev 220446)
+++ trunk/Source/WebCore/style/RenderTreeUpdater.cpp 2017-08-09 07:36:45 UTC (rev 220447)
@@ -124,6 +124,8 @@
for (auto* root : findRenderingRoots(*m_styleUpdate))
updateRenderTree(*root);
+ m_document.renderView()->updateSpecialRenderers();
+
m_styleUpdate = nullptr;
}
