Title: [111775] trunk/Source/WebCore
Revision
111775
Author
aba...@webkit.org
Date
2012-03-22 15:39:39 -0700 (Thu, 22 Mar 2012)

Log Message

ContainerNode::insertedIntoTree and removedFromTree use weak iteration patterns
https://bugs.webkit.org/show_bug.cgi?id=80570

Reviewed by Ryosuke Niwa.

These functions use weak iteration patterns, but as far as I can tell,
we never execute script below these functions.  This patch adds ASSERTs
to help us avoid adding events in the future.

* dom/ContainerNode.cpp:
(WebCore::ContainerNode::insertedIntoTree):
(WebCore::ContainerNode::removedFromTree):
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::loadInternal):
    - There's a somewhat complex call chain from insertedIntoTree into
      HTMLMediaElement, and somewhat complex control flow below
      loadInternal that eventually leads to the BeforeLoad event being
      fired.  In studying this code, I don't see a way for the
      BeforeLoad event to be fired during insertedIntoTree, but I've
      added this assert here to make sure we don't call loadInternal
      when we're not supposed to dispatch events.  This ASSERT should
      help us catch these BeforeLoad errors more quickly.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (111774 => 111775)


--- trunk/Source/WebCore/ChangeLog	2012-03-22 22:32:54 UTC (rev 111774)
+++ trunk/Source/WebCore/ChangeLog	2012-03-22 22:39:39 UTC (rev 111775)
@@ -1,3 +1,28 @@
+2012-03-22  Adam Barth  <aba...@webkit.org>
+
+        ContainerNode::insertedIntoTree and removedFromTree use weak iteration patterns
+        https://bugs.webkit.org/show_bug.cgi?id=80570
+
+        Reviewed by Ryosuke Niwa.
+
+        These functions use weak iteration patterns, but as far as I can tell,
+        we never execute script below these functions.  This patch adds ASSERTs
+        to help us avoid adding events in the future.
+
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::insertedIntoTree):
+        (WebCore::ContainerNode::removedFromTree):
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::loadInternal):
+            - There's a somewhat complex call chain from insertedIntoTree into
+              HTMLMediaElement, and somewhat complex control flow below
+              loadInternal that eventually leads to the BeforeLoad event being
+              fired.  In studying this code, I don't see a way for the
+              BeforeLoad event to be fired during insertedIntoTree, but I've
+              added this assert here to make sure we don't call loadInternal
+              when we're not supposed to dispatch events.  This ASSERT should
+              help us catch these BeforeLoad errors more quickly.
+
 2012-03-22  Raphael Kubo da Costa  <rak...@freebsd.org>
 
         Crash in fast/dom/navigator-detached-nocrash.html

Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (111774 => 111775)


--- trunk/Source/WebCore/dom/ContainerNode.cpp	2012-03-22 22:32:54 UTC (rev 111774)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp	2012-03-22 22:39:39 UTC (rev 111775)
@@ -795,20 +795,24 @@
 {
     if (!deep)
         return;
+    forbidEventDispatch();
     for (Node* child = m_firstChild; child; child = child->nextSibling()) {
         if (child->isContainerNode())
             toContainerNode(child)->insertedIntoTree(true);
     }
+    allowEventDispatch();
 }
 
 void ContainerNode::removedFromTree(bool deep)
 {
     if (!deep)
         return;
+    forbidEventDispatch();
     for (Node* child = m_firstChild; child; child = child->nextSibling()) {
         if (child->isContainerNode())
             toContainerNode(child)->removedFromTree(true);
     }
+    allowEventDispatch();
 }
 
 void ContainerNode::childrenChanged(bool changedByParser, Node*, Node*, int childCountDelta)

Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (111774 => 111775)


--- trunk/Source/WebCore/html/HTMLMediaElement.cpp	2012-03-22 22:32:54 UTC (rev 111774)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp	2012-03-22 22:39:39 UTC (rev 111775)
@@ -715,6 +715,11 @@
 
 void HTMLMediaElement::loadInternal()
 {
+    // Some of the code paths below this function dispatch the BeforeLoad event. This ASSERT helps
+    // us catch those bugs more quickly without needing all the branches to align to actually
+    // trigger the event.
+    ASSERT(!eventDispatchForbidden());
+
     // If we can't start a load right away, start it later.
     Page* page = document()->page();
     if (pageConsentRequiredForLoad() && page && !page->canStartMedia()) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to