Diff
Modified: trunk/LayoutTests/ChangeLog (168447 => 168448)
--- trunk/LayoutTests/ChangeLog 2014-05-07 22:42:46 UTC (rev 168447)
+++ trunk/LayoutTests/ChangeLog 2014-05-07 22:48:31 UTC (rev 168448)
@@ -1,3 +1,15 @@
+2014-05-07 Radu Stavila <stav...@adobe.com>
+
+ Use after free in WebCore::RenderObject::nextSibling / WebCore::RenderBoxModelObject::moveChildrenTo
+ https://bugs.webkit.org/show_bug.cgi?id=132625
+
+ Reviewed by David Hyatt.
+
+ Added test for special case which might cause use after free.
+
+ * fast/multicol/newmulticol/first-letter-create-expected.html: Added.
+ * fast/multicol/newmulticol/first-letter-create.html: Added.
+
2014-05-07 Chris Fleizach <cfleiz...@apple.com>
AX: aria-expanded changes are not communicated to clients
Added: trunk/LayoutTests/fast/multicol/newmulticol/first-letter-create-expected.html (0 => 168448)
--- trunk/LayoutTests/fast/multicol/newmulticol/first-letter-create-expected.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/newmulticol/first-letter-create-expected.html 2014-05-07 22:48:31 UTC (rev 168448)
@@ -0,0 +1,36 @@
+<!-- This test passes if it doesn't crash - https://bugs.webkit.org/show_bug.cgi?id=132625 -->
+
+<style>
+ .f::first-letter { color: green }
+</style>
+
+<script>
+ window._onload_ = function()
+ {
+ var lst = document.getElementsByClassName("d");
+ for (var i = 0; i < lst.length; ++i)
+ lst[i].insertBefore(document.createTextNode("Text"), lst[i].firstChild);
+ }
+</script>
+
+<table>
+ <tr>
+ <td class="f d">More Text</td>
+ </tr>
+</table>
+
+<script>
+ var head = document.getElementsByTagName("head")[0];
+ var style = document.createElement("style");
+ style.innerHTML="* { \n\
+ -webkit-animation-name: name5; \n\
+ -webkit-animation-duration: 4s; \n\
+ } \n\
+ @-webkit-keyframes name5 { \n\
+ from { \n\
+ } \n\
+ to { \n\
+ -webkit-columns: 1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111; \n\
+ ";
+ head.appendChild(style);
+</script>
\ No newline at end of file
Added: trunk/LayoutTests/fast/multicol/newmulticol/first-letter-create.html (0 => 168448)
--- trunk/LayoutTests/fast/multicol/newmulticol/first-letter-create.html (rev 0)
+++ trunk/LayoutTests/fast/multicol/newmulticol/first-letter-create.html 2014-05-07 22:48:31 UTC (rev 168448)
@@ -0,0 +1,36 @@
+<!-- This test passes if it doesn't crash - https://bugs.webkit.org/show_bug.cgi?id=132625 -->
+
+<style>
+ .f::first-letter { color: green }
+</style>
+
+<script>
+ window._onload_ = function()
+ {
+ var lst = document.getElementsByClassName("d");
+ for (var i = 0; i < lst.length; ++i)
+ lst[i].insertBefore(document.createTextNode("Text"), lst[i].firstChild);
+ }
+</script>
+
+<table class="test">
+ <tr>
+ <td class="f d">More Text</td>
+ </tr>
+</table>
+
+<script>
+ var head = document.getElementsByTagName("head")[0];
+ var style = document.createElement("style");
+ style.innerHTML="* { \n\
+ -webkit-animation-name: name5; \n\
+ -webkit-animation-duration: 4s; \n\
+ } \n\
+ @-webkit-keyframes name5 { \n\
+ from { \n\
+ } \n\
+ to { \n\
+ -webkit-columns: 1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111; \n\
+ ";
+ head.appendChild(style);
+</script>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (168447 => 168448)
--- trunk/Source/WebCore/ChangeLog 2014-05-07 22:42:46 UTC (rev 168447)
+++ trunk/Source/WebCore/ChangeLog 2014-05-07 22:48:31 UTC (rev 168448)
@@ -1,3 +1,21 @@
+2014-05-07 Radu Stavila <stav...@adobe.com>
+
+ Use after free in WebCore::RenderObject::nextSibling / WebCore::RenderBoxModelObject::moveChildrenTo
+ https://bugs.webkit.org/show_bug.cgi?id=132625
+
+ Reviewed by David Hyatt.
+
+ Fixed problem with dynamically inserting first letter elements.
+
+ Test: fast/multicol/newmulticol/first-letter-create.html
+
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::getFirstLetter):
+ (WebCore::RenderBlock::updateFirstLetter):
+ * rendering/RenderBlock.h:
+ * rendering/RenderBoxModelObject.cpp:
+ (WebCore::RenderBoxModelObject::moveChildrenTo):
+
2014-05-07 Enrica Casucci <enr...@apple.com>
WK2: Programatic scroll requests during scroll or zoom animation to reveal focused element are ignored.
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (168447 => 168448)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2014-05-07 22:42:46 UTC (rev 168447)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2014-05-07 22:48:31 UTC (rev 168448)
@@ -3555,63 +3555,84 @@
currentTextChild->destroy();
}
}
+
+void RenderBlock::getFirstLetter(RenderObject*& firstLetter, RenderElement*& firstLetterContainer, RenderObject* skipObject)
+{
+ firstLetter = nullptr;
+ firstLetterContainer = nullptr;
-void RenderBlock::updateFirstLetter()
-{
if (!document().styleSheetCollection().usesFirstLetterRules())
return;
+
// Don't recur
if (style().styleType() == FIRST_LETTER)
return;
-
+
// FIXME: We need to destroy the first-letter object if it is no longer the first child. Need to find
// an efficient way to check for that situation though before implementing anything.
- RenderElement* firstLetterBlock = findFirstLetterBlock(this);
- if (!firstLetterBlock)
+ firstLetterContainer = findFirstLetterBlock(this);
+ if (!firstLetterContainer)
return;
-
+
// Drill into inlines looking for our first text descendant.
- RenderObject* descendant = firstLetterBlock->firstChild();
- while (descendant) {
- if (descendant->isText())
+ firstLetter = firstLetterContainer->firstChild();
+ while (firstLetter) {
+ if (firstLetter->isText()) {
+ if (firstLetter == skipObject) {
+ firstLetter = firstLetter->nextSibling();
+ continue;
+ }
+
break;
- RenderElement& current = toRenderElement(*descendant);
+ }
+
+ RenderElement& current = toRenderElement(*firstLetter);
if (current.isListMarker())
- descendant = current.nextSibling();
+ firstLetter = current.nextSibling();
else if (current.isFloatingOrOutOfFlowPositioned()) {
if (current.style().styleType() == FIRST_LETTER) {
- descendant = current.firstChild();
+ firstLetter = current.firstChild();
break;
}
- descendant = current.nextSibling();
+ firstLetter = current.nextSibling();
} else if (current.isReplaced() || current.isRenderButton() || current.isMenuList())
break;
else if (current.style().hasPseudoStyle(FIRST_LETTER) && current.canHaveGeneratedChildren()) {
// We found a lower-level node with first-letter, which supersedes the higher-level style
- firstLetterBlock = ¤t;
- descendant = current.firstChild();
+ firstLetterContainer = ¤t;
+ firstLetter = current.firstChild();
} else
- descendant = current.firstChild();
+ firstLetter = current.firstChild();
}
+
+ if (!firstLetter)
+ firstLetterContainer = nullptr;
+}
- if (!descendant)
+void RenderBlock::updateFirstLetter()
+{
+ RenderObject* firstLetterObj;
+ RenderElement* firstLetterContainer;
+ getFirstLetter(firstLetterObj, firstLetterContainer);
+
+ if (!firstLetterObj)
return;
// If the child already has style, then it has already been created, so we just want
// to update it.
- if (descendant->parent()->style().styleType() == FIRST_LETTER) {
- updateFirstLetterStyle(firstLetterBlock, descendant);
+ if (firstLetterObj->parent()->style().styleType() == FIRST_LETTER) {
+ updateFirstLetterStyle(firstLetterContainer, firstLetterObj);
return;
}
- if (!descendant->isText())
+ if (!firstLetterObj->isText())
return;
// Our layout state is not valid for the repaints we are going to trigger by
// adding and removing children of firstLetterContainer.
LayoutStateDisabler layoutStateDisabler(&view());
- createFirstLetterRenderer(firstLetterBlock, toRenderText(descendant));
+ createFirstLetterRenderer(firstLetterContainer, toRenderText(firstLetterObj));
}
LayoutUnit RenderBlock::paginationStrut() const
Modified: trunk/Source/WebCore/rendering/RenderBlock.h (168447 => 168448)
--- trunk/Source/WebCore/rendering/RenderBlock.h 2014-05-07 22:42:46 UTC (rev 168447)
+++ trunk/Source/WebCore/rendering/RenderBlock.h 2014-05-07 22:48:31 UTC (rev 168448)
@@ -260,6 +260,7 @@
LayoutUnit collapsedMarginAfterForChild(const RenderBox& child) const;
virtual void updateFirstLetter();
+ void getFirstLetter(RenderObject*& firstLetter, RenderElement*& firstLetterContainer, RenderObject* skipObject = nullptr);
virtual void scrollbarsChanged(bool /*horizontalScrollbarChanged*/, bool /*verticalScrollbarChanged*/) { }
Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (168447 => 168448)
--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2014-05-07 22:42:46 UTC (rev 168447)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2014-05-07 22:48:31 UTC (rev 168448)
@@ -48,6 +48,8 @@
#include "RenderNamedFlowThread.h"
#include "RenderRegion.h"
#include "RenderTable.h"
+#include "RenderText.h"
+#include "RenderTextFragment.h"
#include "RenderView.h"
#include "ScrollingConstraints.h"
#include "Settings.h"
@@ -2693,6 +2695,22 @@
for (RenderObject* child = startChild; child && child != endChild; ) {
// Save our next sibling as moveChildTo will clear it.
RenderObject* nextSibling = child->nextSibling();
+
+ // Check to make sure we're not saving the firstLetter as the nextSibling.
+ // When the |child| object will be moved, its firstLetter will be recreated,
+ // so saving it now in nextSibling would let us with a destroyed object.
+ if (child->isText() && toRenderText(child)->isTextFragment() && nextSibling && nextSibling->isText()) {
+ RenderObject* firstLetterObj = nullptr;
+ if (RenderBlock* block = toRenderTextFragment(child)->blockForAccompanyingFirstLetter()) {
+ RenderElement* firstLetterContainer = nullptr;
+ block->getFirstLetter(firstLetterObj, firstLetterContainer, child);
+ }
+
+ // This is the first letter, skip it.
+ if (firstLetterObj == nextSibling)
+ nextSibling = nextSibling->nextSibling();
+ }
+
moveChildTo(toBoxModelObject, child, beforeChild, fullRemoveInsert);
child = nextSibling;
}
Modified: trunk/Source/WebCore/rendering/RenderTextFragment.h (168447 => 168448)
--- trunk/Source/WebCore/rendering/RenderTextFragment.h 2014-05-07 22:42:46 UTC (rev 168447)
+++ trunk/Source/WebCore/rendering/RenderTextFragment.h 2014-05-07 22:48:31 UTC (rev 168448)
@@ -48,6 +48,8 @@
RenderBoxModelObject* firstLetter() const { return m_firstLetter; }
void setFirstLetter(RenderBoxModelObject& firstLetter) { m_firstLetter = &firstLetter; }
+
+ RenderBlock* blockForAccompanyingFirstLetter();
StringImpl* contentString() const { return m_contentString.impl(); }
@@ -61,7 +63,6 @@
virtual void willBeDestroyed() override;
virtual UChar previousCharacter() const override;
- RenderBlock* blockForAccompanyingFirstLetter();
unsigned m_start;
unsigned m_end;
