Move the Widget hierarchy to the Oilpan heap.

Source/core/html/HTMLFrameOwnerElement.cpp

Index: Source/core/html/HTMLFrameOwnerElement.cpp
diff --git a/Source/core/html/HTMLFrameOwnerElement.cpp b/Source/core/html/HTMLFrameOwnerElement.cpp
index 175b6ac0d4948abbbe4b1824ab5eece18d39dac8..e987c6ecbf18547f054724adeeb252d0f290252a 100644
--- a/Source/core/html/HTMLFrameOwnerElement.cpp
+++ b/Source/core/html/HTMLFrameOwnerElement.cpp
@@ -30,6 +30,7 @@
 #include "core/frame/LocalFrame.h"
 #include "core/loader/FrameLoader.h"
 #include "core/loader/FrameLoaderClient.h"
+#include "core/plugins/PluginView.h"
 #include "core/rendering/RenderLayer.h"
 #include "core/rendering/RenderPart.h"
 #include "core/rendering/compositing/RenderLayerCompositor.h"
@@ -38,11 +39,11 @@
 
 namespace blink {
 
-typedef HashMap<RefPtr<Widget>, FrameView*> WidgetToParentMap;
+typedef WillBeHeapHashMap<RefPtrWillBeMember<Widget>, RawPtrWillBeMember<FrameView> > WidgetToParentMap;
 static WidgetToParentMap& widgetNewParentMap()
 {
-    DEFINE_STATIC_LOCAL(WidgetToParentMap, map, ());
-    return map;
+    DEFINE_STATIC_LOCAL(OwnPtrWillBePersistent<WidgetToParentMap>, map, (adoptPtrWillBeNoop(new WidgetToParentMap())));
+    return *map;
 }
 
 WillBeHeapHashCountedSet<RawPtrWillBeMember<Node> >& SubframeLoadingDisabler::disabledSubtreeRoots()
@@ -72,6 +73,10 @@ void HTMLFrameOwnerElement::UpdateSuspendScope::performDeferredWidgetTreeOperati
                 currentParent->removeChild(child);
             if (newParent)
                 newParent->addChild(child);
+#if ENABLE(OILPAN)
+            if (currentParent && !newParent)
+                HTMLFrameOwnerElement::disposeWidget(child);
+#endif
         }
     }
 }
@@ -87,10 +92,14 @@ HTMLFrameOwnerElement::UpdateSuspendScope::~UpdateSuspendScope()
 static void moveWidgetToParentSoon(Widget* child, FrameView* parent)
 {
     if (!s_updateSuspendCount) {
-        if (parent)
+        if (parent) {
             parent->addChild(child);
-        else if (toScrollView(child->parent()))
+        } else if (toScrollView(child->parent())) {
             toScrollView(child->parent())->removeChild(child);
+#if ENABLE(OILPAN)
+            HTMLFrameOwnerElement::disposeWidget(child);
+#endif
+        }
         return;
     }
     widgetNewParentMap().set(child, parent);
@@ -144,17 +153,12 @@ void HTMLFrameOwnerElement::disconnectContentFrame()
     // see if this behavior is really needed as Gecko does not allow this.
     if (RefPtrWillBeRawPtr<Frame> frame = contentFrame()) {
         frame->detach();
-#if ENABLE(OILPAN)
-        // FIXME: Oilpan: the plugin container is released and finalized here
-        // in order to work around the current inability to make the plugin
-        // container a FrameDestructionObserver (it needs to effectively be on
-        // the heap, and Widget isn't). Hence, release it here while its
-        // frame reference is still valid.
-        if (m_widget && m_widget->isPluginContainer())
-            m_widget = nullptr;
-#endif
         frame->disconnectOwnerElement();
     }
+#if ENABLE(OILPAN)
+    disposeWidget(m_widget.get());
+    m_widget = nullptr;
+#endif
 }
 
 HTMLFrameOwnerElement::~HTMLFrameOwnerElement()
@@ -169,6 +173,18 @@ HTMLFrameOwnerElement::~HTMLFrameOwnerElement()
 #endif
 }
 
+#if ENABLE(OILPAN)
+void HTMLFrameOwnerElement::disposeWidget(Widget* widget)

[haraken, 2014/09/29 14:16:36]

Can we avoid adding this static method by defining virtual Widget::dispose() and
overriding it in PluginView ?

Instead of calling HTMLFrameOwnerElement::disposeWidget(child), you can just
call child->dispose().

[sof, 2014/10/07 11:03:36]

ok, let's do that instead (as we've reduced our 'virtual' budget on Widget by
one here already.)

+{
+    // Oilpan: a plugin container must be disposed of prior to (GC)

[haraken, 2014/09/29 14:16:36]

I don't fully understand this well, but what happens if we introduce:

  PersistentHeapHashMap<WeakMember<Widget>, OwnPtr<WidgetDisposer>>;

?

Then can we avoid introducing Widget::dispose()?

[sof, 2014/10/07 11:03:36]

This suggestion doesn't seem to address what the comment tries to explain must
hold when finalizing the plugin (container) -- that it needs to happen while the
frame (and all that it contains) is still alive & not being finalized. Because
it might perform an unbounded&unknown amount of work and operations on those
objects.

+    // finalization of it and all it depends on. Needed to ensure that
+    // the underlying plugin is able to access a fully-functioning
+    // frame while it destructs.
+    if (widget && widget->isPluginView())

[haraken, 2014/09/29 14:16:36]

Is it possible that this method is called with a null widget?

[sof, 2014/10/07 11:03:36]

As it is currently used, yes.

+        toPluginView(widget)->dispose();
+}
+#endif
+
 Document* HTMLFrameOwnerElement::contentDocument() const
 {
     return (m_contentFrame && m_contentFrame->isLocalFrame()) ? toLocalFrame(m_contentFrame)->document() : 0;
@@ -202,7 +218,7 @@ Document* HTMLFrameOwnerElement::getSVGDocument(ExceptionState& exceptionState)
     return 0;
 }
 
-void HTMLFrameOwnerElement::setWidget(PassRefPtr<Widget> widget)
+void HTMLFrameOwnerElement::setWidget(PassRefPtrWillBeRawPtr<Widget> widget)
 {
     if (widget == m_widget)
         return;
@@ -259,6 +275,7 @@ bool HTMLFrameOwnerElement::loadOrRedirectSubframe(const KURL& url, const Atomic
 void HTMLFrameOwnerElement::trace(Visitor* visitor)
 {
     visitor->trace(m_contentFrame);
+    visitor->trace(m_widget);
     HTMLElement::trace(visitor);
     FrameOwner::trace(visitor);
 }