Streamline frame detach

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 55056c63b5c9dc03dc855c24cc1339d83a7178b2..52b915921cbf2fdde639a4ec5be572e99b76ddda 100644
--- a/Source/core/loader/FrameLoader.cpp
+++ b/Source/core/loader/FrameLoader.cpp
@@ -1088,10 +1088,9 @@ void FrameLoader::restoreScrollPositionAndViewState()
 void FrameLoader::checkLoadComplete()
 {
     ASSERT(client()->hasWebView());
-    if (Page* page = m_frame->page()) {
-        if (page->mainFrame()->isLocalFrame())
-            page->deprecatedLocalMainFrame()->loader().checkLoadCompleteForThisFrame();
-    }
+    Frame* top = m_frame->tree().top();

[dcheng, 2014/09/25 23:35:43]

Was this a necessary change or just a cleanup change?

[Nate Chapin, 2014/09/25 23:57:50]

I think it might have been necessary as a result of a mistake that I then later
fixed? I can't remember anymore :(

I'll revert it for now.

+    if (top->isLocalFrame())
+        toLocalFrame(top)->loader().checkLoadCompleteForThisFrame();
 }
 
 String FrameLoader::userAgent(const KURL& url) const
@@ -1101,60 +1100,21 @@ String FrameLoader::userAgent(const KURL& url) const
     return userAgent;
 }
 
-void FrameLoader::detachFromParent()
+void FrameLoader::detach()
 {
 #if !ENABLE(OILPAN)
     // The caller must protect a reference to m_frame.
     ASSERT(m_frame->refCount() > 1);
 #endif
-
-    InspectorInstrumentation::frameDetachedFromParent(m_frame);
-
     if (m_documentLoader)
         m_documentLoader->detachFromFrame();
     m_documentLoader = nullptr;
 
-    if (!client())
-        return;
-
-    // FIXME: All this code belongs up in Page.
     Frame* parent = m_frame->tree().parent();
-    if (parent && parent->isLocalFrame()) {
-        m_frame->setView(nullptr);
-        // FIXME: Shouldn't need to check if page() is null here.
-        if (m_frame->owner() && m_frame->page())
-            m_frame->page()->decrementSubframeCount();
-        m_frame->willDetachFrameHost();
-        detachClient();
+    if (parent && parent->isLocalFrame())
         toLocalFrame(parent)->loader().scheduleCheckCompleted();
-    } else {
-        m_frame->setView(nullptr);
-        m_frame->willDetachFrameHost();
-        detachClient();
-    }
-    m_frame->detachFromFrameHost();
-}
-
-void FrameLoader::detachClient()
-{
-    ASSERT(client());
-
-    // Finish all cleanup work that might require talking to the embedder.
     m_progressTracker.clear();
     setOpener(0);
-    // Notify ScriptController that the frame is closing, since its cleanup ends up calling
-    // back to FrameLoaderClient via WindowProxy.
-    m_frame->script().clearForClose();
-
-    // client() should never be null because that means we somehow re-entered
-    // the frame detach code... but it is sometimes.
-    // FIXME: Understand why this is happening so we can document this insanity.
-    if (client()) {
-        // After this, we must no longer talk to the client since this clears
-        // its owning reference back to our owning LocalFrame.
-        client()->detachedFromParent();
-        m_frame->clearClient();
-    }
 }
 
 void FrameLoader::receivedMainResourceError(const ResourceError& error)