Detach old frame on WebFrame::swap.

Source/web/WebFrame.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "public/web/WebFrame.h"
 
 #include "bindings/core/v8/WindowProxyManager.h"
 #include "core/frame/FrameHost.h"
 #include "core/frame/FrameView.h"
@@ skipping 17 matching lines @@
     if (!frame)
         return 0;
 
     return frame->isWebLocalFrame()
         ? static_cast<Frame*>(toWebLocalFrameImpl(frame)->frame())
         : toWebRemoteFrameImpl(frame)->frame();
 }
 
 bool WebFrame::swap(WebFrame* frame)
 {
-    using std::swap;
     RefPtrWillBeRawPtr<Frame> oldFrame = toCoreFrame(this);
 
-    // All child frames must be detached first.
-    oldFrame->detachChildren();
+    if (isWebLocalFrame())

[dcheng, 2015/03/27 04:08:47]

I think you can get the unload() behavior you want for free by doing something
like:

dispatchUnloadEvent();  // Only dispatches unload event for this subframe.
oldFrame->detachChildren();  // Now any remaining children are detached.

// If the unload event itself or the children already detached this frame, then
just
// return. There is no longer a place to swap this frame into the frame tree.
if (!oldFrame->host())
  return;

/* cache state */
oldFrame->detach();  // The children of the original frame are already gone, but
detach oldFrame itself now.

[lfg, 2015/03/27 04:57:52]

Interesting, I'll try that.

+        dispatchUnloadEvent();
 
     // If the frame has been detached during detaching its children, return
     // immediately.
     // FIXME: There is no unit test for this condition, so one needs to be
     // written.
     if (!oldFrame->host())
         return false;
 
-    if (m_parent) {
-        if (m_parent->m_firstChild == this)
-            m_parent->m_firstChild = frame;
-        if (m_parent->m_lastChild == this)
-            m_parent->m_lastChild = frame;
-        // FIXME: This is due to the fact that the |frame| may be a provisional
-        // local frame, because we don't know if the navigation will result in
-        // an actual page or something else, like a download. The PlzNavigate
-        // project will remove the need for provisional local frames.
-        frame->m_parent = m_parent;
-        m_parent = nullptr;
+    FrameOwner* owner = oldFrame->owner();
+    const AtomicString name = oldFrame->tree().name();
+    FrameHost* host = oldFrame->host();
+    WebFrame* parent = m_parent;
+    WebFrame* parentOldFirstChild = parent ? parent->m_firstChild : nullptr;
+    WebFrame* parentOldLastChild = parent ? parent->m_lastChild : nullptr;
+    WebFrame* previousSibling = m_previousSibling;
+    WebFrame* nextSibling = m_nextSibling;
+    WebFrame* opener = m_opener;
+    WebPrivateOwnPtr<OpenedFrameTracker> openedFrameTracker;
+    openedFrameTracker.reset(m_openedFrameTracker.release());
+
+    oldFrame->prepareSwapFrom(oldFrame.get());
+
+    oldFrame->detach();
+    // Object is invalid beyond this point.

[dcheng, 2015/03/27 04:08:47]

I don't think this comment is completely accurate--after all, we call
finishSwapFrom(oldFrame.get()) below.

[lfg, 2015/03/27 04:57:52]

The WebFrame is gone (i.e. the object pointed by the 'this' pointer), but the
Frame is still alive because of the refcounted reference we took at the
beginning of this function.

+
+    if (parent) {
+        if (parentOldFirstChild == this)
+            parent->m_firstChild = frame;
+        if (parentOldLastChild == this)
+            parent->m_lastChild = frame;
+        frame->m_parent = parent;
     }
 
-    if (m_previousSibling) {
-        m_previousSibling->m_nextSibling = frame;
-        swap(m_previousSibling, frame->m_previousSibling);
+    if (previousSibling) {
+        previousSibling->m_nextSibling = frame;
+        frame->m_previousSibling = previousSibling;
     }
-    if (m_nextSibling) {
-        m_nextSibling->m_previousSibling = frame;
-        swap(m_nextSibling, frame->m_nextSibling);
+    if (nextSibling) {
+        nextSibling->m_previousSibling = frame;
+        frame->m_nextSibling = nextSibling;
     }
 
-    if (m_opener) {
-        m_opener->m_openedFrameTracker->remove(this);
-        m_opener->m_openedFrameTracker->add(frame);
-        swap(m_opener, frame->m_opener);
+    if (opener) {
+        opener->m_openedFrameTracker->remove(this);
+        opener->m_openedFrameTracker->add(frame);
+        frame->m_opener = opener;
     }
-    if (!m_openedFrameTracker->isEmpty()) {
-        m_openedFrameTracker->updateOpener(frame);
-        frame->m_openedFrameTracker.reset(m_openedFrameTracker.release());
+    if (!openedFrameTracker->isEmpty()) {
+        openedFrameTracker->updateOpener(frame);
+        frame->m_openedFrameTracker.reset(openedFrameTracker.release());
     }
 
     // Finally, clone the state of the current Frame into one matching
     // the type of the passed in WebFrame.
     // FIXME: This is a bit clunky; this results in pointless decrements and
     // increments of connected subframes.
-    FrameOwner* owner = oldFrame->owner();
-    oldFrame->disconnectOwnerElement();
     if (frame->isWebLocalFrame()) {
         LocalFrame& localFrame = *toWebLocalFrameImpl(frame)->frame();
         ASSERT(owner == localFrame.owner());
         if (owner) {
             if (owner->isLocal()) {
                 HTMLFrameOwnerElement* ownerElement = toHTMLFrameOwnerElement(owner);
                 ownerElement->setContentFrame(localFrame);
                 ownerElement->setWidget(localFrame.view());
             } else {
                 toRemoteBridgeFrameOwner(owner)->setContentFrame(toWebLocalFrameImpl(frame));
             }
         } else {
             localFrame.page()->setMainFrame(&localFrame);
         }
     } else {
-        toWebRemoteFrameImpl(frame)->initializeCoreFrame(oldFrame->host(), owner, oldFrame->tree().name());
+        toWebRemoteFrameImpl(frame)->initializeCoreFrame(host, owner, name);
     }
     toCoreFrame(frame)->finishSwapFrom(oldFrame.get());
+    toCoreFrame(frame)->tree().invalidateScopedChildCount();
+    toCoreFrame(frame)->host()->incrementSubframeCount();
+    openedFrameTracker.reset(0);
 
     return true;
 }
 
 void WebFrame::detach()
 {
     toCoreFrame(this)->detach();
 }
 
 WebSecurityOrigin WebFrame::securityOrigin() const
@@ skipping 201 matching lines @@
     bool WebFrame::isFrameAlive(VisitorType visitor, const WebFrame* frame) { return isFrameAliveImpl(visitor, frame); } \
     void WebFrame::clearWeakFrames(VisitorType visitor) { clearWeakFramesImpl(visitor); }
 
 DEFINE_VISITOR_METHOD(Visitor*)
 DEFINE_VISITOR_METHOD(InlinedGlobalMarkingVisitor)
 
 #undef DEFINE_VISITOR_METHOD
 #endif
 
 } // namespace blink