Invert the owning relationship between WebFrame and Frame.

Source/web/FrameLoaderClientImpl.cpp

Index: Source/web/FrameLoaderClientImpl.cpp
diff --git a/Source/web/FrameLoaderClientImpl.cpp b/Source/web/FrameLoaderClientImpl.cpp
index b9c348639719e4f7c955c0d69125252da1810bd8..f83193876723573a3b97590186a1c30b154785eb 100644
--- a/Source/web/FrameLoaderClientImpl.cpp
+++ b/Source/web/FrameLoaderClientImpl.cpp
@@ -110,17 +110,6 @@ FrameLoaderClientImpl::~FrameLoaderClientImpl()
 {
 }
 
-void FrameLoaderClientImpl::frameLoaderDestroyed()
-{
-    // When the WebFrame was created, it had an extra reference given to it on
-    // behalf of the Frame.  Since the WebFrame owns us, this extra ref also
-    // serves to keep us alive until the FrameLoader is done with us.  The
-    // FrameLoader calls this method when it's going away.  Therefore, we balance
-    // out that extra reference, which may cause 'this' to be deleted.
-    ASSERT(!m_webFrame->frame());
-    m_webFrame->deref();
-}
-
 void FrameLoaderClientImpl::dispatchDidClearWindowObjectInWorld(DOMWrapperWorld*)
 {
     if (m_webFrame->client()) {
@@ -253,24 +242,28 @@ bool FrameLoaderClientImpl::hasFrameView() const
     return m_webFrame->viewImpl();
 }
 
-void FrameLoaderClientImpl::detachedFromParent()
+void FrameLoaderClientImpl::willDetachParent()

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

What's "parent" in this case?  The page?  The parent frame?  Is this called for
the main frame?

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

I can't think of a better name for this unfortunately. It's called for any
frame, regardless of whether or not its the main frame. This mirrors the naming
convention of FrameLoader::detachFromParent... perhaps a followup patch to think
of more apt names is in order.

 {
-    // Close down the proxy.  The purpose of this change is to make the
-    // call to ScriptController::clearWindowShell a no-op when called from
-    // Frame::pageDestroyed.  Without this change, this call to clearWindowShell
-    // will cause a crash.  If you 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_webFrame->frame()->script().clearForClose();
+    m_webFrame->willDetachParent();
+}
 
+void FrameLoaderClientImpl::detachedFromParent()
+{
     // Alert the client that the frame is being detached. This is the last

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

Is this still the last chance? or are we completely done already?  I would think
the willDetach method would be the last chance and this would be after our
chance is over?

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

This is the last chance because the final line of this function is what
(typically) triggers the embedder to release its reference to the WebFrame (and
consequently, this object).

     // chance we have to communicate with the client.
-    if (m_webFrame->client())
-        m_webFrame->client()->frameDetached(m_webFrame);
+    m_webFrame->setWebCoreFrame(0);
 
-    // Stop communicating with the WebFrameClient at this point since we are no
-    // longer associated with the Page.
+    // Signal that no further communication with WebFrameClient should take
+    // place at this point since we are no longer associated with the Page.
+    WebFrameClient* client = m_webFrame->client();
     m_webFrame->setClient(0);
+
+    // frameDetached() may release the final reference to WebFrame. Since we are

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

I'm confused by this comment.  The goal is to clear our m_webFrame pointer
before we tell WebFrameClient that we're detached just in case it calls back to
us and tries to talk to a dead m_webFrame?

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

I've tried to clarify this comment. Basically, calling
WebFrameClient::frameDetached() typically results in destruction of this.

+    // owned by WebFrame, we can't reference any members after this point, so
+    // clear WebFrame now.
+    WebFrame* webFrame = m_webFrame;
+    m_webFrame = 0;
+    client->frameDetached(webFrame);
 }
 
 void FrameLoaderClientImpl::dispatchWillRequestAfterPreconnect(ResourceRequest& request)