Crash due to first-letter block processing
https://bugs.webkit.org/show_bug.cgi?id=74009
Source/WebCore:
Fixing the way updateFirstLetter() finds the remaining text fragment
for a given first-letter. Previously this was unreliable in some
circumstances.
This patch provides a reliable mechanism to identify the remaining
text by storing first-letter to remaining text associations in a
hash map, managed by methods in RenderBoxModelObject.
Patch by Ken Buchanan <kenrb@chromium.org> on 2012-01-05
Reviewed by David Hyatt.
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::updateFirstLetter)
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::willBeDestroyed):
(WebCore::RenderBoxModelObject::setFirstLetterRemainingText): Added
(WebCore::RenderBoxModelObject::firstLetterRemainingText): Added
* rendering/RenderBoxModelObject.h:
(WebCore::RenderBoxModelObject::setFirstLetterRemainingText): Added
(WebCore::RenderBoxModelObject::firstLetterRemainingText): Added
LayoutTests:
Test for crashing condition in 74009.
Patch by Ken Buchanan <kenrb@chromium.org> on 2012-01-05
Reviewed by David Hyatt.
* fast/css/first-letter-inline-flow-split-table-crash-expected.txt: Added
* fast/css/first-letter-inline-flow-split-table-crash.html: Added
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@104123 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
old mode 100644
new mode 100755
index 0e6bdbd..4534b85
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2012-01-05 Ken Buchanan <kenrb@chromium.org>
+
+ Crash due to first-letter block processing
+ https://bugs.webkit.org/show_bug.cgi?id=74009
+
+ Test for crashing condition in 74009.
+
+ Reviewed by David Hyatt.
+
+ * fast/css/first-letter-inline-flow-split-table-crash-expected.txt: Added
+ * fast/css/first-letter-inline-flow-split-table-crash.html: Added
+
2012-01-05 Mihnea Ovidenie <mihnea@adobe.com>
Crash in RenderFlowThread::getRegionRangeForBox.
diff --git a/LayoutTests/fast/css/first-letter-inline-flow-split-table-crash-expected.txt b/LayoutTests/fast/css/first-letter-inline-flow-split-table-crash-expected.txt
new file mode 100755
index 0000000..ace3cba
--- /dev/null
+++ b/LayoutTests/fast/css/first-letter-inline-flow-split-table-crash-expected.txt
@@ -0,0 +1 @@
+PASS, if no exception or crash in debug
diff --git a/LayoutTests/fast/css/first-letter-inline-flow-split-table-crash.html b/LayoutTests/fast/css/first-letter-inline-flow-split-table-crash.html
new file mode 100755
index 0000000..fa07640
--- /dev/null
+++ b/LayoutTests/fast/css/first-letter-inline-flow-split-table-crash.html
@@ -0,0 +1,47 @@
+<style>
+.noFloat:empty { float: none; }
+.theadStyle:nth-last-child(odd) { display: table-header-group; float: right; }
+.pSpanStyle { overflow: hidden; -webkit-appearance: button; }
+.pSpanStyle:first-letter { text-align: -webkit-left; content: counter(section); }
+</style>
+<script>
+var parentSpan = document.createElement('span');
+var childSpan = document.createElement('span');
+var thead = document.createElement('thead');
+var textNode = document.createTextNode('abc');
+
+function removeTextNode() {
+ childSpan.removeChild(textNode);
+ delete textNode;
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+}
+
+function changeClass() {
+ thead.setAttribute('class', 'noFloat');
+ setTimeout("removeTable()", 10);
+}
+
+function removeTable() {
+ childSpan.removeChild(thead);
+ setTimeout('removeTextNode();', 10);
+}
+
+function runTest() {
+ parentSpan.setAttribute('class', 'pSpanStyle');
+ document.documentElement.appendChild(parentSpan);
+ childSpan.setAttribute('class', 'noFloat');
+ parentSpan.appendChild(childSpan);
+ thead.setAttribute('class', 'theadStyle');
+ childSpan.appendChild(thead);
+ childSpan.appendChild(textNode);
+ setTimeout('changeClass();', 10);
+
+ if (window.layoutTestController) {
+ layoutTestController.waitUntilDone();
+ layoutTestController.dumpAsText();
+ }
+}
+window.onload = runTest;
+</script>
+PASS, if no exception or crash in debug
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
old mode 100644
new mode 100755
index 2a3e5ff..21d65f6
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2012-01-05 Ken Buchanan <kenrb@chromium.org>
+
+ Crash due to first-letter block processing
+ https://bugs.webkit.org/show_bug.cgi?id=74009
+
+ Fixing the way updateFirstLetter() finds the remaining text fragment
+ for a given first-letter. Previously this was unreliable in some
+ circumstances.
+
+ This patch provides a reliable mechanism to identify the remaining
+ text by storing first-letter to remaining text associations in a
+ hash map, managed by methods in RenderBoxModelObject.
+
+ Reviewed by David Hyatt.
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::updateFirstLetter)
+ * rendering/RenderBoxModelObject.cpp:
+ (WebCore::RenderBoxModelObject::willBeDestroyed):
+ (WebCore::RenderBoxModelObject::setFirstLetterRemainingText): Added
+ (WebCore::RenderBoxModelObject::firstLetterRemainingText): Added
+ * rendering/RenderBoxModelObject.h:
+ (WebCore::RenderBoxModelObject::setFirstLetterRemainingText): Added
+ (WebCore::RenderBoxModelObject::firstLetterRemainingText): Added
+
2012-01-05 Mihnea Ovidenie <mihnea@adobe.com>
Crash in RenderRegion::getRegionRangeForBox.
diff --git a/Source/WebCore/rendering/RenderBlock.cpp b/Source/WebCore/rendering/RenderBlock.cpp
index 8381803..1306ae2 100755
--- a/Source/WebCore/rendering/RenderBlock.cpp
+++ b/Source/WebCore/rendering/RenderBlock.cpp
@@ -5752,38 +5752,14 @@
RenderTextFragment* remainingText = 0;
RenderObject* nextSibling = firstLetter->nextSibling();
- RenderObject* next = nextSibling;
- while (next) {
- if (next->isText() && toRenderText(next)->isTextFragment()) {
- remainingText = toRenderTextFragment(next);
- break;
- }
- next = next->nextSibling();
- }
- if (!remainingText && firstLetterContainer->isAnonymousBlock()) {
- // The remaining text fragment could have been wrapped in a different anonymous block since creation
- RenderObject* nextChild;
- next = firstLetterContainer->nextSibling();
- while (next && !remainingText) {
- if (next->isAnonymousBlock()) {
- nextChild = next->firstChild();
- while (nextChild) {
- if (nextChild->isText() && toRenderText(nextChild)->isTextFragment()
- && (toRenderTextFragment(nextChild)->firstLetter() == firstLetter)) {
- remainingText = toRenderTextFragment(nextChild);
- break;
- }
- nextChild = nextChild->nextSibling();
- }
- } else
- break;
- next = next->nextSibling();
- }
- }
+ RenderObject* remainingTextObject = toRenderBoxModelObject(firstLetter)->firstLetterRemainingText();
+ if (remainingTextObject && remainingTextObject->isText() && toRenderText(remainingTextObject)->isTextFragment())
+ remainingText = toRenderTextFragment(remainingTextObject);
if (remainingText) {
ASSERT(remainingText->isAnonymous() || remainingText->node()->renderer() == remainingText);
// Replace the old renderer with the new one.
remainingText->setFirstLetter(newFirstLetter);
+ toRenderBoxModelObject(newFirstLetter)->setFirstLetterRemainingText(remainingText);
}
firstLetter->destroy();
firstLetter = newFirstLetter;
@@ -5861,6 +5837,7 @@
firstLetterContainer->addChild(remainingText, textObj);
firstLetterContainer->removeChild(textObj);
remainingText->setFirstLetter(firstLetter);
+ toRenderBoxModelObject(firstLetter)->setFirstLetterRemainingText(remainingText);
// construct text fragment for the first letter
RenderTextFragment* letter =
diff --git a/Source/WebCore/rendering/RenderBoxModelObject.cpp b/Source/WebCore/rendering/RenderBoxModelObject.cpp
index 791ed5c..3ab4e21 100644
--- a/Source/WebCore/rendering/RenderBoxModelObject.cpp
+++ b/Source/WebCore/rendering/RenderBoxModelObject.cpp
@@ -66,6 +66,11 @@
typedef HashMap<const RenderBoxModelObject*, RenderBoxModelObject*> ContinuationMap;
static ContinuationMap* continuationMap = 0;
+// This HashMap is similar to the continuation map, but connects first-letter
+// renderers to their remaining text fragments.
+typedef HashMap<const RenderBoxModelObject*, RenderObject*> FirstLetterRemainingTextMap;
+static FirstLetterRemainingTextMap* firstLetterRemainingTextMap = 0;
+
class ImageQualityController {
WTF_MAKE_NONCOPYABLE(ImageQualityController); WTF_MAKE_FAST_ALLOCATED;
public:
@@ -283,6 +288,11 @@
// A continuation of this RenderObject should be destroyed at subclasses.
ASSERT(!continuation());
+ // If this is a first-letter object with a remaining text fragment then the
+ // entry needs to be cleared from the map.
+ if (firstLetterRemainingText())
+ setFirstLetterRemainingText(0);
+
// RenderObject::willBeDestroyed calls back to destroyLayer() for layer destruction
RenderObject::willBeDestroyed();
}
@@ -2712,6 +2722,23 @@
}
}
+RenderObject* RenderBoxModelObject::firstLetterRemainingText() const
+{
+ if (!firstLetterRemainingTextMap)
+ return 0;
+ return firstLetterRemainingTextMap->get(this);
+}
+
+void RenderBoxModelObject::setFirstLetterRemainingText(RenderObject* remainingText)
+{
+ if (remainingText) {
+ if (!firstLetterRemainingTextMap)
+ firstLetterRemainingTextMap = new FirstLetterRemainingTextMap;
+ firstLetterRemainingTextMap->set(this, remainingText);
+ } else if (firstLetterRemainingTextMap)
+ firstLetterRemainingTextMap->remove(this);
+}
+
bool RenderBoxModelObject::shouldAntialiasLines(GraphicsContext* context)
{
// FIXME: We may want to not antialias when scaled by an integral value,
diff --git a/Source/WebCore/rendering/RenderBoxModelObject.h b/Source/WebCore/rendering/RenderBoxModelObject.h
index b07e8d6..bb81bd3 100644
--- a/Source/WebCore/rendering/RenderBoxModelObject.h
+++ b/Source/WebCore/rendering/RenderBoxModelObject.h
@@ -194,6 +194,11 @@
static bool shouldAntialiasLines(GraphicsContext*);
+public:
+ // For RenderBlocks and RenderInlines with m_style->styleType() == FIRST_LETTER, this tracks their remaining text fragments
+ RenderObject* firstLetterRemainingText() const;
+ void setFirstLetterRemainingText(RenderObject*);
+
private:
virtual bool isBoxModelObject() const { return true; }
