Title: [235729] branches/safari-606-branch
Revision
235729
Author
[email protected]
Date
2018-09-06 01:11:54 -0700 (Thu, 06 Sep 2018)

Log Message

Cherry-pick r235537. rdar://problem/44169516

    CounterMaps should hold a unique_ptr of CounterMap.
    https://bugs.webkit.org/show_bug.cgi?id=189174
    <rdar://problem/43686458>

    Reviewed by Ryosuke Niwa.

    Source/WebCore:

    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):

    LayoutTests:

    * fast/css/counters/crash-when-cloning-body-expected.txt: Added.
    * fast/css/counters/crash-when-cloning-body.html: Added.

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@235537 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Added Paths

Diff

Modified: branches/safari-606-branch/LayoutTests/ChangeLog (235728 => 235729)


--- branches/safari-606-branch/LayoutTests/ChangeLog	2018-09-06 08:11:50 UTC (rev 235728)
+++ branches/safari-606-branch/LayoutTests/ChangeLog	2018-09-06 08:11:54 UTC (rev 235729)
@@ -1,5 +1,47 @@
 2018-09-06  Babak Shafiei  <[email protected]>
 
+        Cherry-pick r235537. rdar://problem/44169516
+
+    CounterMaps should hold a unique_ptr of CounterMap.
+    https://bugs.webkit.org/show_bug.cgi?id=189174
+    <rdar://problem/43686458>
+    
+    Reviewed by Ryosuke Niwa.
+    
+    Source/WebCore:
+    
+    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):
+    
+    LayoutTests:
+    
+    * fast/css/counters/crash-when-cloning-body-expected.txt: Added.
+    * fast/css/counters/crash-when-cloning-body.html: Added.
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@235537 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-09-06  Babak Shafiei  <[email protected]>
+
         Cherry-pick r233902. rdar://problem/44168991
 
     Rebaseline imported/w3c/web-platform-tests/WebCryptoAPI/derive_bits_keys/pbkdf2.https.worker.html for Sierra after r233898.

Added: branches/safari-606-branch/LayoutTests/fast/css/counters/crash-when-cloning-body-expected.txt (0 => 235729)


--- branches/safari-606-branch/LayoutTests/fast/css/counters/crash-when-cloning-body-expected.txt	                        (rev 0)
+++ branches/safari-606-branch/LayoutTests/fast/css/counters/crash-when-cloning-body-expected.txt	2018-09-06 08:11:54 UTC (rev 235729)
@@ -0,0 +1,2 @@
+Pass if no crash.
+Pass if no crash.

Added: branches/safari-606-branch/LayoutTests/fast/css/counters/crash-when-cloning-body.html (0 => 235729)


--- branches/safari-606-branch/LayoutTests/fast/css/counters/crash-when-cloning-body.html	                        (rev 0)
+++ branches/safari-606-branch/LayoutTests/fast/css/counters/crash-when-cloning-body.html	2018-09-06 08:11:54 UTC (rev 235729)
@@ -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: branches/safari-606-branch/Source/WebCore/ChangeLog (235728 => 235729)


--- branches/safari-606-branch/Source/WebCore/ChangeLog	2018-09-06 08:11:50 UTC (rev 235728)
+++ branches/safari-606-branch/Source/WebCore/ChangeLog	2018-09-06 08:11:54 UTC (rev 235729)
@@ -1,5 +1,56 @@
 2018-09-06  Babak Shafiei  <[email protected]>
 
+        Cherry-pick r235537. rdar://problem/44169516
+
+    CounterMaps should hold a unique_ptr of CounterMap.
+    https://bugs.webkit.org/show_bug.cgi?id=189174
+    <rdar://problem/43686458>
+    
+    Reviewed by Ryosuke Niwa.
+    
+    Source/WebCore:
+    
+    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):
+    
+    LayoutTests:
+    
+    * fast/css/counters/crash-when-cloning-body-expected.txt: Added.
+    * fast/css/counters/crash-when-cloning-body.html: Added.
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@235537 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    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-09-06  Babak Shafiei  <[email protected]>
+
         Cherry-pick r233898. rdar://problem/44168991
 
     [WebCrypto] Crypto operations should copy their parameters before hoping to another thread

Modified: branches/safari-606-branch/Source/WebCore/rendering/RenderCounter.cpp (235728 => 235729)


--- branches/safari-606-branch/Source/WebCore/rendering/RenderCounter.cpp	2018-09-06 08:11:50 UTC (rev 235728)
+++ branches/safari-606-branch/Source/WebCore/rendering/RenderCounter.cpp	2018-09-06 08:11:54 UTC (rev 235729)
@@ -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);
 }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to