Invert the owning relationship between WebFrame and Frame.

Source/core/loader/FrameLoader.cpp

Index: Source/core/loader/FrameLoader.cpp
diff --git a/Source/core/loader/FrameLoader.cpp b/Source/core/loader/FrameLoader.cpp
index 4b84a785c5f88982648dffc563ad047c68ed20ad..bb990911ee4ceddd9024d58a607100becdf58809 100644
--- a/Source/core/loader/FrameLoader.cpp
+++ b/Source/core/loader/FrameLoader.cpp
@@ -159,13 +159,9 @@ FrameLoader::FrameLoader(Frame* frame, FrameLoaderClient* client)
 
 FrameLoader::~FrameLoader()
 {
-    setOpener(0);
-
     HashSet<Frame*>::iterator end = m_openedFrames.end();
     for (HashSet<Frame*>::iterator it = m_openedFrames.begin(); it != end; ++it)
         (*it)->loader().m_opener = 0;
-
-    m_client->frameLoaderDestroyed();
 }
 
 void FrameLoader::init()
@@ -418,7 +414,8 @@ void FrameLoader::finishedParsing()
     // Null-checking the FrameView indicates whether or not we're in the destructor.
     RefPtr<Frame> protector = m_frame->view() ? m_frame : 0;
 
-    m_client->dispatchDidFinishDocumentLoad();
+    if (m_client)
+        m_client->dispatchDidFinishDocumentLoad();
 
     checkCompleted();
 
@@ -823,7 +820,10 @@ void FrameLoader::stopAllLoaders()
 
     m_inStopAllLoaders = false;
 
-    m_client->didStopAllLoaders();
+    // detachFromParent() can be called multiple times on same Frame, which
+    // means we may no longer have a FrameLoaderClient to talk to.
+    if (m_client)
+        m_client->didStopAllLoaders();
 }
 
 DocumentLoader* FrameLoader::activeDocumentLoader() const
@@ -1125,10 +1125,12 @@ void FrameLoader::detachFromParent()
     if (m_documentLoader)
         m_documentLoader->detachFromFrame();
     m_documentLoader = 0;
-    m_client->detachedFromParent();
 
     m_progressTracker.clear();
 
+    if (m_client)
+        m_client->willDetachParent();
+
     // FIXME: All this code belongs up in Page.
     if (Frame* parent = m_frame->tree().parent()) {
         parent->loader().closeAndRemoveChild(m_frame);
@@ -1138,6 +1140,22 @@ void FrameLoader::detachFromParent()
         m_frame->willDetachFrameHost();
         m_frame->detachFromFrameHost();
     }
+
+    if (m_client) {
+        setOpener(0);
+
+        // Close down the proxy. The purpose of this change is to make the call to

[eseidel, 2013/12/28 01:18:40]

I think you mean "call" instead of change.

[dcheng, 2013/12/28 01:51:05]

I don't have any clue what the original author meant (this was moved verbatim
from FrameLoaderClientImpl).

+        // ScriptController::clearWindowShell a no-op when called from Frame::pageDestroyed.
+        // Without this change, this call to clearWindowShell will cause a crash. If you

[eseidel, 2013/12/28 01:18:40]

clearWindowShell is called by clearForClose? or did you mean clearForClose?

[dcheng, 2013/12/28 01:51:05]

This comment seems to have suffered from significant bitrot. For example, there
is no such thing as Frame::pageDestroyed() anymore. I've rewritten it so
hopefully it makes sense now.

+        // remove/modify this, just ensure that you can go to a page and then navigate to a new
+        // page without getting any asserts or crashes.
+        m_frame->script().clearForClose();
+
+        // After this, we must no longer talk to the client since this clears
+        // its owning reference back to us.
+        m_client->detachedFromParent();
+        m_client = 0;
+    }
 }
 
 void FrameLoader::addHTTPOriginIfNeeded(ResourceRequest& request, const AtomicString& origin)
@@ -1290,6 +1308,10 @@ void FrameLoader::loadWithNavigationAction(const NavigationAction& action, Frame
     if (!m_stateMachine.committedFirstRealDocumentLoad() && m_frame->ownerElement() && !m_frame->ownerElement()->dispatchBeforeLoadEvent(request.url().string()))
         return;
 
+    // Dispatching the beforeload event could have blown away the frame.
+    if (!m_client)
+        return;
+
     if (!m_stateMachine.startedFirstRealLoad())
         m_stateMachine.advanceTo(FrameLoaderStateMachine::StartedFirstRealLoad);