2010-08-12 Dimitri Glazkov <dglazkov@chromium.org>
authordglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Aug 2010 23:23:13 +0000 (23:23 +0000)
committerdglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 12 Aug 2010 23:23:13 +0000 (23:23 +0000)
        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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@65281 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/html/HTMLConstructionSite.cpp
WebCore/html/HTMLLinkElement.cpp
WebCore/html/HTMLLinkElement.h

index 977e9ee..c5fc211 100644 (file)
@@ -1,3 +1,18 @@
+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>
 
         Reviewed by Dumitru Daniliuc.
index 2b28da4..a25c7d9 100644 (file)
@@ -97,6 +97,12 @@ PassRefPtr<ChildType> HTMLConstructionSite::attach(Node* parent, PassRefPtr<Chil
     }
 
     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
     // |parent| to hold a ref at this point.  In the common case (at least
index fc13165..b9fb8f0 100644 (file)
@@ -51,7 +51,6 @@ inline HTMLLinkElement::HTMLLinkElement(const QualifiedName& tagName, Document*
     , m_disabledState(Unset)
     , m_loading(false)
     , m_createdByParser(createdByParser)
-    , m_shouldProcessAfterAttach(false)
 {
     ASSERT(hasTagName(linkTag));
 }
@@ -242,28 +241,12 @@ void HTMLLinkElement::process()
         document()->updateStyleSelector();
     }
 }
-    
-void HTMLLinkElement::processCallback(Node* node)
-{
-    ASSERT_ARG(node, node && node->hasTagName(linkTag));
-    static_cast<HTMLLinkElement*>(node)->process();
-}
 
 void HTMLLinkElement::insertedIntoDocument()
 {
     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();
 }
 
@@ -276,20 +259,8 @@ void HTMLLinkElement::removedFromDocument()
     // FIXME: It's terrible to do a synchronous update of the style selector just because a <style> or <link> element got removed.
     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()
 {
     m_createdByParser = false;
index db069eb..f8ccd12 100644 (file)
@@ -73,9 +73,6 @@ public:
     bool isDisabled() const { return m_disabledState == Disabled; }
     bool isEnabledViaScript() const { return m_disabledState == EnabledViaScript; }
     bool isIcon() const { return m_relAttribute.m_isIcon; }
-    
-    virtual void attach();
-    virtual bool canLazyAttach() { return false; }
 
 private:
     virtual HTMLTagStatus endTagRequirement() const { return TagStatusForbidden; }
@@ -125,7 +122,6 @@ private:
     RelAttribute m_relAttribute;
     bool m_loading;
     bool m_createdByParser;
-    bool m_shouldProcessAfterAttach;
 };
 
 } //namespace