BrowserPlugin/WebView - Move plugin lifetime to DOM

Source/core/rendering/RenderWidget.cpp

Index: Source/core/rendering/RenderWidget.cpp
diff --git a/Source/core/rendering/RenderWidget.cpp b/Source/core/rendering/RenderWidget.cpp
index 93c4daced00f2ee26e8265924711a9a8a32d8c1c..7ae120983d91c1fa9c1422ca8180ce9ed73e7267 100644
--- a/Source/core/rendering/RenderWidget.cpp
+++ b/Source/core/rendering/RenderWidget.cpp
@@ -26,6 +26,7 @@
 
 #include "core/accessibility/AXObjectCache.h"
 #include "core/frame/Frame.h"
+#include "core/html/HTMLPlugInElement.h"
 #include "core/platform/graphics/GraphicsContext.h"
 #include "core/rendering/CompositedLayerMapping.h"
 #include "core/rendering/GraphicsContextAnnotator.h"
@@ -127,6 +128,19 @@ RenderWidget::~RenderWidget()
     clearWidget();
 }
 
+Widget* RenderWidget::widget() const
+{
+    if (m_widget.get())

[eseidel, 2013/11/18 22:09:54]

Can we just remove RenderWidget's m_widget pointer?  You can't have anonymous
renderers with plugins to my knowledge, so can't we just remove this check?

[wjmaclean, 2013/11/25 17:51:56]

I've taken a stab at this .. please take a look and let me know if anything
looks overly naive.

Done.

+        return m_widget.get();
+
+    // Plugin widgets are now stored in their DOM node, so check there next.
+    Element* element = toElement(node());
+    if (element && element->isPluginElement())
+        return toHTMLPlugInElement(element)->plugin().get();
+
+    return 0;
+}
+
 // Widgets are always placed on integer boundaries, so rounding the size is actually
 // the desired behavior. This function is here because it's otherwise seldom what we
 // want to do with a LayoutRect.
@@ -140,10 +154,13 @@ bool RenderWidget::setWidgetGeometry(const LayoutRect& frame)
     if (!node())
         return false;
 
+    Widget* widget = this->widget();
+    ASSERT(widget);
+
     IntRect clipRect = roundedIntRect(enclosingLayer()->childrenClipRect());
     IntRect newFrame = roundedIntRect(frame);
     bool clipChanged = m_clipRect != clipRect;
-    bool frameRectChanged = m_widget->frameRect() != newFrame;
+    bool frameRectChanged = widget->frameRect() != newFrame;
 
     if (!frameRectChanged && !clipChanged)
         return false;
@@ -152,23 +169,25 @@ bool RenderWidget::setWidgetGeometry(const LayoutRect& frame)
 
     RefPtr<RenderWidget> protector(this);
     RefPtr<Node> protectedNode(node());
-    m_widget->setFrameRect(newFrame);
+    widget->setFrameRect(newFrame);
 
     if (clipChanged && !frameRectChanged)
-        m_widget->clipRectChanged();
+        widget->clipRectChanged();
 
     if (hasLayer() && layer()->compositingState() == PaintsIntoOwnBacking)
         layer()->compositedLayerMapping()->updateAfterWidgetResize();
 
