Diff
Modified: trunk/Source/WebCore/ChangeLog (207373 => 207374)
--- trunk/Source/WebCore/ChangeLog 2016-10-15 09:15:41 UTC (rev 207373)
+++ trunk/Source/WebCore/ChangeLog 2016-10-15 14:24:33 UTC (rev 207374)
@@ -1,3 +1,36 @@
+2016-10-15 Zalan Bujtas <za...@apple.com>
+
+ CounterNode::resetRenderers is so inefficient.
+ https://bugs.webkit.org/show_bug.cgi?id=163480
+
+ Reviewed by Simon Fraser.
+
+ CounterNode::resetRenderers() removes all the associated renderers from this CounterNode
+ and sets the dirty bit on them.
+ This patch does all that in a loop, instead of traversing the linked tree on each removal.
+
+ No change in functionality.
+
+ * rendering/CounterNode.cpp:
+ (WebCore::CounterNode::CounterNode):
+ (WebCore::CounterNode::~CounterNode):
+ (WebCore::CounterNode::nextInPreOrderAfterChildren):
+ (WebCore::CounterNode::lastDescendant):
+ (WebCore::CounterNode::addRenderer): These assertions do not seem super useful.
+ (WebCore::CounterNode::removeRenderer):
+ (WebCore::CounterNode::resetRenderers):
+ (WebCore::CounterNode::insertAfter):
+ (WebCore::CounterNode::removeChild):
+ * rendering/CounterNode.h:
+ * rendering/RenderCounter.cpp:
+ (WebCore::makeCounterNode):
+ (WebCore::RenderCounter::RenderCounter):
+ (WebCore::RenderCounter::~RenderCounter):
+ (WebCore::RenderCounter::originalText):
+ (WebCore::updateCounters):
+ (WebCore::RenderCounter::invalidate): Deleted.
+ * rendering/RenderCounter.h:
+
2016-10-15 Antoine Quint <grao...@apple.com>
[Modern Media Controls] macOS inline controls
Modified: trunk/Source/WebCore/rendering/CounterNode.cpp (207373 => 207374)
--- trunk/Source/WebCore/rendering/CounterNode.cpp 2016-10-15 09:15:41 UTC (rev 207373)
+++ trunk/Source/WebCore/rendering/CounterNode.cpp 2016-10-15 14:24:33 UTC (rev 207374)
@@ -33,12 +33,6 @@
, m_value(value)
, m_countInParent(0)
, m_owner(owner)
- , m_rootRenderer(0)
- , m_parent(0)
- , m_previousSibling(0)
- , m_nextSibling(0)
- , m_firstChild(0)
- , m_lastChild(0)
{
}
@@ -47,8 +41,8 @@
// Ideally this would be an assert and this would never be reached. In reality this happens a lot
// so we need to handle these cases. The node is still connected to the tree so we need to detach it.
if (m_parent || m_previousSibling || m_nextSibling || m_firstChild || m_lastChild) {
- CounterNode* oldParent = 0;
- CounterNode* oldPreviousSibling = 0;
+ CounterNode* oldParent = nullptr;
+ CounterNode* oldPreviousSibling = nullptr;
// Instead of calling removeChild() we do this safely as the tree is likely broken if we get here.
if (m_parent) {
if (m_parent->m_firstChild == this)
@@ -56,24 +50,24 @@
if (m_parent->m_lastChild == this)
m_parent->m_lastChild = m_previousSibling;
oldParent = m_parent;
- m_parent = 0;
+ m_parent = nullptr;
}
if (m_previousSibling) {
if (m_previousSibling->m_nextSibling == this)
m_previousSibling->m_nextSibling = m_nextSibling;
oldPreviousSibling = m_previousSibling;
- m_previousSibling = 0;
+ m_previousSibling = nullptr;
}
if (m_nextSibling) {
if (m_nextSibling->m_previousSibling == this)
m_nextSibling->m_previousSibling = oldPreviousSibling;
- m_nextSibling = 0;
+ m_nextSibling = nullptr;
}
if (m_firstChild) {
// The node's children are reparented to the old parent.
for (CounterNode* child = m_firstChild; child; ) {
CounterNode* nextChild = child->m_nextSibling;
- CounterNode* nextSibling = 0;
+ CounterNode* nextSibling = nullptr;
child->m_parent = oldParent;
if (oldPreviousSibling) {
nextSibling = oldPreviousSibling->m_nextSibling;
@@ -98,7 +92,7 @@
CounterNode* CounterNode::nextInPreOrderAfterChildren(const CounterNode* stayWithin) const
{
if (this == stayWithin)
- return 0;
+ return nullptr;
const CounterNode* current = this;
CounterNode* next;
@@ -105,7 +99,7 @@
while (!(next = current->m_nextSibling)) {
current = current->m_parent;
if (!current || current == stayWithin)
- return 0;
+ return nullptr;
}
return next;
}
@@ -122,7 +116,7 @@
{
CounterNode* last = m_lastChild;
if (!last)
- return 0;
+ return nullptr;
while (CounterNode* lastChild = last->m_lastChild)
last = lastChild;
@@ -151,64 +145,49 @@
return m_parent->m_value + increment;
}
-void CounterNode::addRenderer(RenderCounter* value)
+void CounterNode::addRenderer(RenderCounter& renderer)
{
- if (!value) {
- ASSERT_NOT_REACHED();
- return;
- }
- if (value->m_counterNode) {
- ASSERT_NOT_REACHED();
- value->m_counterNode->removeRenderer(value);
- }
- ASSERT(!value->m_nextForSameCounter);
- for (RenderCounter* iterator = m_rootRenderer;iterator; iterator = iterator->m_nextForSameCounter) {
- if (iterator == value) {
- ASSERT_NOT_REACHED();
- return;
- }
- }
- value->m_nextForSameCounter = m_rootRenderer;
- m_rootRenderer = value;
- if (value->m_counterNode != this) {
- if (value->m_counterNode) {
- ASSERT_NOT_REACHED();
- value->m_counterNode->removeRenderer(value);
- }
- value->m_counterNode = this;
- }
+ ASSERT(!renderer.m_counterNode);
+ ASSERT(!renderer.m_nextForSameCounter);
+ renderer.m_nextForSameCounter = m_rootRenderer;
+ m_rootRenderer = &renderer;
+ renderer.m_counterNode = this;
}
-void CounterNode::removeRenderer(RenderCounter* value)
+void CounterNode::removeRenderer(RenderCounter& renderer)
{
- if (!value) {
- ASSERT_NOT_REACHED();
+ ASSERT(renderer.m_counterNode && renderer.m_counterNode == this);
+ RenderCounter* previous = nullptr;
+ for (auto* current = m_rootRenderer; current; previous = current, current = current->m_nextForSameCounter) {
+ if (current != &renderer)
+ continue;
+
+ if (previous)
+ previous->m_nextForSameCounter = renderer.m_nextForSameCounter;
+ else
+ m_rootRenderer = renderer.m_nextForSameCounter;
+ renderer.m_nextForSameCounter = nullptr;
+ renderer.m_counterNode = nullptr;
return;
}
- if (value->m_counterNode && value->m_counterNode != this) {
- ASSERT_NOT_REACHED();
- value->m_counterNode->removeRenderer(value);
- }
- RenderCounter* previous = 0;
- for (RenderCounter* iterator = m_rootRenderer;iterator; iterator = iterator->m_nextForSameCounter) {
- if (iterator == value) {
- if (previous)
- previous->m_nextForSameCounter = value->m_nextForSameCounter;
- else
- m_rootRenderer = value->m_nextForSameCounter;
- value->m_nextForSameCounter = 0;
- value->m_counterNode = 0;
- return;
- }
- previous = iterator;
- }
ASSERT_NOT_REACHED();
}
void CounterNode::resetRenderers()
{
- while (m_rootRenderer)
- m_rootRenderer->invalidate(); // This makes m_rootRenderer point to the next renderer if any since it disconnects the m_rootRenderer from this.
+ if (!m_rootRenderer)
+ return;
+ bool skipLayoutAndPerfWidthsRecalc = m_rootRenderer->documentBeingDestroyed();
+ auto* current = m_rootRenderer;
+ while (current) {
+ if (!skipLayoutAndPerfWidthsRecalc)
+ current->setNeedsLayoutAndPrefWidthsRecalc();
+ auto* next = current->m_nextForSameCounter;
+ current->m_nextForSameCounter = nullptr;
+ current->m_counterNode = nullptr;
+ current = next;
+ }
+ m_rootRenderer = nullptr;
}
void CounterNode::resetThisAndDescendantsRenderers()
@@ -312,8 +291,8 @@
break;
}
}
- newChild->m_firstChild = 0;
- newChild->m_lastChild = 0;
+ newChild->m_firstChild = nullptr;
+ newChild->m_lastChild = nullptr;
newChild->m_countInParent = newChild->computeCountInParent();
newChild->resetRenderers();
first->recount();
@@ -328,9 +307,9 @@
CounterNode* next = oldChild->m_nextSibling;
CounterNode* previous = oldChild->m_previousSibling;
- oldChild->m_nextSibling = 0;
- oldChild->m_previousSibling = 0;
- oldChild->m_parent = 0;
+ oldChild->m_nextSibling = nullptr;
+ oldChild->m_previousSibling = nullptr;
+ oldChild->m_parent = nullptr;
if (previous)
previous->m_nextSibling = next;
Modified: trunk/Source/WebCore/rendering/CounterNode.h (207373 => 207374)
--- trunk/Source/WebCore/rendering/CounterNode.h 2016-10-15 09:15:41 UTC (rev 207373)
+++ trunk/Source/WebCore/rendering/CounterNode.h 2016-10-15 14:24:33 UTC (rev 207374)
@@ -49,8 +49,8 @@
int value() const { return m_value; }
int countInParent() const { return m_countInParent; }
RenderElement& owner() const { return m_owner; }
- void addRenderer(RenderCounter*);
- void removeRenderer(RenderCounter*);
+ void addRenderer(RenderCounter&);
+ void removeRenderer(RenderCounter&);
// Invalidates the text in the renderers of this counter, if any.
void resetRenderers();
@@ -62,8 +62,8 @@
CounterNode* lastChild() const { return m_lastChild; }
CounterNode* lastDescendant() const;
CounterNode* previousInPreOrder() const;
- CounterNode* nextInPreOrder(const CounterNode* stayWithin = 0) const;
- CounterNode* nextInPreOrderAfterChildren(const CounterNode* stayWithin = 0) const;
+ CounterNode* nextInPreOrder(const CounterNode* stayWithin = nullptr) const;
+ CounterNode* nextInPreOrderAfterChildren(const CounterNode* stayWithin = nullptr) const;
void insertAfter(CounterNode* newChild, CounterNode* beforeChild, const AtomicString& identifier);
@@ -82,13 +82,13 @@
int m_value;
int m_countInParent;
RenderElement& m_owner;
- RenderCounter* m_rootRenderer;
+ RenderCounter* m_rootRenderer { nullptr };
- CounterNode* m_parent;
- CounterNode* m_previousSibling;
- CounterNode* m_nextSibling;
- CounterNode* m_firstChild;
- CounterNode* m_lastChild;
+ CounterNode* m_parent { nullptr };
+ CounterNode* m_previousSibling { nullptr };
+ CounterNode* m_nextSibling { nullptr };
+ CounterNode* m_firstChild { nullptr };
+ CounterNode* m_lastChild { nullptr };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/rendering/RenderCounter.cpp (207373 => 207374)
--- trunk/Source/WebCore/rendering/RenderCounter.cpp 2016-10-15 09:15:41 UTC (rev 207373)
+++ trunk/Source/WebCore/rendering/RenderCounter.cpp 2016-10-15 14:24:33 UTC (rev 207374)
@@ -304,8 +304,8 @@
if (!planCounter(renderer, identifier, isReset, value) && !alwaysCreateCounter)
return nullptr;
- RefPtr<CounterNode> newParent = 0;
- RefPtr<CounterNode> newPreviousSibling = 0;
+ RefPtr<CounterNode> newParent;
+ RefPtr<CounterNode> newPreviousSibling;
RefPtr<CounterNode> newNode = CounterNode::create(renderer, isReset, value);
if (findPlaceForCounter(renderer, identifier, isReset, newParent, newPreviousSibling))
newParent->insertAfter(newNode.get(), newPreviousSibling.get(), identifier);
@@ -345,8 +345,6 @@
RenderCounter::RenderCounter(Document& document, const CounterContent& counter)
: RenderText(document, emptyString())
, m_counter(counter)
- , m_counterNode(nullptr)
- , m_nextForSameCounter(0)
{
view().addRenderCounter();
}
@@ -356,7 +354,7 @@
view().removeRenderCounter();
if (m_counterNode) {
- m_counterNode->removeRenderer(this);
+ m_counterNode->removeRenderer(*this);
ASSERT(!m_counterNode);
}
}
@@ -385,7 +383,7 @@
break;
beforeAfterContainer = beforeAfterContainer->parent();
}
- makeCounterNode(*beforeAfterContainer, m_counter.identifier(), true)->addRenderer(const_cast<RenderCounter*>(this));
+ makeCounterNode(*beforeAfterContainer, m_counter.identifier(), true)->addRenderer(const_cast<RenderCounter&>(*this));
ASSERT(m_counterNode);
}
CounterNode* child = m_counterNode;
@@ -427,15 +425,6 @@
RenderText::computePreferredLogicalWidths(lead);
}
-void RenderCounter::invalidate()
-{
- m_counterNode->removeRenderer(this);
- ASSERT(!m_counterNode);
- if (documentBeingDestroyed())
- return;
- setNeedsLayoutAndPrefWidthsRecalc();
-}
-
static void destroyCounterNodeWithoutMapRemoval(const AtomicString& identifier, CounterNode* node)
{
CounterNode* previous;
@@ -522,8 +511,8 @@
makeCounterNode(renderer, it->key, false);
continue;
}
- RefPtr<CounterNode> newParent = 0;
- RefPtr<CounterNode> newPreviousSibling = 0;
+ RefPtr<CounterNode> newParent;
+ RefPtr<CounterNode> newPreviousSibling;
findPlaceForCounter(renderer, it->key, node->hasResetType(), newParent, newPreviousSibling);
if (node != counterMap->get(it->key))
Modified: trunk/Source/WebCore/rendering/RenderCounter.h (207373 => 207374)
--- trunk/Source/WebCore/rendering/RenderCounter.h 2016-10-15 09:15:41 UTC (rev 207373)
+++ trunk/Source/WebCore/rendering/RenderCounter.h 2016-10-15 14:24:33 UTC (rev 207374)
@@ -49,13 +49,9 @@
void computePreferredLogicalWidths(float leadWidth) override;
- // Removes the reference to the CounterNode associated with this renderer.
- // This is used to cause a counter display update when the CounterNode tree changes.
- void invalidate();
-
CounterContent m_counter;
- CounterNode* m_counterNode;
- RenderCounter* m_nextForSameCounter;
+ CounterNode* m_counterNode { nullptr };
+ RenderCounter* m_nextForSameCounter { nullptr };
friend class CounterNode;
};
@@ -65,7 +61,7 @@
#if ENABLE(TREE_DEBUGGING)
// Outside the WebCore namespace for ease of invocation from gdb.
-void showCounterRendererTree(const WebCore::RenderObject*, const char* counterName = 0);
+void showCounterRendererTree(const WebCore::RenderObject*, const char* counterName = nullptr);
#endif
#endif // RenderCounter_h
