Modified: trunk/LayoutTests/ChangeLog (235536 => 235537)
--- trunk/LayoutTests/ChangeLog 2018-08-31 04:17:19 UTC (rev 235536)
+++ trunk/LayoutTests/ChangeLog 2018-08-31 04:27:44 UTC (rev 235537)
@@ -1,3 +1,14 @@
+2018-08-30 Zalan Bujtas <[email protected]>
+
+ CounterMaps should hold a unique_ptr of CounterMap.
+ https://bugs.webkit.org/show_bug.cgi?id=189174
+ <rdar://problem/43686458>
+
+ Reviewed by Ryosuke Niwa.
+
+ * fast/css/counters/crash-when-cloning-body-expected.txt: Added.
+ * fast/css/counters/crash-when-cloning-body.html: Added.
+
2018-08-30 Truitt Savell <[email protected]>
Unreviewed, rolling out r235516.
Added: trunk/LayoutTests/fast/css/counters/crash-when-cloning-body-expected.txt (0 => 235537)
--- trunk/LayoutTests/fast/css/counters/crash-when-cloning-body-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/counters/crash-when-cloning-body-expected.txt 2018-08-31 04:27:44 UTC (rev 235537)
@@ -0,0 +1,2 @@
+Pass if no crash.
+Pass if no crash.
Added: trunk/LayoutTests/fast/css/counters/crash-when-cloning-body.html (0 => 235537)
--- trunk/LayoutTests/fast/css/counters/crash-when-cloning-body.html (rev 0)
+++ trunk/LayoutTests/fast/css/counters/crash-when-cloning-body.html 2018-08-31 04:27:44 UTC (rev 235537)
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+*:first-child {
+ display: table;
+ counter-increment: foo2;
+}
+
+* {
+ display: inline;
+ -webkit-appearance: unset;
+ visibility: hidden;
+}
+
+:nth-last-child(1) {
+ counter-reset: foo1;
+}
+</style>
+</head>
+<body>
+<div id=firstDiv></div>
+<div id=secondDiv></div>
+<div style="visibility: visible">Pass if no crash.</div>
+</body>
+
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+document.body.offsetHeight;
+
+let div = document.createElement('div');
+div.appendChild(document.body.cloneNode(true))
+
+document.styleSheets[0].addRule('#firstDiv::after','content: counter(foobar)');
+
+secondDiv.appendChild(document.createElement('input'))
+firstDiv.appendChild(div);
+</script>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (235536 => 235537)
--- trunk/Source/WebCore/ChangeLog 2018-08-31 04:17:19 UTC (rev 235536)
+++ trunk/Source/WebCore/ChangeLog 2018-08-31 04:27:44 UTC (rev 235537)
@@ -1,3 +1,23 @@
+2018-08-30 Zalan Bujtas <[email protected]>
+
+ CounterMaps should hold a unique_ptr of CounterMap.
+ https://bugs.webkit.org/show_bug.cgi?id=189174
+ <rdar://problem/43686458>
+
+ Reviewed by Ryosuke Niwa.
+
+ In certain cases calls to CounterMaps might lead to unexpected deletion of the CounterMap object.
+
+ Test: fast/css/counters/crash-when-cloning-body.html
+
+ * rendering/RenderCounter.cpp:
+ (WebCore::makeCounterNode):
+ (WebCore::destroyCounterNodeWithoutMapRemoval):
+ (WebCore::RenderCounter::destroyCounterNodes):
+ (WebCore::RenderCounter::destroyCounterNode):
+ (WebCore::updateCounters):
+ (showCounterRendererTree):
+
2018-08-30 Ross Kirsling <[email protected]>
Speculative build fix for WPE after r235531.
Modified: trunk/Source/WebCore/rendering/RenderCounter.cpp (235536 => 235537)
--- trunk/Source/WebCore/rendering/RenderCounter.cpp 2018-08-31 04:17:19 UTC (rev 235536)
+++ trunk/Source/WebCore/rendering/RenderCounter.cpp 2018-08-31 04:27:44 UTC (rev 235537)
@@ -47,7 +47,7 @@
WTF_MAKE_ISO_ALLOCATED_IMPL(RenderCounter);
using CounterMap = HashMap<AtomicString, Ref<CounterNode>>;
-using CounterMaps = HashMap<const RenderElement*, CounterMap>;
+using CounterMaps = HashMap<const RenderElement*, std::unique_ptr<CounterMap>>;
static CounterNode* makeCounterNode(RenderElement&, const AtomicString& identifier, bool alwaysCreateCounter);
@@ -296,7 +296,7 @@
{
if (renderer.hasCounterNodeMap()) {
ASSERT(counterMaps().contains(&renderer));
- if (auto* node = counterMaps().find(&renderer)->value.get(identifier))
+ if (auto* node = counterMaps().find(&renderer)->value->get(identifier))
return node;
}
@@ -312,7 +312,7 @@
if (place.parent)
place.parent->insertAfter(newNode, place.previousSibling.get(), identifier);
- maps.add(&renderer, CounterMap { }).iterator->value.add(identifier, newNode.copyRef());
+ maps.add(&renderer, std::make_unique<CounterMap>()).iterator->value->add(identifier, newNode.copyRef());
renderer.setHasCounterNodeMap(true);
if (newNode->parent())
@@ -326,7 +326,7 @@
skipDescendants = false;
if (!currentRenderer->hasCounterNodeMap())
continue;
- auto* currentCounter = maps.find(currentRenderer)->value.get(identifier);
+ auto* currentCounter = maps.find(currentRenderer)->value->get(identifier);
if (!currentCounter)
continue;
skipDescendants = true;
@@ -436,8 +436,8 @@
for (RefPtr<CounterNode> child = node.lastDescendant(); child && child != &node; child = WTFMove(previous)) {
previous = child->previousInPreOrder();
child->parent()->removeChild(*child);
- ASSERT(counterMaps().find(&child->owner())->value.get(identifier) == child);
- counterMaps().find(&child->owner())->value.remove(identifier);
+ ASSERT(counterMaps().find(&child->owner())->value->get(identifier) == child);
+ counterMaps().find(&child->owner())->value->remove(identifier);
}
if (auto* parent = node.parent())
parent->removeChild(node);
@@ -448,8 +448,9 @@
ASSERT(owner.hasCounterNodeMap());
auto& maps = counterMaps();
ASSERT(maps.contains(&owner));
- for (auto& keyValue : maps.take(&owner))
- destroyCounterNodeWithoutMapRemoval(keyValue.key, keyValue.value);
+ auto counterMap = maps.take(&owner);
+ for (auto& counterMapEntry : *counterMap)
+ destroyCounterNodeWithoutMapRemoval(counterMapEntry.key, counterMapEntry.value);
owner.setHasCounterNodeMap(false);
}
@@ -458,7 +459,7 @@
auto map = counterMaps().find(&owner);
if (map == counterMaps().end())
return;
- auto node = map->value.take(identifier);
+ auto node = map->value->take(identifier);
if (!node)
return;
destroyCounterNodeWithoutMapRemoval(identifier, *node);
@@ -499,15 +500,15 @@
return;
}
ASSERT(counterMaps().contains(&renderer));
- auto& counterMap = counterMaps().find(&renderer)->value;
+ auto* counterMap = counterMaps().find(&renderer)->value.get();
for (auto& key : directiveMap->keys()) {
- RefPtr<CounterNode> node = counterMap.get(key);
+ RefPtr<CounterNode> node = counterMap->get(key);
if (!node) {
makeCounterNode(renderer, key, false);
continue;
}
auto place = findPlaceForCounter(renderer, key, node->hasResetType());
- if (node != counterMap.get(key))
+ if (node != counterMap->get(key))
continue;
CounterNode* parent = node->parent();
if (place.parent == parent && place.previousSibling == node->previousSibling())
@@ -601,7 +602,7 @@
fprintf(stderr, "%p N:%p P:%p PS:%p NS:%p C:%p\n",
current, current->node(), current->parent(), current->previousSibling(),
current->nextSibling(), downcast<WebCore::RenderElement>(*current).hasCounterNodeMap() ?
- counterName ? WebCore::counterMaps().find(downcast<WebCore::RenderElement>(current))->value.get(identifier) : (WebCore::CounterNode*)1 : (WebCore::CounterNode*)0);
+ counterName ? WebCore::counterMaps().find(downcast<WebCore::RenderElement>(current))->value->get(identifier) : (WebCore::CounterNode*)1 : (WebCore::CounterNode*)0);
}
fflush(stderr);
}