-    bool boundsChanged = m_widget->frameRect().size() != newFrame.size();
+    bool boundsChanged = widget->frameRect().size() != newFrame.size();
     return boundsChanged;
 }
 
 bool RenderWidget::updateWidgetGeometry()
 {
+    Widget* widget = this->widget();
+
     LayoutRect contentBox = contentBoxRect();
     LayoutRect absoluteContentBox(localToAbsoluteQuad(FloatQuad(contentBox)).boundingBox());
-    if (m_widget->isFrameView()) {
+    if (widget->isFrameView()) {
         contentBox.setLocation(absoluteContentBox.location());
         return setWidgetGeometry(contentBox);
     }
@@ -176,6 +195,9 @@ bool RenderWidget::updateWidgetGeometry()
     return setWidgetGeometry(absoluteContentBox);
 }
 
+// FIXME: Once Frame/FrameElementBase/FrameOwnerElement and HTMLAppletElement

[eseidel, 2013/11/18 22:26:45]

I see.  The problem is there are multiple DOM classes which know how to have a
Widget?  Seems we should just move them all over before adding the browser bits.

I understand you're here for the browser bits.  I'm just not comfortable with
the complexity this change adds to the core code.  We should fix the core code
to make your usecase the easy case first, imo.

[wjmaclean, 2013/11/25 17:51:56]

As per comment regarding removing widget pointer.

Done.

+// can be updated to own their own widgets, this function can be retired in
+// favour of configureWidget().
 void RenderWidget::setWidget(PassRefPtr<Widget> widget)
 {
     if (widget == m_widget)
@@ -208,6 +230,36 @@ void RenderWidget::setWidget(PassRefPtr<Widget> widget)
         cache->childrenChanged(this);
 }
 
+void RenderWidget::configureWidget(ConfigureType type)
+{
+    Widget* widget = this->widget();
+    ASSERT(widget);
+
+    if (type == ConfigureOnDetach) {
+        moveWidgetToParentSoon(widget, 0);
+        return;
+    }
+
+    // type == ConfigureOnAttach.

[eseidel, 2013/11/18 22:26:45]

confused.  Which one of these is the old path vs. the new path?

[wjmaclean, 2013/11/25 17:51:56]

It's not so much an old vs. new distinction as attach/detach. I've restructured
this code in the new patch in a way that will hopefully clarify this.

+    if (widget) {
+        if (style()) {
+            if (!needsLayout())
+                updateWidgetGeometry();
+
+            if (style()->visibility() != VISIBLE) {
+                widget->hide();
+            } else {
+                widget->show();
+                repaint();
+            }
+            if (!widget->parent())
+                moveWidgetToParentSoon(widget, m_frameView);
+        }
+        if (AXObjectCache* cache = document().existingAXObjectCache())
+            cache->childrenChanged(this);
+    }
+}
+
 void RenderWidget::layout()
 {
     ASSERT(needsLayout());
@@ -219,11 +271,13 @@ void RenderWidget::layout()
 void RenderWidget::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
 {
     RenderReplaced::styleDidChange(diff, oldStyle);
-    if (m_widget) {
+    Widget* widget = this->widget();
+
+    if (widget) {
         if (style()->visibility() != VISIBLE)
-            m_widget->hide();
+            widget->hide();
         else
-            m_widget->show();
+            widget->show();
     }
 }
 
@@ -231,9 +285,12 @@ void RenderWidget::paintContents(PaintInfo& paintInfo, const LayoutPoint& paintO
 {
     LayoutPoint adjustedPaintOffset = paintOffset + location();
 
+    Widget* widget = this->widget();
+    ASSERT(widget);
+
     // Tell the widget to paint now. This is the only time the widget is allowed
     // to paint itself. That way it will composite properly with z-indexed layers.
-    IntPoint widgetLocation = m_widget->frameRect().location();
+    IntPoint widgetLocation = widget->frameRect().location();
     IntPoint paintLocation(roundToInt(adjustedPaintOffset.x() + borderLeft() + paddingLeft()),
         roundToInt(adjustedPaintOffset.y() + borderTop() + paddingTop()));
     IntRect paintRect = paintInfo.rect;
@@ -245,17 +302,17 @@ void RenderWidget::paintContents(PaintInfo& paintInfo, const LayoutPoint& paintO
         paintInfo.context->translate(widgetPaintOffset);
         paintRect.move(-widgetPaintOffset);
     }
-    m_widget->paint(paintInfo.context, paintRect);
+    widget->paint(paintInfo.context, paintRect);
 
     if (!widgetPaintOffset.isZero())
         paintInfo.context->translate(-widgetPaintOffset);
 
-    if (m_widget->isFrameView()) {
-        FrameView* frameView = toFrameView(m_widget.get());
+    if (widget->isFrameView()) {
+        FrameView* frameView = toFrameView(widget);
         bool runOverlapTests = !frameView->useSlowRepaintsIfNotOverlapped() || frameView->hasCompositedContent();
         if (paintInfo.overlapTestRequests && runOverlapTests) {
             ASSERT(!paintInfo.overlapTestRequests->contains(this));
-            paintInfo.overlapTestRequests->set(this, m_widget->frameRect());
+            paintInfo.overlapTestRequests->set(this, widget->frameRect());
         }
     }
 }
@@ -296,7 +353,8 @@ void RenderWidget::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
         clipRoundedInnerRect(paintInfo.context, borderRect, roundedInnerRect);
     }
 
-    if (m_widget)
+    Widget* widget = this->widget();
+    if (widget)
         paintContents(paintInfo, paintOffset);
 
     if (style()->hasBorderRadius())
@@ -314,9 +372,10 @@ void RenderWidget::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 
 void RenderWidget::setIsOverlapped(bool isOverlapped)
 {
-    ASSERT(m_widget);
-    ASSERT(m_widget->isFrameView());
-    toFrameView(m_widget.get())->setIsOverlapped(isOverlapped);
+    Widget* widget = this->widget();
+    ASSERT(widget);
+    ASSERT(widget->isFrameView());
+    toFrameView(widget)->setIsOverlapped(isOverlapped);
 }
 
 void RenderWidget::deref()
@@ -327,15 +386,16 @@ void RenderWidget::deref()
 
 void RenderWidget::updateWidgetPosition()
 {
-    if (!m_widget || !node()) // Check the node in case destroy() has been called.
+    Widget* widget = this->widget();
+    if (!widget || !node()) // Check the node in case destroy() has been called.
         return;
 
     bool boundsChanged = updateWidgetGeometry();
 
     // if the frame bounds got changed, or if view needs layout (possibly indicating
     // content size is wrong) we have to do a layout to set the right widget size
-    if (m_widget && m_widget->isFrameView()) {
-        FrameView* frameView = toFrameView(m_widget.get());
+    if (widget && widget->isFrameView()) {
+        FrameView* frameView = toFrameView(widget);
         // Check the frame's page to make sure that the frame isn't in the process of being destroyed.
         if ((boundsChanged || frameView->needsLayout()) && frameView->frame().page())
             frameView->layout();
@@ -344,9 +404,10 @@ void RenderWidget::updateWidgetPosition()
 
 void RenderWidget::widgetPositionsUpdated()
 {
-    if (!m_widget)
+    Widget* widget = this->widget();
+    if (!widget)
         return;
-    m_widget->widgetPositionsUpdated();
+    widget->widgetPositionsUpdated();
 }
 
 IntRect RenderWidget::windowClipRect() const