Changeset 67182 in webkit


Ignore:
Timestamp:
Sep 10, 2010, 2:12:11 AM (14 years ago)
Author:
eric@webkit.org
Message:

2010-09-10 Eric Seidel <​eric@webkit.org>

Reviewed by Dimitri Glazkov.

Remove support for non-lazy attach and fix frames to load from insertedIntoDocument instead of attach
​https://bugs.webkit.org/show_bug.cgi?id=45365

  • fast/dom/beforeload/remove-frame-in-beforeload-listener.html:
    • Add a comment to make clear that this test intentionally does not cancel the load in the beforeload event, but rather relies on WebCore's own Node::willRemove() implementation to cancel the load when the frame is removed. (The load is not actually directly canceled per say, but the loading logic checks to see if the frame is still in the frame tree right after creating the frame and aborts if its not. HTMLFrameOwnerElement::willRemove clears the frame owner pointer causing the frame to be removed from the tree.)

2010-09-10 Eric Seidel <​eric@webkit.org>

Reviewed by Dimitri Glazkov.

Remove support for non-lazy attach and fix frames to load from insertedIntoDocument instead of attach
​https://bugs.webkit.org/show_bug.cgi?id=45365

This change is the last piece of the puzzle which was preventing us from
removing all eager-attach logic and moving WebCore to using an entirely
recalcStyle-driven, on-demand renderer creation system, instead of every
element being synchronously attached during parsing, regardless of whether
it was ever going to be displayed or not.

This does not change the places we call lazyAttach vs. attach. This only
changes the behavior of frame/plugin elements when lazyAttach is called.
Previously lazyAttach would eager-attach those elements (and their ancestors)
because they were marked as canLazyAttach() { return false; }.

This is a very tricky change, please review carefully.

Mostly I'm moving logic which used to be in attach() into
insertedIntoDocument. Once it was there, there was no longer any reason
why frame elements couldn't lazyAttach, thus removing the need
for the non-lazy attach code path entirely.
We've not yet converted all callsites over to using lazyAttach() instead
of attach() however.

In order to move frame loading logic into insertedIntoDocument
instead of attach, I needed to make sure that any javascript calls
during the load would see an attached element. Thus I needed to mark
the element as needing style resolve so that it would attach itself
if needed.

I was not able to just call lazyAttach() from insertedIntoDocument directly
due to two conflicting assumptions in the rendering tree:

  1. ContainerNode::attach() assumes its "safe" to call attach on all children without checking first if the child is already attached. This seems sane since its strange to think of a subtree as being attached w/o ancestors already being attached. Although it is a hard rule that subtrees may not have renderers w/o ancestors also having renderers, I do not believe it's a hard rule that subtrees may not be attached. Remember, attached() does not imply renderer()! It's possible ContainerNode::attach()'s assumption is wrong here.
  2. Node::attach() asserts !attached(). This makes sense and I would not want to remove this assert, however it means that if insertedIntoDocument were to call lazyAttach() (thus marking the element as attached()) then all callers would need to check if the element was already attached after insertedIntoDocument (called by appendChild, parserAppendChild, etc.) before calling attach or lazyAttach(). The following example: element.innerHTML = "<span><iframe></span>" is one case where this ASSERT would be hit if insertedIntoDocument called lazyAttach, since ContainerNode::attach() (called on the span by in appendChild(DocumentFragment) code) does not check if iframe is already attached.

Note: One subtle change here is that synchronous javascript which results
from javascript: or beforeload is now run as part of insertedIntoDocument
(thus any insert/append call *even* parserAddChild) instead of being
run during attach (technically in the post-attach callbacks).

This is covered by numerous layout tests.

  • dom/ContainerNode.cpp: (WebCore::willRemoveChild): (WebCore::willRemoveChildren):
    • Since insertedIntoDocument starts a load and yet does not mark the element as attached, we need to always call willRemove(). See note above as to why we don't just mark attached() in insertedIntoDocument.
  • dom/Node.cpp: (WebCore::Node::markAncestorsWithChildNeedsStyleRecalc):
    • Share some code between lazyAttach and setNeedsStyleRecalc.

