Diff
Copied: branches/chromium/1025/LayoutTests/media/video-beforeload-remove-source-expected.txt (from rev 111895, trunk/LayoutTests/media/video-beforeload-remove-source-expected.txt) (0 => 112581)
--- branches/chromium/1025/LayoutTests/media/video-beforeload-remove-source-expected.txt (rev 0)
+++ branches/chromium/1025/LayoutTests/media/video-beforeload-remove-source-expected.txt 2012-03-29 21:51:01 UTC (rev 112581)
@@ -0,0 +1 @@
+Test passes if it does not crash.
Copied: branches/chromium/1025/LayoutTests/media/video-beforeload-remove-source.html (from rev 111895, trunk/LayoutTests/media/video-beforeload-remove-source.html) (0 => 112581)
--- branches/chromium/1025/LayoutTests/media/video-beforeload-remove-source.html (rev 0)
+++ branches/chromium/1025/LayoutTests/media/video-beforeload-remove-source.html 2012-03-29 21:51:01 UTC (rev 112581)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <script src=""
+ </head>
+ <body>
+ <video controls autoplay>
+ <b id="start"></b>
+ <source src="" type="video/mp4">
+ <source src="" type="video/ogg">
+ <b id="end"></b>
+ </video>
+ <div>Test passes if it does not crash.</div>
+ </body>
+ <script>
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ }
+
+ function removeNodes(start, end) {
+ var range = document.createRange();
+ range.setStart(start, 0);
+ range.setEnd(end, 0);
+ range.deleteContents();
+ }
+
+ function beforeLoadFunc()
+ {
+ document.removeEventListener("beforeload", beforeLoadFunc, true);
+ var start = document.getElementById("start");
+ var end = document.getElementById("end");
+ removeNodes(start, end);
+
+ gc();
+ if (window.layoutTestController)
+ setTimeout("layoutTestController.notifyDone()", 0);
+ }
+
+ document.addEventListener("beforeload", beforeLoadFunc, true);
+ </script>
+</html>
Modified: branches/chromium/1025/Source/WebCore/dom/ContainerNode.cpp (112580 => 112581)
--- branches/chromium/1025/Source/WebCore/dom/ContainerNode.cpp 2012-03-29 21:48:38 UTC (rev 112580)
+++ branches/chromium/1025/Source/WebCore/dom/ContainerNode.cpp 2012-03-29 21:51:01 UTC (rev 112581)
@@ -56,25 +56,18 @@
typedef pair<NodeCallback, CallbackParameters> CallbackInfo;
typedef Vector<CallbackInfo> NodeCallbackQueue;
-typedef Vector<RefPtr<Node>, 1> NodeVector;
static NodeCallbackQueue* s_postAttachCallbackQueue;
static size_t s_attachDepth;
static bool s_shouldReEnableMemoryCacheCallsAfterAttach;
-static inline void collectNodes(Node* node, NodeVector& nodes)
-{
- for (Node* child = node->firstChild(); child; child = child->nextSibling())
- nodes.append(child);
-}
-
static void collectTargetNodes(Node* node, NodeVector& nodes)
{
if (node->nodeType() != Node::DOCUMENT_FRAGMENT_NODE) {
nodes.append(node);
return;
}
- collectNodes(node, nodes);
+ getChildNodes(node, nodes);
}
void ContainerNode::removeAllChildren()
@@ -85,7 +78,7 @@
void ContainerNode::takeAllChildrenFrom(ContainerNode* oldParent)
{
NodeVector children;
- collectNodes(oldParent, children);
+ getChildNodes(oldParent, children);
oldParent->removeAllChildren();
for (unsigned i = 0; i < children.size(); ++i) {
@@ -387,7 +380,7 @@
RefPtr<Node> protect(this);
NodeVector children;
- collectNodes(this, children);
+ getChildNodes(this, children);
for (size_t i = 0; i < children.size(); ++i) {
if (children[i]->parentNode() != this) // Check for child being removed from subtree while removing.
continue;
@@ -414,7 +407,7 @@
container->document()->incDOMTreeVersion();
NodeVector children;
- collectNodes(container, children);
+ getChildNodes(container, children);
#if ENABLE(MUTATION_OBSERVERS)
ChildListMutationScope mutation(container);
@@ -814,7 +807,7 @@
insertedIntoTree(false);
NodeVector children;
- collectNodes(this, children);
+ getChildNodes(this, children);
for (size_t i = 0; i < children.size(); ++i) {
// If we have been removed from the document during this loop, then
// we don't want to tell the rest of our children that they've been
@@ -836,7 +829,7 @@
removedFromTree(false);
NodeVector children;
- collectNodes(this, children);
+ getChildNodes(this, children);
for (size_t i = 0; i < children.size(); ++i) {
// If we have been added to the document during this loop, then we
// don't want to tell the rest of our children that they've been
Modified: branches/chromium/1025/Source/WebCore/dom/ContainerNode.h (112580 => 112581)
--- branches/chromium/1025/Source/WebCore/dom/ContainerNode.h 2012-03-29 21:48:38 UTC (rev 112580)
+++ branches/chromium/1025/Source/WebCore/dom/ContainerNode.h 2012-03-29 21:51:01 UTC (rev 112581)
@@ -166,6 +166,15 @@
return toContainerNode(this)->lastChild();
}
+typedef Vector<RefPtr<Node>, 11> NodeVector;
+
+inline void getChildNodes(Node* node, NodeVector& nodes)
+{
+ ASSERT(!nodes.size());
+ for (Node* child = node->firstChild(); child; child = child->nextSibling())
+ nodes.append(child);
+}
+
} // namespace WebCore
#endif // ContainerNode_h
Modified: branches/chromium/1025/Source/WebCore/editing/ReplaceSelectionCommand.cpp (112580 => 112581)
--- branches/chromium/1025/Source/WebCore/editing/ReplaceSelectionCommand.cpp 2012-03-29 21:48:38 UTC (rev 112580)
+++ branches/chromium/1025/Source/WebCore/editing/ReplaceSelectionCommand.cpp 2012-03-29 21:51:01 UTC (rev 112581)
@@ -60,8 +60,6 @@
namespace WebCore {
-typedef Vector<RefPtr<Node> > NodeVector;
-
using namespace HTMLNames;
enum EFragmentType { EmptyFragment, SingleTextNodeFragment, TreeFragment };
Modified: branches/chromium/1025/Source/WebCore/html/HTMLMediaElement.cpp (112580 => 112581)
--- branches/chromium/1025/Source/WebCore/html/HTMLMediaElement.cpp 2012-03-29 21:48:38 UTC (rev 112580)
+++ branches/chromium/1025/Source/WebCore/html/HTMLMediaElement.cpp 2012-03-29 21:51:01 UTC (rev 112581)
@@ -190,8 +190,6 @@
, m_lastTimeUpdateEventWallTime(0)
, m_lastTimeUpdateEventMovieTime(numeric_limits<float>::max())
, m_loadState(WaitingForSource)
- , m_currentSourceNode(0)
- , m_nextChildNodeToConsider(0)
#if ENABLE(PLUGIN_PROXY_FOR_VIDEO)
, m_proxyWidget(0)
#endif
@@ -548,6 +546,8 @@
void HTMLMediaElement::loadTimerFired(Timer<HTMLMediaElement>*)
{
+ RefPtr<HTMLMediaElement> protect(this); // loadNextSourceChild may fire 'beforeload', which can make arbitrary DOM mutations.
+
if (m_pendingLoadFlags & MediaResource) {
if (m_loadState == LoadingFromSourceElement)
loadNextSourceChild();
@@ -604,6 +604,8 @@
void HTMLMediaElement::load(ExceptionCode& ec)
{
+ RefPtr<HTMLMediaElement> protect(this); // loadInternal may result in a 'beforeload' event, which can make arbitrary DOM mutations.
+
LOG(Media, "HTMLMediaElement::load()");
if (userGestureRequiredForLoad() && !ScriptController::processingUserGesture())
@@ -757,7 +759,7 @@
// source element child in tree order.
if (node) {
mode = children;
- m_nextChildNodeToConsider = 0;
+ m_nextChildNodeToConsider = node;
m_currentSourceNode = 0;
} else {
// Otherwise the media element has neither a src attribute nor a source element
@@ -2451,8 +2453,8 @@
{
// Stash the current <source> node and next nodes so we can restore them after checking
// to see there is another potential.
- HTMLSourceElement* currentSourceNode = m_currentSourceNode;
- Node* nextNode = m_nextChildNodeToConsider;
+ RefPtr<HTMLSourceElement> currentSourceNode = m_currentSourceNode;
+ RefPtr<Node> nextNode = m_nextChildNodeToConsider;
KURL nextURL = selectNextSourceChild(0, DoNothing);
@@ -2471,7 +2473,7 @@
LOG(Media, "HTMLMediaElement::selectNextSourceChild");
#endif
- if (m_nextChildNodeToConsider == sourceChildEndOfListValue()) {
+ if (!m_nextChildNodeToConsider) {
#if !LOG_DISABLED
if (shouldLog)
LOG(Media, "HTMLMediaElement::selectNextSourceChild -> 0x0000, \"\"");
@@ -2482,16 +2484,24 @@
KURL mediaURL;
Node* node;
HTMLSourceElement* source = 0;
+ String type;
bool lookingForStartNode = m_nextChildNodeToConsider;
- bool canUse = false;
+ bool canUseSourceElement = false;
+ bool okToLoadSourceURL;
- for (node = firstChild(); !canUse && node; node = node->nextSibling()) {
+ NodeVector potentialSourceNodes;
+ getChildNodes(this, potentialSourceNodes);
+
+ for (unsigned i = 0; !canUseSourceElement && i < potentialSourceNodes.size(); ++i) {
+ node = potentialSourceNodes[i].get();
if (lookingForStartNode && m_nextChildNodeToConsider != node)
continue;
lookingForStartNode = false;
-
+
if (!node->hasTagName(sourceTag))
continue;
+ if (node->parentNode() != this)
+ continue;
source = static_cast<HTMLSourceElement*>(node);
@@ -2525,34 +2535,41 @@
}
// Is it safe to load this url?
- if (!isSafeToLoadURL(mediaURL, actionIfInvalid) || !dispatchBeforeLoadEvent(mediaURL.string()))
+ okToLoadSourceURL = isSafeToLoadURL(mediaURL, actionIfInvalid) && dispatchBeforeLoadEvent(mediaURL.string());
+
+ // A 'beforeload' event handler can mutate the DOM, so check to see if the source element is still a child node.
+ if (node->parentNode() != this) {
+ LOG(Media, "HTMLMediaElement::selectNextSourceChild : 'beforeload' removed current element");
+ source = 0;
goto check_again;
+ }
+ if (!okToLoadSourceURL)
+ goto check_again;
+
// Making it this far means the <source> looks reasonable.
- canUse = true;
+ canUseSourceElement = true;
check_again:
- if (!canUse && actionIfInvalid == Complain)
+ if (!canUseSourceElement && actionIfInvalid == Complain && source)
source->scheduleErrorEvent();
}
- if (canUse) {
+ if (canUseSourceElement) {
if (contentType)
*contentType = ContentType(source->type());
m_currentSourceNode = source;
m_nextChildNodeToConsider = source->nextSibling();
- if (!m_nextChildNodeToConsider)
- m_nextChildNodeToConsider = sourceChildEndOfListValue();
} else {
m_currentSourceNode = 0;
- m_nextChildNodeToConsider = sourceChildEndOfListValue();
+ m_nextChildNodeToConsider = 0;
}
#if !LOG_DISABLED
if (shouldLog)
- LOG(Media, "HTMLMediaElement::selectNextSourceChild -> %p, %s", m_currentSourceNode, canUse ? urlForLogging(mediaURL).utf8().data() : "");
+ LOG(Media, "HTMLMediaElement::selectNextSourceChild -> %p, %s", m_currentSourceNode.get(), canUseSourceElement ? urlForLogging(mediaURL).utf8().data() : "");
#endif
- return canUse ? mediaURL : KURL();
+ return canUseSourceElement ? mediaURL : KURL();
}
void HTMLMediaElement::sourceWasAdded(HTMLSourceElement* source)
@@ -2584,20 +2601,20 @@
return;
}
- if (m_nextChildNodeToConsider != sourceChildEndOfListValue())
+ if (m_nextChildNodeToConsider)
return;
// 4.8.9.5, resource selection algorithm, source elements section:
- // 20 - Wait until the node after pointer is a node other than the end of the list. (This step might wait forever.)
- // 21 - Asynchronously await a stable state...
- // 22 - Set the element's delaying-the-load-event flag back to true (this delays the load event again, in case
+ // 21. Wait until the node after pointer is a node other than the end of the list. (This step might wait forever.)
+ // 22. Asynchronously await a stable state...
+ // 23. Set the element's delaying-the-load-event flag back to true (this delays the load event again, in case
// it hasn't been fired yet).
setShouldDelayLoadEvent(true);
- // 23 - Set the networkState back to NETWORK_LOADING.
+ // 24. Set the networkState back to NETWORK_LOADING.
m_networkState = NETWORK_LOADING;
- // 24 - Jump back to the find next candidate step above.
+ // 25. Jump back to the find next candidate step above.
m_nextChildNodeToConsider = source;
scheduleNextSourceChild();
}
@@ -2619,8 +2636,8 @@
if (source == m_nextChildNodeToConsider) {
m_nextChildNodeToConsider = m_nextChildNodeToConsider->nextSibling();
if (!m_nextChildNodeToConsider)
- m_nextChildNodeToConsider = sourceChildEndOfListValue();
- LOG(Media, "HTMLMediaElement::sourceRemoved - m_nextChildNodeToConsider set to %p", m_nextChildNodeToConsider);
+ m_nextChildNodeToConsider = 0;
+ LOG(Media, "HTMLMediaElement::sourceRemoved - m_nextChildNodeToConsider set to %p", m_nextChildNodeToConsider.get());
} else if (source == m_currentSourceNode) {
// Clear the current source node pointer, but don't change the movie as the spec says:
// 4.8.8 - Dynamically modifying a source element and its attribute when the element is already
@@ -3212,6 +3229,8 @@
void HTMLMediaElement::getPluginProxyParams(KURL& url, Vector<String>& names, Vector<String>& values)
{
+ RefPtr<HTMLMediaElement> protect(this); // selectNextSourceChild may fire 'beforeload', which can make arbitrary DOM mutations.
+
Frame* frame = document()->frame();
if (isVideo()) {
Modified: branches/chromium/1025/Source/WebCore/html/HTMLMediaElement.h (112580 => 112581)
--- branches/chromium/1025/Source/WebCore/html/HTMLMediaElement.h 2012-03-29 21:48:38 UTC (rev 112580)
+++ branches/chromium/1025/Source/WebCore/html/HTMLMediaElement.h 2012-03-29 21:51:01 UTC (rev 112581)
@@ -502,9 +502,8 @@
// Loading state.
enum LoadState { WaitingForSource, LoadingFromSrcAttr, LoadingFromSourceElement };
LoadState m_loadState;
- HTMLSourceElement* m_currentSourceNode;
- Node* m_nextChildNodeToConsider;
- Node* sourceChildEndOfListValue() { return static_cast<Node*>(this); }
+ RefPtr<HTMLSourceElement> m_currentSourceNode;
+ RefPtr<Node> m_nextChildNodeToConsider;
OwnPtr<MediaPlayer> m_player;
#if ENABLE(PLUGIN_PROXY_FOR_VIDEO)
