BrowserPlugin/WebView - Move plugin lifetime to DOM

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 20774db3cc50b436864598b568014e034aa8c0d2..3339e2bd0f168687448ce4752485cf0bc6fb8abc 100644
--- a/Source/core/html/HTMLFrameOwnerElement.cpp
+++ b/Source/core/html/HTMLFrameOwnerElement.cpp
@@ -23,6 +23,7 @@
 
 #include "bindings/v8/ExceptionMessages.h"
 #include "bindings/v8/ExceptionState.h"
+#include "core/accessibility/AXObjectCache.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/loader/FrameLoader.h"
 #include "core/loader/FrameLoaderClient.h"
@@ -35,6 +36,54 @@
 
 namespace WebCore {
 
+typedef HashMap<RefPtr<Widget>, FrameView*> WidgetToParentMap;
+static WidgetToParentMap& widgetNewParentMap()
+{
+    DEFINE_STATIC_LOCAL(WidgetToParentMap, map, ());
+    return map;
+}
+
+static unsigned s_updateSuspendCount = 0;
+
+HTMLFrameOwnerElement::UpdateSuspendScope::UpdateSuspendScope()
+{
+    ++s_updateSuspendCount;
+}
+
+HTMLFrameOwnerElement::UpdateSuspendScope::~UpdateSuspendScope()
+{
+    ASSERT(s_updateSuspendCount > 0);
+    if (s_updateSuspendCount == 1) {
+        WidgetToParentMap map;
+        widgetNewParentMap().swap(map);
+        WidgetToParentMap::iterator end = map.end();
+        for (WidgetToParentMap::iterator it = map.begin(); it != end; ++it) {
+            Widget* child = it->key.get();
+            ScrollView* currentParent = toScrollView(child->parent());
+            FrameView* newParent = it->value;
+            if (newParent != currentParent) {
+                if (currentParent)
+                    currentParent->removeChild(child);
+                if (newParent)
+                    newParent->addChild(child);
+            }
+        }
+    }
+    --s_updateSuspendCount;
+}
+
+static void moveWidgetToParentSoon(Widget* child, FrameView* parent)
+{
+    if (!s_updateSuspendCount) {
+        if (parent)
+            parent->addChild(child);
+        else if (toScrollView(child->parent()))
+            toScrollView(child->parent())->removeChild(child);
+        return;
+    }
+    widgetNewParentMap().set(child, parent);
+}
+
 HTMLFrameOwnerElement::HTMLFrameOwnerElement(const QualifiedName& tagName, Document& document)
     : HTMLElement(tagName, document)
     , m_contentFrame(0)
@@ -121,6 +170,52 @@ SVGDocument* HTMLFrameOwnerElement::getSVGDocument(ExceptionState& exceptionStat
     return 0;
 }
 
+void HTMLFrameOwnerElement::setWidget(PassRefPtr<Widget> widget)
+{
+    if (widget == m_widget)
+        return;
+
+    if (m_widget) {
+        if (m_widget->parent())
+            moveWidgetToParentSoon(m_widget.get(), 0);
+        m_widget = 0;
+    }
+
+    m_widget = widget;
+
+    RenderWidget* renderWidget = toRenderWidget(renderer());
+    if (!renderWidget || !renderWidget->isWidget())

[eseidel, 2013/12/12 20:40:13]

What does isWidget() do here?

[wjmaclean, 2013/12/12 22:31:15]

I'm not entirely sure, but even RenderWidget() was making this check, so I was
loathe to remove it. I will try to remove it and see what happens.

+        return;
+
+    if (m_widget) {
+        if (renderWidget->style()) {

[eseidel, 2013/12/12 20:40:13]

It feels liek this wants to call some method on the renderer.  The rest of this
function looks like code which belongs in the rendering tree, if still called
from this DOM.

[wjmaclean, 2013/12/12 22:31:15]

OK.

+            if (!renderWidget->needsLayout())
+                renderWidget->updateWidgetGeometry();
+
+            if (renderWidget->style()->visibility() != VISIBLE) {

[eseidel, 2013/12/12 20:40:13]

This all seems suspect.  The DOM shouldn't be dealing with things like
repainting.

[wjmaclean, 2013/12/12 22:31:15]

Is it ok if dealt with as described above? Or do we need to call something like
needsRepaint() on the renderer?

+                m_widget->hide();
+            } else {
+                m_widget->show();
+                renderWidget->repaint();
+            }
+        }
+        ASSERT(document().view() == renderWidget->frameView());
+        ASSERT(renderWidget->frameView());
+        moveWidgetToParentSoon(m_widget.get(), renderWidget->frameView());
+    }
+
+    if (AXObjectCache* cache = document().existingAXObjectCache())
+        cache->childrenChanged(renderWidget);
+
+    if (RenderPart* renderPart = toRenderPart(renderWidget))
+        renderPart->viewCleared();
+}
+
+Widget* HTMLFrameOwnerElement::widget() const
+{
+    return m_widget.get();
+}
+
 bool HTMLFrameOwnerElement::loadOrRedirectSubframe(const KURL& url, const AtomicString& frameName, bool lockBackForwardList)
 {
     RefPtr<Frame> parentFrame = document().frame();
@@ -153,10 +248,12 @@ bool HTMLFrameOwnerElement::loadOrRedirectSubframe(const KURL& url, const Atomic
     // FIXME: Can we remove this entirely? m_isComplete normally gets set to false when a load is committed.
     childFrame->loader().started();
 
-    RenderObject* renderObject = renderer();
     FrameView* view = childFrame->view();
+    RenderObject* renderObject = renderer();
+    // We need to test the existence of renderObject and its widget-ness, as
+    // failing to do so causes problems.
     if (renderObject && renderObject->isWidget() && view)
-        toRenderWidget(renderObject)->setWidget(view);
+        setWidget(view);
 
     // Some loads are performed synchronously (e.g., about:blank and loads
     // cancelled by returning a null ResourceRequest from requestFromDelegate).