Detach old frame on WebFrame::swap.

Source/web/WebFrame.cpp

Index: Source/web/WebFrame.cpp
diff --git a/Source/web/WebFrame.cpp b/Source/web/WebFrame.cpp
index b7504104a33a316fa96046dccffa5a74da7d4b35..40c2cafffe1c96f475f9094523a138e96d855982 100644
--- a/Source/web/WebFrame.cpp
+++ b/Source/web/WebFrame.cpp
@@ -12,6 +12,7 @@
 #include "core/frame/RemoteFrame.h"
 #include "core/html/HTMLFrameElementBase.h"
 #include "core/html/HTMLFrameOwnerElement.h"
+#include "core/loader/DocumentLoader.h"

[dcheng, 2015/06/10 19:56:29]

Nit: remove this since it's no longer needed here.

[lfg, 2015/06/10 22:43:35]

Done.

 #include "core/page/Page.h"
 #include "platform/UserGestureIndicator.h"
 #include "platform/heap/Handle.h"
@@ -39,17 +40,32 @@ bool WebFrame::swap(WebFrame* frame)
 {
     using std::swap;
     RefPtrWillBeRawPtr<Frame> oldFrame = toCoreFrame(this);
+#if !ENABLE(OILPAN)
+    RefPtrWillBeRawPtr<WebLocalFrameImpl> protectWebLocalFrame = isWebLocalFrame() ? toWebLocalFrameImpl(this) : nullptr;
+    RefPtrWillBeRawPtr<WebRemoteFrameImpl> protectWebRemoteFrame = isWebRemoteFrame() ? toWebRemoteFrameImpl(this) : nullptr;
+#endif
 
-    // All child frames must be detached first.
-    oldFrame->detachChildren();
+    if (isWebLocalFrame())
+        toLocalFrame(toCoreFrame(this))->loader().prepareForCommit();

[dcheng, 2015/06/10 19:56:29]

Let's use virtual dispatch on Frame instead. We can add a new virtual to Frame,
that does the right thing depending on whether it's Local or Remote.

[lfg, 2015/06/10 22:43:35]

I've moved to a virtual dispatch, but I'm hoping you can come up with a better
name for the function.

[dcheng, 2015/06/11 18:45:02]

Meh. It's good enough =)

+    else
+        oldFrame->detachChildren();
 
-    // If the frame has been detached during detaching its children, return
-    // immediately.
+    // If the frame has been detached during detaching by any JS code, we should
+    // cancel the swap.
     // FIXME: There is no unit test for this condition, so one needs to be
     // written.
     if (!oldFrame->host())
         return false;
 
+    FrameOwner* owner = oldFrame->owner();
+    FrameHost* host = oldFrame->host();
+
+    // Frame::detach will call clearForClose(), we need to call
+    // clearForNavigate() before detaching.
+    oldFrame->prepareSwapFrom(oldFrame.get());
+
+    oldFrame->detach(FrameDetachType::Swap);
+
     if (m_parent) {
         if (m_parent->m_firstChild == this)
             m_parent->m_firstChild = frame;
@@ -86,8 +102,6 @@ bool WebFrame::swap(WebFrame* frame)
     // 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());
@@ -103,7 +117,7 @@ bool WebFrame::swap(WebFrame* frame)
             localFrame.page()->setMainFrame(&localFrame);
         }
     } else {
-        toWebRemoteFrameImpl(frame)->initializeCoreFrame(oldFrame->host(), owner, oldFrame->tree().name());
+        toWebRemoteFrameImpl(frame)->initializeCoreFrame(host, owner, oldFrame->tree().name());
     }
     toCoreFrame(frame)->finishSwapFrom(oldFrame.get());
 
@@ -112,7 +126,7 @@ bool WebFrame::swap(WebFrame* frame)
 
 void WebFrame::detach()
 {
-    toCoreFrame(this)->detach();
+    toCoreFrame(this)->detach(FrameDetachType::Remove);
 }
 
 WebSecurityOrigin WebFrame::securityOrigin() const