Changeset 65281 in webkit

Timestamp:

Aug 12, 2010 4:23:13 PM (14 years ago)

Author:

Dimitri Glazkov

Message:

2010-08-12 Dimitri Glazkov <Dimitri Glazkov>

Reviewed by Adam Barth.

Ensure that parser doesn't attach children that have been removed by JavaScript event handlers.
​https://bugs.webkit.org/show_bug.cgi?id=43813

This patch re-fixes bug 40742 in a way that keeps allowing HTMLLinkElement
to lazy-attach.

• html/HTMLConstructionSite.cpp: (WebCore::HTMLConstructionSite::attach): Added parent check.
• html/HTMLLinkElement.cpp: Basically undoes changes introduced by r61424.
• html/HTMLLinkElement.h: Ditto.

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/HTMLConstructionSite.cpp (modified) (1 diff)
• html/HTMLLinkElement.cpp (modified) (4 diffs)
• html/HTMLLinkElement.h (modified) (2 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-08-12  Dimitri Glazkov  <dglazkov@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        Ensure that parser doesn't attach children that have been removed by JavaScript event handlers.
+        https://bugs.webkit.org/show_bug.cgi?id=43813
+
+        This patch re-fixes bug 40742 in a way that keeps allowing HTMLLinkElement
+        to lazy-attach.
+
+        * html/HTMLConstructionSite.cpp:
+        (WebCore::HTMLConstructionSite::attach): Added parent check.
+        * html/HTMLLinkElement.cpp: Basically undoes changes introduced by r61424.
+        * html/HTMLLinkElement.h: Ditto.
+
 2010-08-12  Justin Schuh  <jschuh@chromium.org>
 

trunk/WebCore/html/HTMLConstructionSite.cpp

@@ -98 +98 @@
 
     parent->parserAddChild(child);
+
+    // An event handler (DOM Mutation, beforeload, et al.) could have removed
+    // the child, in which case we shouldn't try attaching it.
+    if (!child->parentNode())
+        return child.release();
+
     // It's slightly unfortunate that we need to hold a reference to child
     // here to call attach().  We should investigate whether we can rely on

trunk/WebCore/html/HTMLLinkElement.cpp

@@ -52 +52 @@
     , m_loading(false)
     , m_createdByParser(createdByParser)
-    , m_shouldProcessAfterAttach(false)
 {
     ASSERT(hasTagName(linkTag));
@@ -243 +242 @@
     }
 }
-    
-void HTMLLinkElement::processCallback(Node* node)
-{
-    ASSERT_ARG(node, node && node->hasTagName(linkTag));
-    static_cast<HTMLLinkElement*>(node)->process();
-}
 
 void HTMLLinkElement::insertedIntoDocument()
@@ -254 +247 @@
     HTMLElement::insertedIntoDocument();
     document()->addStyleSheetCandidateNode(this, m_createdByParser);
-
-    // Since processing a stylesheet link causes a beforeload event
-    // to fire, it is possible for JavaScript to remove the element in the midst
-    // of it being inserted into the DOM, which can lead to assertion failures
-    // and crashes. Avoid this by postponing the beforeload/load until after
-    // attach if there are beforeload listeners.
-    if (document()->hasListenerType(Document::BEFORELOAD_LISTENER)) {
-        m_shouldProcessAfterAttach = true;
-        return;
-    }
 
     process();
@@ -277 +260 @@
     if (document()->renderer())
         document()->updateStyleSelector();
-    
-    m_shouldProcessAfterAttach = false;
-}
-
-void HTMLLinkElement::attach()
-{
-    if (m_shouldProcessAfterAttach) {
-        m_shouldProcessAfterAttach = false;
-        queuePostAttachCallback(&HTMLLinkElement::processCallback, this);
-    }
-
-    HTMLElement::attach();
-}
-    
+}
+
 void HTMLLinkElement::finishParsingChildren()
 {

trunk/WebCore/html/HTMLLinkElement.h

@@ -74 +74 @@
     bool isEnabledViaScript() const { return m_disabledState == EnabledViaScript; }
     bool isIcon() const { return m_relAttribute.m_isIcon; }
-    
-    virtual void attach();
-    virtual bool canLazyAttach() { return false; }
 
 private:
@@ -126 +123 @@
     bool m_loading;
     bool m_createdByParser;
-    bool m_shouldProcessAfterAttach;
 };