RemoteBridgeFrameOwner: keep better track of the attached frame

third_party/WebKit/Source/web/WebFrame.cpp

Index: third_party/WebKit/Source/web/WebFrame.cpp
diff --git a/third_party/WebKit/Source/web/WebFrame.cpp b/third_party/WebKit/Source/web/WebFrame.cpp
index 8f185679f5004756b27b28c4b10bb6ca20d7c6b5..959255bdd566ce3b4819674c92d5bbfb0dc1dfc8 100644
--- a/third_party/WebKit/Source/web/WebFrame.cpp
+++ b/third_party/WebKit/Source/web/WebFrame.cpp
@@ -72,7 +72,6 @@ bool WebFrame::swap(WebFrame* frame)
     AtomicString name = oldFrame->tree().name();
     AtomicString uniqueName = oldFrame->tree().uniqueName();
     FrameOwner* owner = oldFrame->owner();

[dcheng, 2016/03/16 09:26:18]

Note: there's actually a rather embarrassing leak. A remote frame maintains a
map of child frames to frame owners, to keep the frame owners alive.
Unfortunately, this map does /not/ get updated on swap.

If non-Oilpan support could be removed, fixing this would be easy: I would just
get rid of the map and use a self-persistent that gets set/unset in
setContentFrame/clearContentFrame, and rely on the stack pointer here to keep it
live in swap().

Unfortunately, since I still need to support non-Oilpan, it gets trickier. I
can't just make this a RefPtrWillBeRawPtr<FrameOwner> because FrameOwner is a
mixin and is a base class of an already refcounted class. I plan on addressing
this in a followup.

-    oldFrame->disconnectOwnerElement();
 
     v8::HandleScope handleScope(v8::Isolate::GetCurrent());
     HashMap<DOMWrapperWorld*, v8::Local<v8::Object>> globals;
@@ -83,21 +82,22 @@ bool WebFrame::swap(WebFrame* frame)
     // associated with the frame itself have not yet been freed yet.
     oldFrame->detach(FrameDetachType::Swap);
 
-    // Finally, clone the state of the current Frame into one matching
-    // the type of the passed in WebFrame.
+    // Clone the state of the current Frame into the one being swapped in.
     // FIXME: This is a bit clunky; this results in pointless decrements and
     // increments of connected subframes.
     if (frame->isWebLocalFrame()) {
+        // TODO(dcheng): in an ideal world, both branches would just use
+        // WebFrameImplBase's initializeCoreFrame() helper. However, Blink
+        // currently requires a 'provisional' local frame to serve as a
+        // placeholder for loading state when swapping to a local frame.
+        // In this case, the core LocalFrame is already initialized, so just
+        // update a bit of state.
         LocalFrame& localFrame = *toWebLocalFrameImpl(frame)->frame();
         ASSERT(owner == localFrame.owner());
         if (owner) {
-            if (owner->isLocal()) {
-                HTMLFrameOwnerElement* ownerElement = toHTMLFrameOwnerElement(owner);
-                ownerElement->setContentFrame(localFrame);
-                ownerElement->setWidget(localFrame.view());
-            } else {
-                toRemoteBridgeFrameOwner(owner)->setContentFrame(toWebLocalFrameImpl(frame));
-            }
+            owner->setContentFrame(localFrame);
+            if (owner->isLocal())
+                toHTMLFrameOwnerElement(owner)->setWidget(localFrame.view());
         } else {
             localFrame.page()->setMainFrame(&localFrame);
         }