https://bugs.webkit.org/show_bug.cgi?id=81163
Reviewed by Alexey Proskuryakov.
Source/WebCore:
Test: media/video-beforeload-remove-source.html
* dom/ContainerNode.cpp: Make NodeVector and collectNodes public, renamed as getChildNodes.
(WebCore::ContainerNode::takeAllChildrenFrom): collectNodes -> getChildNodes.
(WebCore::ContainerNode::willRemove): collectNodes -> getChildNodes.
(WebCore::ContainerNode::willRemoveChildren): collectNodes -> getChildNodes.
(WebCore::ContainerNode::insertedIntoDocument): collectNodes -> getChildNodes.
(WebCore::ContainerNode::removedFromDocument): collectNodes -> getChildNodes.
* dom/ContainerNode.h:
(WebCore::getChildNodes):
* editing/ReplaceSelectionCommand.cpp: Remove unused NodeVector declaration.
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::HTMLMediaElement): m_nextChildNodeToConsider and m_currentSourceNode
are now RefPtrs.
(WebCore::HTMLMediaElement::loadTimerFired): Protect HTMLMediaElement from being deleted during
a DOM modification during an event callback.
(WebCore::HTMLMediaElement::load): Ditto.
(WebCore::HTMLMediaElement::selectMediaResource): Set m_nextChildNodeToConsider to the first
child node, it will be the first node considered.
(WebCore::HTMLMediaElement::havePotentialSourceChild): m_nextChildNodeToConsider and m_currentSourceNode
are now RefPtrs.
(WebCore::HTMLMediaElement::selectNextSourceChild): Collect all child nodes in a vector before
looking for <source> nodes because 'beforeload' event handlers can mutate the DOM. Don't
use a <source> that is no longer a child node after 'beforeload'. Use 0 to represent the end
of the child node list because m_nextChildNodeToConsider is now a RefPtr so using the previous
sentinel, "this", would cause a retain cycle.
(WebCore::HTMLMediaElement::sourceWasAdded): m_nextChildNodeToConsider and m_currentSourceNode
are now RefPtrs.
(WebCore::HTMLMediaElement::sourceWillBeRemoved): Ditto.
(WebCore::HTMLMediaElement::getPluginProxyParams): Protect HTMLMediaElement from being deleted during
a DOM modification during an event callback.
* html/HTMLMediaElement.h:
LayoutTests:
* media/video-beforeload-remove-source-expected.txt: Added.
* media/video-beforeload-remove-source.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@111895
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2012-03-23 Eric Carlson <eric.carlson@apple.com>
+
+ Deal with DOM modifications when evaluating source elements.
+ https://bugs.webkit.org/show_bug.cgi?id=81163
+
+ Reviewed by Alexey Proskuryakov.
+
+ * media/video-beforeload-remove-source-expected.txt: Added.
+ * media/video-beforeload-remove-source.html: Added.
+
2012-03-23 Dan Bernstein <mitz@apple.com>
Added all tests that crashed more than once on the Lion WebKit2 bots between r111865
--- /dev/null
+Test passes if it does not crash.
--- /dev/null
+<!DOCTYPE html>
+<html>
+ <head>
+ <script src=../resources/gc.js></script>
+ </head>
+ <body>
+ <video controls autoplay>
+ <b id="start"></b>
+ <source src="content/test.mp4" type="video/mp4">
+ <source src="content/test.ogv" 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>
+2012-03-23 Eric Carlson <eric.carlson@apple.com>
+
+ Deal with DOM modifications when evaluating source elements.
+ https://bugs.webkit.org/show_bug.cgi?id=81163
+
+ Reviewed by Alexey Proskuryakov.
+
+ Test: media/video-beforeload-remove-source.html
+
+ * dom/ContainerNode.cpp: Make NodeVector and collectNodes public, renamed as getChildNodes.
+ (WebCore::ContainerNode::takeAllChildrenFrom): collectNodes -> getChildNodes.
+ (WebCore::ContainerNode::willRemove): collectNodes -> getChildNodes.
+ (WebCore::ContainerNode::willRemoveChildren): collectNodes -> getChildNodes.
+ (WebCore::ContainerNode::insertedIntoDocument): collectNodes -> getChildNodes.
+ (WebCore::ContainerNode::removedFromDocument): collectNodes -> getChildNodes.
+ * dom/ContainerNode.h:
+ (WebCore::getChildNodes):
+
+ * editing/ReplaceSelectionCommand.cpp: Remove unused NodeVector declaration.
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::HTMLMediaElement): m_nextChildNodeToConsider and m_currentSourceNode
+ are now RefPtrs.
+ (WebCore::HTMLMediaElement::loadTimerFired): Protect HTMLMediaElement from being deleted during
+ a DOM modification during an event callback.
+ (WebCore::HTMLMediaElement::load): Ditto.
+ (WebCore::HTMLMediaElement::selectMediaResource): Set m_nextChildNodeToConsider to the first
+ child node, it will be the first node considered.
+ (WebCore::HTMLMediaElement::havePotentialSourceChild): m_nextChildNodeToConsider and m_currentSourceNode
+ are now RefPtrs.
+ (WebCore::HTMLMediaElement::selectNextSourceChild): Collect all child nodes in a vector before
+ looking for <source> nodes because 'beforeload' event handlers can mutate the DOM. Don't
+ use a <source> that is no longer a child node after 'beforeload'. Use 0 to represent the end
+ of the child node list because m_nextChildNodeToConsider is now a RefPtr so using the previous
+ sentinel, "this", would cause a retain cycle.
+ (WebCore::HTMLMediaElement::sourceWasAdded): m_nextChildNodeToConsider and m_currentSourceNode
+ are now RefPtrs.
+ (WebCore::HTMLMediaElement::sourceWillBeRemoved): Ditto.
+ (WebCore::HTMLMediaElement::getPluginProxyParams): Protect HTMLMediaElement from being deleted during
+ a DOM modification during an event callback.
+ * html/HTMLMediaElement.h:
+
2012-03-23 Dean Jackson <dino@apple.com>
Disable CSS_SHADERS in Apple builds
typedef pair<NodeCallback, CallbackParameters> CallbackInfo;
typedef Vector<CallbackInfo> NodeCallbackQueue;
-typedef Vector<RefPtr<Node>, 11> 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()
void ContainerNode::takeAllChildrenFrom(ContainerNode* oldParent)
{
NodeVector children;
- collectNodes(oldParent, children);
+ getChildNodes(oldParent, children);
oldParent->removeAllChildren();
for (unsigned i = 0; i < children.size(); ++i) {
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;
container->document()->nodeChildrenWillBeRemoved(container);
NodeVector children;
- collectNodes(container, children);
+ getChildNodes(container, children);
#if ENABLE(MUTATION_OBSERVERS)
ChildListMutationScope mutation(container);
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
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
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
namespace WebCore {
-typedef Vector<RefPtr<Node> > NodeVector;
-
using namespace HTMLNames;
enum EFragmentType { EmptyFragment, SingleTextNodeFragment, TreeFragment };
, 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
void HTMLMediaElement::loadTimerFired(Timer<HTMLMediaElement>*)
{
+ RefPtr<HTMLMediaElement> protect(this); // loadNextSourceChild may fire 'beforeload', which can make arbitrary DOM mutations.
+
#if ENABLE(VIDEO_TRACK)
if (m_pendingLoadFlags & TextTrackResource)
configureNewTextTracks();
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())
// 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
{
// 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);
LOG(Media, "HTMLMediaElement::selectNextSourceChild");
#endif
- if (m_nextChildNodeToConsider == sourceChildEndOfListValue()) {
+ if (!m_nextChildNodeToConsider) {
#if !LOG_DISABLED
if (shouldLog)
LOG(Media, "HTMLMediaElement::selectNextSourceChild -> 0x0000, \"\"");
KURL mediaURL;
Node* node;
HTMLSourceElement* source = 0;
- bool lookingForStartNode = m_nextChildNodeToConsider;
- bool canUse = false;
String type;
+ bool lookingForStartNode = m_nextChildNodeToConsider;
+ bool canUseSourceElement = false;
+ bool okToLoadSourceURL;
+
+ NodeVector potentialSourceNodes;
+ getChildNodes(this, potentialSourceNodes);
- for (node = firstChild(); !canUse && node; node = node->nextSibling()) {
+ 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);
}
// 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(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)
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();
}
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
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()) {
// 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)
git://git.webkit.org / WebKit-https.git / commitdiff