(WebCore::Node::setNeedsStyleRecalc):

  • Use the new markAncestorsWithChildNeedsStyleRecalc

(WebCore::Node::lazyAttach):

  • Remove the non-lazy code path, and use markAncestorsWithChildNeedsStyleRecalc.
  • Add an option to lazyAttach without marking attached(), used by HTMLFrameElementBase::insertedIntoDocument.
  • dom/Node.h:
  • html/HTMLFrameElementBase.cpp:
    • m_shouldOpenURLAfterAttach is no longer needed, yay!
    • m_checkAttachedTimer no longer has anything to do with attached(), so renamed it. I also documented that the newly named m_checkInDocumentTimer is all about the "magic iframe" performance quirk. (Which is actually speced in HTML5). I was initially baffled by this code, so I documented it.

(WebCore::HTMLFrameElementBase::HTMLFrameElementBase)
(WebCore::HTMLFrameElementBase::insertedIntoDocument):

  • This is the meat of this change, see discussion above.

(WebCore::HTMLFrameElementBase::attach):

  • Code deleted or moved to insertedIntoDocument.

(WebCore::HTMLFrameElementBase::width):

  • Fixed a bug in height()/width() which was probably causing crashes and was causing incorrect behavior after this change. renderBox() is not necessarily valid unless layout is up to date. Updating layout, can cause renderBox() to go invalid, thus this could have been a null-pointer crash.

(WebCore::HTMLFrameElementBase::height): see width()
(WebCore::HTMLFrameElementBase::setRemainsAliveOnRemovalFromTree): Timer rename.
(WebCore::HTMLFrameElementBase::checkInDocumentTimerFired): Timer rename.

  • html/HTMLFrameElementBase.h:
  • html/HTMLFrameOwnerElement.cpp: (WebCore::HTMLFrameOwnerElement::willRemove):
    • Disconnecting the owner element removes the frame from the frame tree. frameDetached() calls Page::frameCount which expects that the frame is already gone at this point and asserts when it's not. It's unclear how this worked before, except that the frame removal was likely done in the post-attach callback, so the frameCount was wrong (too high) during frameDetached(), but was fixed up in the post-detach callback.
  • html/parser/HTMLConstructionSite.cpp: (WebCore::HTMLConstructionSite::attachAtSite):
    • Simplified this code, and added a check for the case when the node was already removed. Since the load logic is now run during insertedIntoDocument instead of attach(), synchronous javascript is now running during insertedIntoDocument and we need to make sure that the child is still in the tree.
