Changeset 140856 in webkit


Ignore:
Timestamp:
Jan 25, 2013 12:13:32 PM (11 years ago)
Author:
esprehn@chromium.org
Message:

Consider all ancestors not just parentElement when disconnecting frames
https://bugs.webkit.org/show_bug.cgi?id=107769

Reviewed by Eric Seidel.

Source/WebCore:

Previous we only used the parentElement of the frame owner to decide if
we should disconnect the frame, but this means if you reparent a subtree
that contains multiple frames from inside an unload handler we'll disconnect
the subframes even though they're now in a different part of the document.

We can fix this by using containsIncludingShadowDOM, and also simplify the
code by removing ChildFrameDisconnector::Target.

Test: fast/frames/unload-reparent-sibling-frame.html

  • dom/ContainerNodeAlgorithms.cpp:
  • dom/ContainerNodeAlgorithms.h:

(ChildFrameDisconnector):
(ChildFrameDisconnector::Target): Removed.
(WebCore::ChildFrameDisconnector::collectFrameOwners):
(WebCore::ChildFrameDisconnector::disconnectCollectedFrameOwners):

LayoutTests:

Add a test for moving frames around inside unload handlers.

  • fast/frames/unload-reparent-sibling-frame-expected.txt: Added.
  • fast/frames/unload-reparent-sibling-frame.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r140854 r140856  
     12013-01-25  Elliott Sprehn  <esprehn@chromium.org>
     2
     3        Consider all ancestors not just parentElement when disconnecting frames
     4        https://bugs.webkit.org/show_bug.cgi?id=107769
     5
     6        Reviewed by Eric Seidel.
     7
     8        Add a test for moving frames around inside unload handlers.
     9
     10        * fast/frames/unload-reparent-sibling-frame-expected.txt: Added.
     11        * fast/frames/unload-reparent-sibling-frame.html: Added.
     12
    1132013-01-25  Tony Chang  <tony@chromium.org>
    214
  • trunk/Source/WebCore/ChangeLog

    r140854 r140856  
     12013-01-25  Elliott Sprehn  <esprehn@chromium.org>
     2
     3        Consider all ancestors not just parentElement when disconnecting frames
     4        https://bugs.webkit.org/show_bug.cgi?id=107769
     5
     6        Reviewed by Eric Seidel.
     7
     8        Previous we only used the parentElement of the frame owner to decide if
     9        we should disconnect the frame, but this means if you reparent a subtree
     10        that contains multiple frames from inside an unload handler we'll disconnect
     11        the subframes even though they're now in a different part of the document.
     12
     13        We can fix this by using containsIncludingShadowDOM, and also simplify the
     14        code by removing ChildFrameDisconnector::Target.
     15
     16        Test: fast/frames/unload-reparent-sibling-frame.html
     17
     18        * dom/ContainerNodeAlgorithms.cpp:
     19        * dom/ContainerNodeAlgorithms.h:
     20        (ChildFrameDisconnector):
     21        (ChildFrameDisconnector::Target): Removed.
     22        (WebCore::ChildFrameDisconnector::collectFrameOwners):
     23        (WebCore::ChildFrameDisconnector::disconnectCollectedFrameOwners):
     24
    1252013-01-25  Tony Chang  <tony@chromium.org>
    226
  • trunk/Source/WebCore/dom/ContainerNodeAlgorithms.cpp

    r140807 r140856  
    116116}
    117117
    118 void ChildFrameDisconnector::Target::disconnect()
    119 {
    120     ASSERT(isValid());
    121     toFrameOwnerElement(m_owner.get())->disconnectContentFrame();
    122 }
    123 
    124118#ifndef NDEBUG
    125119unsigned assertConnectedSubrameCountIsConsistent(Node* node)
  • trunk/Source/WebCore/dom/ContainerNodeAlgorithms.h

    r140807 r140856  
    278278    void disconnectCollectedFrameOwners();
    279279
    280     class Target {
    281     public:
    282         Target(HTMLFrameOwnerElement* element)
    283             : m_owner(element)
    284             , m_ownerParent(element->parentNode())
    285         {
    286         }
    287 
    288         bool isValid() const { return m_owner->parentNode() == m_ownerParent; }
    289         void disconnect();
    290 
    291     private:
    292         RefPtr<HTMLFrameOwnerElement> m_owner;
    293         ContainerNode* m_ownerParent;
    294     };
    295 
    296     Vector<Target, 10> m_list;
     280    Vector<RefPtr<HTMLFrameOwnerElement>, 10> m_frameOwners;
    297281    Node* m_root;
    298282};
     
    310294    // and we should not depend on hasCustomCallbacks().
    311295    if (root->hasCustomCallbacks() && root->isFrameOwnerElement())
    312         m_list.append(toFrameOwnerElement(root));
     296        m_frameOwners.append(toFrameOwnerElement(root));
    313297
    314298    for (Node* child = root->firstChild(); child; child = child->nextSibling())
     
    325309    // insert more frames and create loaded frames in detached subtrees.
    326310    SubframeLoadingDisabler disabler(m_root);
    327     unsigned size = m_list.size();
    328     for (unsigned i = 0; i < size; ++i) {
    329         Target& target = m_list[i];
    330         if (target.isValid())
    331             target.disconnect();
     311
     312    for (unsigned i = 0; i < m_frameOwners.size(); ++i) {
     313        HTMLFrameOwnerElement* owner = m_frameOwners[i].get();
     314        // Don't need to traverse up the tree for the first owner since no
     315        // script could have moved it.
     316        if (!i || m_root->containsIncludingShadowDOM(owner))
     317            owner->disconnectContentFrame();
    332318    }
    333319}
Note: See TracChangeset for help on using the changeset viewer.