Title: [159481] trunk
- Revision
- 159481
- Author
- za...@apple.com
- Date
- 2013-11-18 22:01:45 -0800 (Mon, 18 Nov 2013)
Log Message
use after free in WebCore::DocumentOrderedMap::remove / WebCore::TreeScope::removeElementById
https://bugs.webkit.org/show_bug.cgi?id=121324
Reviewed by Ryosuke Niwa.
Update the document ordered map for an image element before dispatching load or error events
when it's inserted into a document.
Source/WebCore:
Test: fast/dom/modify-node-and-while-in-the-callback-too-crash.html
* dom/DocumentOrderedMap.cpp: defensive fix to avoid use after free issues.
(WebCore::DocumentOrderedMap::remove):
* html/HTMLImageElement.cpp:
(WebCore::HTMLImageElement::insertedInto):
* loader/ImageLoader.cpp:
(WebCore::ImageLoader::updateFromElement): setting m_failedLoadURL makes
repeated updateFromElement calls return early.
LayoutTests:
* fast/dom/modify-node-and-while-in-the-callback-too-crash-expected.txt: Added.
* fast/dom/modify-node-and-while-in-the-callback-too-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (159480 => 159481)
--- trunk/LayoutTests/ChangeLog 2013-11-19 05:14:36 UTC (rev 159480)
+++ trunk/LayoutTests/ChangeLog 2013-11-19 06:01:45 UTC (rev 159481)
@@ -1,3 +1,16 @@
+2013-11-18 Zalan Bujtas <za...@apple.com>
+
+ use after free in WebCore::DocumentOrderedMap::remove / WebCore::TreeScope::removeElementById
+ https://bugs.webkit.org/show_bug.cgi?id=121324
+
+ Reviewed by Ryosuke Niwa.
+
+ Update the document ordered map for an image element before dispatching load or error events
+ when it's inserted into a document.
+
+ * fast/dom/modify-node-and-while-in-the-callback-too-crash-expected.txt: Added.
+ * fast/dom/modify-node-and-while-in-the-callback-too-crash.html: Added.
+
2013-11-18 Sun-woo Nam <sunny....@samsung.com>
[EFL] Layout tests need to be rebaselined.
Added: trunk/LayoutTests/fast/dom/modify-node-and-while-in-the-callback-too-crash-expected.txt (0 => 159481)
--- trunk/LayoutTests/fast/dom/modify-node-and-while-in-the-callback-too-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/modify-node-and-while-in-the-callback-too-crash-expected.txt 2013-11-19 06:01:45 UTC (rev 159481)
@@ -0,0 +1,3 @@
+This tests that making changes on a node that triggers a callback where we make changes again on the same node does not result in an assert/crash. Test passes if no crash is observed.
+
+
Added: trunk/LayoutTests/fast/dom/modify-node-and-while-in-the-callback-too-crash.html (0 => 159481)
--- trunk/LayoutTests/fast/dom/modify-node-and-while-in-the-callback-too-crash.html (rev 0)
+++ trunk/LayoutTests/fast/dom/modify-node-and-while-in-the-callback-too-crash.html 2013-11-19 06:01:45 UTC (rev 159481)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<body>
+ <p>This tests that making changes on a node that triggers a callback where we make changes
+ again on the same node does not result in an assert/crash.
+ Test passes if no crash is observed.</p>
+ <img id="error-image" src=""
+ <div id="container"></div>
+
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ var img = document.getElementById('error-image');
+ var container = document.getElementById('container');
+
+ img._onerror_ = function() {
+ container.parentNode.removeChild(container);
+ }
+
+ container.appendChild(img);
+ </script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (159480 => 159481)
--- trunk/Source/WebCore/ChangeLog 2013-11-19 05:14:36 UTC (rev 159480)
+++ trunk/Source/WebCore/ChangeLog 2013-11-19 06:01:45 UTC (rev 159481)
@@ -1,3 +1,23 @@
+2013-11-18 Zalan Bujtas <za...@apple.com>
+
+ use after free in WebCore::DocumentOrderedMap::remove / WebCore::TreeScope::removeElementById
+ https://bugs.webkit.org/show_bug.cgi?id=121324
+
+ Reviewed by Ryosuke Niwa.
+
+ Update the document ordered map for an image element before dispatching load or error events
+ when it's inserted into a document.
+
+ Test: fast/dom/modify-node-and-while-in-the-callback-too-crash.html
+
+ * dom/DocumentOrderedMap.cpp: defensive fix to avoid use after free issues.
+ (WebCore::DocumentOrderedMap::remove):
+ * html/HTMLImageElement.cpp:
+ (WebCore::HTMLImageElement::insertedInto):
+ * loader/ImageLoader.cpp:
+ (WebCore::ImageLoader::updateFromElement): setting m_failedLoadURL makes
+ repeated updateFromElement calls return early.
+
2013-11-18 Benjamin Poulain <bpoul...@apple.com>
Upstream iOS's ResourceHandle implementation
Modified: trunk/Source/WebCore/dom/DocumentOrderedMap.cpp (159480 => 159481)
--- trunk/Source/WebCore/dom/DocumentOrderedMap.cpp 2013-11-19 05:14:36 UTC (rev 159480)
+++ trunk/Source/WebCore/dom/DocumentOrderedMap.cpp 2013-11-19 06:01:45 UTC (rev 159481)
@@ -105,6 +105,9 @@
m_map.checkConsistency();
auto it = m_map.find(&key);
ASSERT(it != m_map.end());
+ if (it == m_map.end())
+ return;
+
MapEntry& entry = it->value;
ASSERT(entry.count);
Modified: trunk/Source/WebCore/html/HTMLImageElement.cpp (159480 => 159481)
--- trunk/Source/WebCore/html/HTMLImageElement.cpp 2013-11-19 05:14:36 UTC (rev 159480)
+++ trunk/Source/WebCore/html/HTMLImageElement.cpp 2013-11-19 06:01:45 UTC (rev 159481)
@@ -221,12 +221,16 @@
if (insertionPoint.inDocument() && !m_lowercasedUsemap.isNull())
document().addImageElementByLowercasedUsemap(*m_lowercasedUsemap.impl(), *this);
+ // Insert needs to complete first, before we start updating the loader. Loader dispatches events which could result
+ // in callbacks back to this node.
+ Node::InsertionNotificationRequest insertNotificationRequest = HTMLElement::insertedInto(insertionPoint);
+
// If we have been inserted from a renderer-less document,
// our loader may have not fetched the image, so do it now.
if (insertionPoint.inDocument() && !m_imageLoader.image())
m_imageLoader.updateFromElement();
- return HTMLElement::insertedInto(insertionPoint);
+ return insertNotificationRequest;
}
void HTMLImageElement::removedFrom(ContainerNode& insertionPoint)
Modified: trunk/Source/WebCore/loader/ImageLoader.cpp (159480 => 159481)
--- trunk/Source/WebCore/loader/ImageLoader.cpp 2013-11-19 05:14:36 UTC (rev 159480)
+++ trunk/Source/WebCore/loader/ImageLoader.cpp 2013-11-19 06:01:45 UTC (rev 159481)
@@ -212,8 +212,9 @@
clearFailedLoadURL();
} else if (!attr.isNull()) {
// Fire an error event if the url is empty.
- // FIXME: Should we fire this event asynchronoulsy via errorEventSender()?
- m_element->dispatchEvent(Event::create(eventNames().errorEvent, false, false));
+ m_failedLoadURL = attr;
+ m_hasPendingErrorEvent = true;
+ errorEventSender().dispatchEventSoon(this);
}
CachedImage* oldImage = m_image.get();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