Location:
trunk
Files:
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r67181 r67182  
     12010-09-10  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Dimitri Glazkov.
     4
     5        Remove support for non-lazy attach and fix frames to load from insertedIntoDocument instead of attach
     6        https://bugs.webkit.org/show_bug.cgi?id=45365
     7
     8        * fast/dom/beforeload/remove-frame-in-beforeload-listener.html:
     9          - Add a comment to make clear that this test intentionally
     10            does not cancel the load in the beforeload event, but rather
     11            relies on WebCore's own Node::willRemove() implementation to
     12            cancel the load when the frame is removed.
     13            (The load is not actually directly canceled per say, but the
     14            loading logic checks to see if the frame is still in the frame
     15            tree right after creating the frame and aborts if its not.
     16            HTMLFrameOwnerElement::willRemove clears the frame owner pointer
     17            causing the frame to be removed from the tree.)
     18
    1192010-09-10  Kent Tamura  <tkent@chromium.org>
    220

  • trunk/LayoutTests/fast/dom/beforeload/remove-frame-in-beforeload-listener.html

    r61744 r67182  
    99            if (event.target && event.target.parentElement)
    1010                event.target.parentElement.removeChild(event.target);
     11                // Note, we intentionally do not cancel the load here,
     12                // WebCore should automatically cancel it.
     13                // Otherwise DRT will print:
     14                // Blocked access to external URL http://webkit.org/
    1115        }, true);
    1216    </script>

  • trunk/WebCore/ChangeLog

    r67179 r67182  
     12010-09-10  Eric Seidel  <eric@webkit.org>
     2
     3        Reviewed by Dimitri Glazkov.
     4
     5        Remove support for non-lazy attach and fix frames to load from insertedIntoDocument instead of attach
     6        https://bugs.webkit.org/show_bug.cgi?id=45365
     7
     8        This change is the last piece of the puzzle which was preventing us from
     9        removing all eager-attach logic and moving WebCore to using an entirely
     10        recalcStyle-driven, on-demand renderer creation system, instead of every
     11        element being synchronously attached during parsing, regardless of whether
     12        it was ever going to be displayed or not.
     13
     14        This does not change the places we call lazyAttach vs. attach.  This only
     15        changes the behavior of frame/plugin elements when lazyAttach is called.
     16        Previously lazyAttach would eager-attach those elements (and their ancestors)
     17        because they were marked as canLazyAttach() { return false; }.
     18
     19        This is a very tricky change, please review carefully.
     20
     21        Mostly I'm moving logic which used to be in attach() into
     22        insertedIntoDocument.  Once it was there, there was no longer any reason
     23        why frame elements couldn't lazyAttach, thus removing the need
     24        for the non-lazy attach code path entirely.
     25        We've not yet converted all callsites over to using lazyAttach() instead
     26        of attach() however.
     27
     28        In order to move frame loading logic into insertedIntoDocument
     29        instead of attach, I needed to make sure that any javascript calls
     30        during the load would see an attached element.  Thus I needed to mark
     31        the element as needing style resolve so that it would attach itself
     32        if needed.
     33
     34        I was not able to just call lazyAttach() from insertedIntoDocument directly
     35        due to two conflicting assumptions in the rendering tree:
     36         1. ContainerNode::attach() assumes its "safe" to call attach on all children
     37            without checking first if the child is already attached.  This seems sane
     38            since its strange to think of a subtree as being attached w/o ancestors
     39            already being attached.  Although it is a hard rule that subtrees may not
     40            have renderers w/o ancestors also having renderers, I do not believe it's
     41            a hard rule that subtrees may not be attached.  Remember, attached() does
     42            not imply renderer()!  It's possible ContainerNode::attach()'s assumption is wrong here.
     43         2. Node::attach() asserts !attached().  This makes sense and I would not
     44            want to remove this assert, however it means that if insertedIntoDocument
     45            were to call lazyAttach() (thus marking the element as attached()) then
     46            all callers would need to check if the element was already attached after
     47            insertedIntoDocument (called by appendChild, parserAppendChild, etc.)
     48            before calling attach or lazyAttach().  The following example:
     49            element.innerHTML = "<span><iframe></span>" is one case where this
     50            ASSERT would be hit if insertedIntoDocument called lazyAttach, since
     51            ContainerNode::attach() (called on the span by in appendChild(DocumentFragment) code)
     52            does not check if iframe is already attached.
     53
     54        Note: One subtle change here is that synchronous javascript which results
     55        from javascript: or beforeload is now run as part of insertedIntoDocument
     56        (thus any insert/append call *even* parserAddChild) instead of being
     57        run during attach (technically in the post-attach callbacks).
     58
     59        This is covered by numerous layout tests.
     60
     61        * dom/ContainerNode.cpp:
     62        (WebCore::willRemoveChild):
     63        (WebCore::willRemoveChildren):
     64         - Since insertedIntoDocument starts a load and yet does not mark the
     65           element as attached, we need to always call willRemove().
     66           See note above as to why we don't just mark attached() in insertedIntoDocument.
     67        * dom/Node.cpp:
     68        (WebCore::Node::markAncestorsWithChildNeedsStyleRecalc):
     69         - Share some code between lazyAttach and setNeedsStyleRecalc.
     70        (WebCore::Node::setNeedsStyleRecalc):
     71         - Use the new markAncestorsWithChildNeedsStyleRecalc
     72        (WebCore::Node::lazyAttach):
     73         - Remove the non-lazy code path, and use markAncestorsWithChildNeedsStyleRecalc.
     74         - Add an option to lazyAttach without marking attached(), used by HTMLFrameElementBase::insertedIntoDocument.
     75        * dom/Node.h:
     76        * html/HTMLFrameElementBase.cpp:
     77         - m_shouldOpenURLAfterAttach is no longer needed, yay!
     78         - m_checkAttachedTimer no longer has anything to do with attached(), so renamed it.
     79           I also documented that the newly named m_checkInDocumentTimer is all about the
     80           "magic iframe" performance quirk.  (Which is actually speced in HTML5).
     81           I was initially baffled by this code, so I documented it.
     82        (WebCore::HTMLFrameElementBase::HTMLFrameElementBase)
     83        (WebCore::HTMLFrameElementBase::insertedIntoDocument):
     84         - This is the meat of this change, see discussion above.
     85        (WebCore::HTMLFrameElementBase::attach):
     86         - Code deleted or moved to insertedIntoDocument.
     87        (WebCore::HTMLFrameElementBase::width):
     88         - Fixed a bug in height()/width() which was probably causing crashes
     89           and was causing incorrect behavior after this change.
     90           renderBox() is not necessarily valid unless layout is up to date.
     91           Updating layout, can cause renderBox() to go invalid, thus this
     92           could have been a null-pointer crash.
     93        (WebCore::HTMLFrameElementBase::height): see width()
     94        (WebCore::HTMLFrameElementBase::setRemainsAliveOnRemovalFromTree): Timer rename.
     95        (WebCore::HTMLFrameElementBase::checkInDocumentTimerFired): Timer rename.
     96        * html/HTMLFrameElementBase.h:
     97        * html/HTMLFrameOwnerElement.cpp:
     98        (WebCore::HTMLFrameOwnerElement::willRemove):
     99         - Disconnecting the owner element removes the frame from the frame tree.
     100           frameDetached() calls Page::frameCount which expects that the frame is
     101           already gone at this point and asserts when it's not.  It's unclear how
     102           this worked before, except that the frame removal was likely done in the
     103           post-attach callback, so the frameCount was wrong (too high) during
     104           frameDetached(), but was fixed up in the post-detach callback.
     105        * html/parser/HTMLConstructionSite.cpp:
     106        (WebCore::HTMLConstructionSite::attachAtSite):
     107          - Simplified this code, and added a check for the case when the node was already removed.
     108            Since the load logic is now run during insertedIntoDocument instead of attach(),
     109            synchronous javascript is now running during insertedIntoDocument and we need to
     110            make sure that the child is still in the tree.
     111
    11122010-09-10  Dirk Pranke  <dpranke@chromium.org>
    2113

  • trunk/WebCore/dom/ContainerNode.cpp

    r66730 r67182  
    360360    // fire removed from document mutation events.
    361361    dispatchChildRemovalEvents(child);
    362 
    363     if (child->attached())
    364         child->willRemove();
     362    child->willRemove();
    365363}
    366364
    … …  
    374372        // fire removed from document mutation events.
    375373        dispatchChildRemovalEvents(child.get());
    376 
    377         if (child->attached())
    378             child->willRemove();
     374        child->willRemove();
    379375    }
    380376}

  • trunk/WebCore/dom/Node.cpp

    r66511 r67182  
    719719}
    720720
     721inline void Node::markAncestorsWithChildNeedsStyleRecalc()
     722{
     723    for (Node* p = parentNode(); p && !p->childNeedsStyleRecalc(); p = p->parentNode())
     724        p->setChildNeedsStyleRecalc();
     725   
     726    if (document()->childNeedsStyleRecalc())
     727        document()->scheduleStyleRecalc();
     728}
     729
    721730void Node::setNeedsStyleRecalc(StyleChangeType changeType)
    722731{
    … …  
    729738        setStyleChange(changeType);
    730739
    731     if (existingChangeType == NoStyleChange) {
    732         for (Node* p = parentNode(); p && !p->childNeedsStyleRecalc(); p = p->parentNode())
    733             p->setChildNeedsStyleRecalc();
    734 
    735         if (document()->childNeedsStyleRecalc())
    736             document()->scheduleStyleRecalc();
    737     }
    738 }
    739 
    740 static Node* outermostLazyAttachedAncestor(Node* start)
    741 {
    742     Node* p = start;
    743     for (Node* next = p->parentNode(); !next->renderer(); p = next, next = next->parentNode()) {}
    744     return p;
    745 }
    746 
    747 void Node::lazyAttach()
    748 {
    749     bool mustDoFullAttach = false;
    750 
     740    if (existingChangeType == NoStyleChange)
     741        markAncestorsWithChildNeedsStyleRecalc();
     742}
     743
     744void Node::lazyAttach(ShouldSetAttached shouldSetAttached)
     745{
    751746    for (Node* n = this; n; n = n->traverseNextNode(this)) {
    752         if (!n->canLazyAttach()) {
    753             mustDoFullAttach = true;
    754             break;
    755         }
    756 
    757747        if (n->firstChild())
    758748            n->setChildNeedsStyleRecalc();
    759749        n->setStyleChange(FullStyleChange);
    760         n->setAttached();
    761     }
    762 
    763     if (mustDoFullAttach) {
    764         Node* lazyAttachedAncestor = outermostLazyAttachedAncestor(this);
    765         if (lazyAttachedAncestor->attached())
    766             lazyAttachedAncestor->detach();
    767         lazyAttachedAncestor->attach();
    768     } else {
    769         for (Node* p = parentNode(); p && !p->childNeedsStyleRecalc(); p = p->parentNode())
    770             p->setChildNeedsStyleRecalc();
    771         if (document()->childNeedsStyleRecalc())
    772             document()->scheduleStyleRecalc();
    773     }
     750        if (shouldSetAttached == SetAttached)
     751            n->setAttached();
     752    }
     753    markAncestorsWithChildNeedsStyleRecalc();
    774754}
    775755

  • trunk/WebCore/dom/Node.h

    r66498 r67182  
    311311    void clearIsLink() { clearFlag(IsLinkFlag); }
    312312
    313     void lazyAttach();
    314     virtual bool canLazyAttach() { return true; }
     313    enum ShouldSetAttached {
     314        SetAttached,
     315        DoNotSetAttached
     316    };
     317    void lazyAttach(ShouldSetAttached = SetAttached);
    315318
    316319    virtual void setFocus(bool b = true);
    … …  
    682685    void setStyleChange(StyleChangeType);
    683686
     687    // Used to share code between lazyAttach and setNeedsStyleRecalc.
     688    void markAncestorsWithChildNeedsStyleRecalc();
     689
    684690    virtual void refEventTarget() { ref(); }
    685691    virtual void derefEventTarget() { deref(); }

  • trunk/WebCore/html/HTMLFrameElementBase.cpp

    r66631 r67182  
    5353    , m_marginWidth(-1)
    5454    , m_marginHeight(-1)
    55     , m_checkAttachedTimer(this, &HTMLFrameElementBase::checkAttachedTimerFired)
     55    , m_checkInDocumentTimer(this, &HTMLFrameElementBase::checkInDocumentTimerFired)
    5656    , m_viewSource(false)
    57     , m_shouldOpenURLAfterAttach(false)
    5857    , m_remainsAliveOnRemovalFromTree(false)
    5958{
    … …  
    184183{
    185184    HTMLFrameOwnerElement::insertedIntoDocument();
    186    
    187     // We delay frame loading until after the render tree is fully constructed.
    188     // Othewise, a synchronous load that executed JavaScript would see incorrect
    189     // (0) values for the frame's renderer-dependent properties, like width.
    190     m_shouldOpenURLAfterAttach = true;
    191 
    192     if (m_remainsAliveOnRemovalFromTree)
     185
     186    if (m_remainsAliveOnRemovalFromTree) {
    193187        updateOnReparenting();
    194 }
    195 
    196 void HTMLFrameElementBase::removedFromDocument()
    197 {
    198     m_shouldOpenURLAfterAttach = false;
    199 
    200     HTMLFrameOwnerElement::removedFromDocument();
     188        setRemainsAliveOnRemovalFromTree(false);
     189        return;
     190    }
     191    // DocumentFragments don't kick of any loads.
     192    if (!document()->frame())
     193        return;
     194
     195    // Loads may cause synchronous javascript execution (e.g. beforeload or
     196    // src=javascript), which could try to access the renderer before the normal
     197    // parser machinery would call lazyAttach() and set us as needing style
     198    // resolve.  Any code which expects this to be attached will resolve style
     199    // before using renderer(), so this will make sure we attach in time.
     200    // FIXME: Normally lazyAttach marks the renderer as attached(), but we don't
     201    // want to do that here, as as callers expect to call attach() right after
     202    // this and attach() will ASSERT(!attached())
     203    ASSERT(!renderer()); // This recalc is unecessary if we already have a renderer.
     204    lazyAttach(DoNotSetAttached);
     205    setNameAndOpenURL();
    201206}
    202207
    203208void HTMLFrameElementBase::attach()
    204209{
    205     if (m_shouldOpenURLAfterAttach) {
    206         m_shouldOpenURLAfterAttach = false;
    207         if (!m_remainsAliveOnRemovalFromTree)
    208             queuePostAttachCallback(&HTMLFrameElementBase::setNameAndOpenURLCallback, this);
    209     }
    210 
    211     setRemainsAliveOnRemovalFromTree(false);
    212 
    213210    HTMLFrameOwnerElement::attach();
    214211
    … …  
    259256int HTMLFrameElementBase::width() const
    260257{
     258    document()->updateLayoutIgnorePendingStylesheets();
    261259    if (!renderBox())
    262260        return 0;
    263 
     261    return renderBox()->width();
     262}
     263
     264int HTMLFrameElementBase::height() const
     265{
    264266    document()->updateLayoutIgnorePendingStylesheets();
    265     return renderBox()->width();
    266 }
    267 
    268 int HTMLFrameElementBase::height() const
    269 {
    270267    if (!renderBox())
    271268        return 0;
    272 
    273     document()->updateLayoutIgnorePendingStylesheets();
    274269    return renderBox()->height();
    275270}
    … …  
    282277    // Start the async timer that is normally stopped by attach(). If it's not stopped and fires, it'll unload the frame.
    283278    if (value)
    284         m_checkAttachedTimer.startOneShot(0);
     279        m_checkInDocumentTimer.startOneShot(0);
    285280    else
    286         m_checkAttachedTimer.stop();
    287 }
    288 
    289 void HTMLFrameElementBase::checkAttachedTimerFired(Timer<HTMLFrameElementBase>*)
     281        m_checkInDocumentTimer.stop();
     282}
     283
     284void HTMLFrameElementBase::checkInDocumentTimerFired(Timer<HTMLFrameElementBase>*)
    290285{
    291286    ASSERT(!attached());

  • trunk/WebCore/html/HTMLFrameElementBase.h

    r59773 r67182  
    5151
    5252    virtual void parseMappedAttribute(Attribute*);
    53 
    5453    virtual void insertedIntoDocument();
    55     virtual void removedFromDocument();
    56 
    5754    virtual void attach();
    5855
    5956private:
    60     virtual bool canLazyAttach() { return false; }
    61 
    6257    virtual bool supportsFocus() const;
    6358    virtual void setFocus(bool);
    … …  
    6863
    6964    virtual void willRemove();
    70     void checkAttachedTimerFired(Timer<HTMLFrameElementBase>*);
     65    void checkInDocumentTimerFired(Timer<HTMLFrameElementBase>*);
    7166    void updateOnReparenting();
    7267
    … …  
    8681    int m_marginHeight;
    8782
    88     Timer<HTMLFrameElementBase> m_checkAttachedTimer;
     83    // This is a performance optimization some call "magic iframe" which avoids
     84    // tearing down the frame hierarchy when a web page calls adoptNode on a
     85    // frame owning element but does not immediately insert it into the new
     86    // document before JavaScript yields to WebCore.  If the element is not yet
     87    // in a document by the time this timer fires, the frame hierarchy teardown
     88    // will continue.  This can also be seen as implementation of:
     89    // "Removing an iframe from a Document does not cause its browsing context
     90    // to be discarded. Indeed, an iframe's browsing context can survive its
     91    // original parent Document if its iframe is moved to another Document."
     92    // From HTML5: http://www.whatwg.org/specs/web-apps/current-work/multipage/the-iframe-element.html#the-iframe-element
     93    Timer<HTMLFrameElementBase> m_checkInDocumentTimer;
    8994
    9095    bool m_viewSource;
    91 
    92     bool m_shouldOpenURLAfterAttach;
    93 
    9496    bool m_remainsAliveOnRemovalFromTree;
    9597};

  • trunk/WebCore/html/HTMLFrameOwnerElement.cpp

    r66631 r67182  
    5252void HTMLFrameOwnerElement::willRemove()
    5353{
     54    // FIXME: It is unclear why this can't be moved to removedFromDocument()
     55    // this is the only implementation of willRemove in WebCore!
    5456    if (Frame* frame = contentFrame()) {
     57        frame->loader()->frameDetached();
    5558        frame->disconnectOwnerElement();
    56         frame->loader()->frameDetached();
    5759    }
    5860

  • trunk/WebCore/html/HTMLPlugInImageElement.h

    r66711 r67182  
    6060
    6161private:
    62     virtual bool canLazyAttach() { return false; }
    6362    virtual RenderObject* createRenderer(RenderArena*, RenderStyle*);
    6463    virtual void recalcStyle(StyleChange);

  • trunk/WebCore/html/parser/HTMLConstructionSite.cpp

    r67030 r67182  
    115115void HTMLConstructionSite::attachAtSite(const AttachmentSite& site, PassRefPtr<Node> prpChild)
    116116{
    117     RefPtr<Node> child = prpChild;
    118 
    119     if (site.nextChild) {
    120         site.parent->parserInsertBefore(child, site.nextChild);
    121         if (site.parent->attached() && !child->attached())
    122             child->attach();
    123         return;
    124     }
    125     site.parent->parserAddChild(child);
    126     // It's slightly unfortunate that we need to hold a reference to child
     117    // FIXME: It's unfortunate that we need to hold a reference to child
    127118    // here to call attach().  We should investigate whether we can rely on
    128119    // |site.parent| to hold a ref at this point.
    129     if (site.parent->attached() && !child->attached())
     120    RefPtr<Node> child = prpChild;
     121
     122    if (site.nextChild)
     123        site.parent->parserInsertBefore(child, site.nextChild);
     124    else
     125        site.parent->parserAddChild(child);
     126
     127    // JavaScript run from beforeload (or DOM Mutation or event handlers)
     128    // might have removed the child, in which case we should not attach it.
     129    if (child->parentNode() && site.parent->attached() && !child->attached())
    130130        child->attach();
    131131}
Note: See TracChangeset for help on using the changeset viewer.