Commit bf874685 authored by cdn@chromium.org's avatar cdn@chromium.org

Source/WebCore: Hold refptr to identified previous sibling within findPlaceForCounter.

https://bugs.webkit.org/show_bug.cgi?id=68563

Reviewed by Adam Barth.

Test: fast/css/counters/counter-after-style-crash.html

* rendering/RenderCounter.cpp:
(WebCore::findPlaceForCounter):

LayoutTests: Add test for crash when performing rich text mutations with counter nodes.
https://bugs.webkit.org/show_bug.cgi?id=68563

Reviewed by Adam Barth.

* fast/css/counters/counter-after-style-crash-expected.txt: Added.
* fast/css/counters/counter-after-style-crash.html: Added.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@96632 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 05850dd0
2011-09-21 Cris Neckar <cdn@chromium.org>
Add test for crash when performing rich text mutations with counter nodes.
https://bugs.webkit.org/show_bug.cgi?id=68563
Reviewed by Adam Barth.
* fast/css/counters/counter-after-style-crash-expected.txt: Added.
* fast/css/counters/counter-after-style-crash.html: Added.
2011-10-04 Joshua Bell <jsbell@chromium.org>
IndexedDB add() should fail if key is NaN
PASS: Counters updated successfully without crashing
<html>
<script>
if (window.layoutTestController) {
layoutTestController.waitUntilDone();
layoutTestController.dumpAsText();
}
</script>
<style>
div {
counter-reset:ctr
}
:after {
content:counter(x);
counter-increment:ctr;
}
</style>
<junk>TESTING..</div><div><div></div>
<span></span>
<table>
</script>
<script>
document.designMode='on';
document.execCommand('selectall');
document.execCommand('italic');
document.execCommand('removeformat');
document.body.innerHTML = "PASS: Counters updated successfully without crashing";
layoutTestController.notifyDone();
</script>
</table>
</html>
\ No newline at end of file
2011-09-21 Cris Neckar <cdn@chromium.org>
Hold refptr to identified previous sibling within findPlaceForCounter.
https://bugs.webkit.org/show_bug.cgi?id=68563
Reviewed by Adam Barth.
Test: fast/css/counters/counter-after-style-crash.html
* rendering/RenderCounter.cpp:
(WebCore::findPlaceForCounter):
2011-10-04 Joshua Bell <jsbell@chromium.org>
IndexedDB add() should fail if key is NaN
......@@ -303,13 +303,15 @@ static bool findPlaceForCounter(RenderObject* counterOwner, const AtomicString&
// we are trying to find a place for. This is the next renderer to be checked.
RenderObject* currentRenderer = previousInPreOrder(counterOwner);
previousSibling = 0;
RefPtr<CounterNode> previousSiblingProtector = 0;
while (currentRenderer) {
CounterNode* currentCounter = makeCounterNode(currentRenderer, identifier, false);
if (searchEndRenderer == currentRenderer) {
// We may be at the end of our search.
if (currentCounter) {
// We have a suitable counter on the EndSearchRenderer.
if (previousSibling) { // But we already found another counter that we come after.
if (previousSiblingProtector) { // But we already found another counter that we come after.
if (currentCounter->actsAsReset()) {
// We found a reset counter that is on a renderer that is a sibling of ours or a parent.
if (isReset && areRenderersElementsSiblings(currentRenderer, counterOwner)) {
......@@ -326,16 +328,19 @@ static bool findPlaceForCounter(RenderObject* counterOwner, const AtomicString&
// In some cases renders can be reparented (ex. nodes inside a table but not in a column or row).
// In these cases the identified previousSibling will be invalid as its parent is different from
// our identified parent.
if (previousSibling->parent() != currentCounter)
previousSibling = 0;
if (previousSiblingProtector->parent() != currentCounter)
previousSiblingProtector = 0;
previousSibling = previousSiblingProtector.get();
return true;
}
// CurrentCounter, the counter at the EndSearchRenderer, is not reset.
if (!isReset || !areRenderersElementsSiblings(currentRenderer, counterOwner)) {
// If the node we are placing is not reset or we have found a counter that is attached
// to an ancestor of the placed counter's owner renderer we know we are a sibling of that node.
ASSERT(currentCounter->parent() == previousSibling->parent());
ASSERT(currentCounter->parent() == previousSiblingProtector->parent());
parent = currentCounter->parent();
previousSibling = previousSiblingProtector.get();
return true;
}
} else {
......@@ -350,6 +355,7 @@ static bool findPlaceForCounter(RenderObject* counterOwner, const AtomicString&
return parent;
}
parent = currentCounter;
previousSibling = previousSiblingProtector.get();
return true;
}
if (!isReset || !areRenderersElementsSiblings(currentRenderer, counterOwner)) {
......@@ -357,7 +363,7 @@ static bool findPlaceForCounter(RenderObject* counterOwner, const AtomicString&
previousSibling = currentCounter;
return true;
}
previousSibling = currentCounter;
previousSiblingProtector = currentCounter;
}
}
// We come here if the previous sibling or parent of our owner renderer had no
......@@ -370,18 +376,18 @@ static bool findPlaceForCounter(RenderObject* counterOwner, const AtomicString&
// counter being placed is attached to.
if (currentCounter) {
// We found a suitable counter.
if (previousSibling) {
if (previousSiblingProtector) {
// Since we had a suitable previous counter before, we should only consider this one as our
// previousSibling if it is a reset counter and hence the current previousSibling is its child.
if (currentCounter->actsAsReset()) {
previousSibling = currentCounter;
previousSiblingProtector = currentCounter;
// We are no longer interested in previous siblings of the currentRenderer or their children
// as counters they may have attached cannot be the previous sibling of the counter we are placing.
currentRenderer = parentElement(currentRenderer)->renderer();
continue;
}
} else
previousSibling = currentCounter;
previousSiblingProtector = currentCounter;
currentRenderer = previousSiblingOrParent(currentRenderer);
continue;
}
......@@ -390,7 +396,7 @@ static bool findPlaceForCounter(RenderObject* counterOwner, const AtomicString&
// which may be done twice in some cases. Rearranging the decision points though, to accommodate this
// performance improvement would create more code duplication than is worthwhile in my oppinion and may further
// impede the readability of this already complex algorithm.
if (previousSibling)
if (previousSiblingProtector)
currentRenderer = previousSiblingOrParent(currentRenderer);
else
currentRenderer = previousInPreOrder(currentRenderer);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment