Invert the owning relationship between WebFrame and Frame.

Source/web/WebFrameImpl.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 17 matching lines @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 // How ownership works
 // -------------------
 //
 // Big oh represents a refcounted relationship: owner O--- ownee
 //
 // WebView (for the toplevel frame only)
 //    O
-//    |
+//    |           WebFrame
+//    |              O
+//    |              |
 //   Page O------- Frame (m_mainFrame) O-------O FrameView
 //                   ||
 //                   ||
-//               FrameLoader O-------- WebFrame (via FrameLoaderClient)
+//               FrameLoader
 //
 // FrameLoader and Frame are formerly one object that was split apart because
 // it got too big. They basically have the same lifetime, hence the double line.
 //
-// WebFrame is refcounted and has one ref on behalf of the FrameLoader/Frame.
-// This is not a normal reference counted pointer because that would require
-// changing WebKit code that we don't control. Instead, it is created with this
-// ref initially and it is removed when the FrameLoader is getting destroyed.
-//
-// WebFrames are created in two places, first in WebViewImpl when the root
-// frame is created, and second in WebFrame::createChildFrame when sub-frames
-// are created. WebKit will hook up this object to the FrameLoader/Frame
-// and the refcount will be correct.
+// From the perspective of the embedder, WebFrame is simply an object that it
+// allocates by calling WebFrame::create() and must be freed by calling close().
+// Internally, WebFrame is actually refcounted and it holds a reference to its
+// corresponding Frame in WebCore.
 //
 // How frames are destroyed
 // ------------------------
 //
 // The main frame is never destroyed and is re-used. The FrameLoader is re-used
 // and a reference to the main frame is kept by the Page.
 //
 // When frame content is replaced, all subframes are destroyed. This happens
-// in FrameLoader::detachFromParent for each subframe.
-//
-// Frame going away causes the FrameLoader to get deleted. In FrameLoader's
-// destructor, it notifies its client with frameLoaderDestroyed. This derefs
-// the WebFrame and will cause it to be deleted (unless an external someone
-// is also holding a reference).
+// in FrameLoader::detachFromParent for each subframe. detachFromParent()

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

Might be nice to know the call order of detachFromParent.  Presumbly its from
the subtree-root up? i.e. recursive?

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

Done.

+// calls FrameLoaderClient::detachedFromParent(), which calls
+// WebFrame::frameDetached(). This triggers WebFrame to clear its reference to
+// Frame, and also notifies the embedder via WebFrameClient that the frame is
+// detached. Most embedders will invoke close() on the WebFrame at this point,
+// triggering its deletion unless something else is still retaining a reference.
 //
 // Thie client is expected to be set whenever the WebFrameImpl is attached to
 // the DOM.
 
 #include "config.h"
 #include "WebFrameImpl.h"
 
 #include <algorithm>
 #include "AssociatedURLLoader.h"
 #include "DOMUtilitiesPrivate.h"
@@ skipping 2013 matching lines @@
 {
     return WebFrameImpl::create(client, generateEmbedderIdentifier());
 }
 
 WebFrameImpl* WebFrameImpl::create(WebFrameClient* client, long long embedderIdentifier)
 {
     return adoptRef(new WebFrameImpl(client, embedderIdentifier)).leakRef();
 }
 
 WebFrameImpl::WebFrameImpl(WebFrameClient* client, long long embedderIdentifier)
-    : FrameDestructionObserver(0)
-    , m_frameInit(WebFrameInit::create(this, embedderIdentifier))
+    : m_frameInit(WebFrameInit::create(this, embedderIdentifier))
     , m_client(client)
     , m_permissionClient(0)
     , m_currentActiveMatchFrame(0)
     , m_activeMatchIndexInCurrentFrame(-1)
     , m_locatingActiveRect(false)
     , m_resumeScopingFromRange(0)
     , m_lastMatchCount(-1)
     , m_totalMatchCount(-1)
     , m_framesScopingCount(-1)
     , m_findRequestIdentifier(-1)
     , m_scopingInProgress(false)
     , m_lastFindRequestCompletedWithNoMatches(false)
     , m_nextInvalidateAfter(0)
     , m_findMatchMarkersVersion(0)
     , m_findMatchRectsAreValid(false)
     , m_inputEventsScaleFactorForEmulation(1)
 {
     blink::Platform::current()->incrementStatsCounter(webFrameActiveCount);
     frameCount++;
 }
 
 WebFrameImpl::~WebFrameImpl()
 {
     blink::Platform::current()->decrementStatsCounter(webFrameActiveCount);
     frameCount--;
 
     cancelPendingScopingEffort();
 }
 
-void WebFrameImpl::setWebCoreFrame(WebCore::Frame* frame)
+void WebFrameImpl::setWebCoreFrame(PassRefPtr<WebCore::Frame> frame)
 {
-    ASSERT(frame);
-    observeFrame(frame);
+    m_frame = frame;
 }
 
 void WebFrameImpl::initializeAsMainFrame(WebCore::Page* page)
 {
-    // FIXME: This whole function can go away once ownerhip of WebFrame is reversed.
-    // Page should create it's main WebFrame, not have FrameLoader do it only
-    // to have to mark the frame as main later.
     m_frameInit->setFrameHost(&page->frameHost());
-    RefPtr<Frame> mainFrame = Frame::create(m_frameInit);
-    setWebCoreFrame(mainFrame.get());
-
-    // Add reference on behalf of FrameLoader. See comments in
-    // WebFrameLoaderClient::frameLoaderDestroyed for more info.
-    ref();
+    setWebCoreFrame(Frame::create(m_frameInit));
 
     // We must call init() after m_frame is assigned because it is referenced
     // during init().
-    frame()->init();
+    m_frame->init();
 }
 
 PassRefPtr<Frame> WebFrameImpl::createChildFrame(const FrameLoadRequest& request, HTMLFrameOwnerElement* ownerElement)
 {
     ASSERT(m_client);
     WebFrameImpl* webframe = toWebFrameImpl(m_client->createChildFrame(this, request.frameName()));
 
-    // Add an extra ref on behalf of the page/FrameLoader, which references the
-    // WebFrame via the FrameLoaderClient interface. See the comment at the top
-    // of this file for more info.
-    webframe->ref();
-
     webframe->m_frameInit->setFrameHost(frame()->host());
     webframe->m_frameInit->setOwnerElement(ownerElement);
     RefPtr<Frame> childFrame = Frame::create(webframe->m_frameInit);
-    webframe->setWebCoreFrame(childFrame.get());
+    webframe->setWebCoreFrame(childFrame);
 
     childFrame->tree().setName(request.frameName());
 
     frame()->tree().appendChild(childFrame);
 
     // Frame::init() can trigger onload event in the parent frame,
     // which may detach this frame and trigger a null-pointer access
     // in FrameTree::removeChild. Move init() after appendChild call
     // so that webframe->mFrame is in the tree before triggering
     // onload event handler.
@@ skipping 310 matching lines @@
     ScriptValue result = frame()->script().executeScriptInMainWorldAndReturnValue(ScriptSourceCode(script));
 
     String scriptResult;
     if (!result.getString(scriptResult))
         return;
 
     if (!frame()->navigationScheduler().locationChangePending())
         frame()->document()->loader()->replaceDocument(scriptResult, ownerDocument.get());
 }
 
-void WebFrameImpl::willDetachFrameHost()
+void WebFrameImpl::willDetachParent()
 {
-    // FIXME: This should never be called if the Frame has already been detached?
-    if (!frame() || !frame()->page())
-        return;
-
     // Do not expect string scoping results from any frames that got detached
     // in the middle of the operation.
     if (m_scopingInProgress) {
 
         // There is a possibility that the frame being detached was the only
         // pending one. We need to make sure final replies can be sent.
         flushCurrentScopingEffort(m_findRequestIdentifier);
 
         cancelPendingScopingEffort();
     }
 }
 
 } // namespace blink