Move plugins to be stored in HTMLPlugInElement.

third_party/WebKit/Source/core/layout/LayoutPart.cpp

Index: third_party/WebKit/Source/core/layout/LayoutPart.cpp
diff --git a/third_party/WebKit/Source/core/layout/LayoutPart.cpp b/third_party/WebKit/Source/core/layout/LayoutPart.cpp
index 9078c43ead331db2c248178d7033e6bdbb811f39..8e8745c8e0f807848d38e254d6bf0143a0a7d8d9 100644
--- a/third_party/WebKit/Source/core/layout/LayoutPart.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutPart.cpp
@@ -28,6 +28,7 @@
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/html/HTMLFrameElementBase.h"
+#include "core/html/HTMLPlugInElement.h"
 #include "core/layout/HitTestResult.h"
 #include "core/layout/LayoutAnalyzer.h"
 #include "core/layout/LayoutView.h"
@@ -98,6 +99,13 @@ FrameViewBase* LayoutPart::frameViewBase() const {
   return nullptr;
 }
 
+PluginView* LayoutPart::plugin() const {
+  // Plugins are stored in their DOM node.
+  return node() && isHTMLPlugInElement(node())
+             ? toHTMLPlugInElement(node())->plugin()
+             : nullptr;
+}
+
 PaintLayerType LayoutPart::layerTypeRequired() const {
   PaintLayerType type = LayoutReplaced::layerTypeRequired();
   if (type != NoPaintLayer)
@@ -110,8 +118,8 @@ bool LayoutPart::requiresAcceleratedCompositing() const {
   // a plugin LayoutObject and the plugin has a layer, then we need a layer.
   // Second, if this is a LayoutObject with a contentDocument and that document
   // needs a layer, then we need a layer.
-  if (frameViewBase() && frameViewBase()->isPluginView() &&
-      toPluginView(frameViewBase())->platformLayer())
+  PluginView* pluginView = plugin();
+  if (pluginView && pluginView->platformLayer())
     return true;
 
   if (!node() || !node()->isFrameOwnerElement())
@@ -247,6 +255,7 @@ void LayoutPart::styleDidChange(StyleDifference diff,
   if (frameViewBase && frameViewBase->isFrameView())
     toFrameView(frameViewBase)->recalculateCustomScrollbarStyle();
 
+  // TODO(joelhockey): Does this need separate code for plugins?

[haraken, 2017/03/22 15:32:33]

I guess yes.

Can you add CHECK(!(node() && isHTMLPlugInElement(node()))) and check if your
guess is correct? The same comment for other TODOs in this CL.

[joelhockey, 2017/03/27 06:42:21]

Thanks, that is a helpful approach.  It showed that plugins do indeed get called
from here.  For now, I am setting FrameViewBase to PluginView if not null, else
FrameView.

Once I remove FrameViewBase, my plan is to have separate version of
updateOnWidgetChange and updateGeometry:
- updateOnFrameChange
- updateFrameGeometry
- updateOnPluginChange
- updatePluginGeometry.

It is clear to see where each of these are called separately.

   if (style()->visibility() != EVisibility::kVisible) {
     frameViewBase->hide();

[haraken, 2017/03/22 15:32:33]

One way to handle both the FrameViewBase case and the plugin case would be to
implement the show/hide method on node() (i.e., HTMLFrameOwnerElement and
HTMLPluginElement). Then HTMLFrameOwnerElement and HTMLPluginElement can
dispatch those methods to proper things (i.e., m_frameViewBase or m_plugin).

[joelhockey, 2017/03/27 06:42:21]

This function will be ok to take FrameViewBase as input and use polymorphism to
call on either Frame or Plugin.  I will create a common ABC with hide/show
methods.

[haraken, 2017/03/28 07:39:41]

However, you're planning to remove the inheritance between FrameViewBase and
Plugin, right? At that point, you can no longer rely on the polymorphism.

[joelhockey, 2017/03/28 22:56:23]

Yes, I will remove the inheritance from FrameView -> FrameViewBase and
PluginView -> FrameViewBase.  One of my current thoughts is to create a new
common abstract base class named Focusable which has functions hide and show. 
FrameView and PluginView will both extend this.  LayoutPart can then have a
single way to get a handle on Focusable and use polymorphism to call hide/show
on either Frame or Plugin.  I do also like your suggestion to defer this method
into Node and then have HTMLPlugInElement and HTMLFrameOwnerElement provide
override implementations.  I might try them both and see what I think looks the
best design.

There are also some other geometry methods that are common to both classes.  I'm
reluctant to put even more methods on Node, although maybe a few more can't hurt
when its already got so many :-)

[haraken, 2017/03/29 11:04:46]

Sounds like a nice plan.

   } else {
@@ -272,7 +281,7 @@ void LayoutPart::paintContents(const PaintInfo& paintInfo,
 
 CursorDirective LayoutPart::getCursor(const LayoutPoint& point,
                                       Cursor& cursor) const {
-  if (frameViewBase() && frameViewBase()->isPluginView()) {
+  if (plugin()) {
     // A plugin is responsible for setting the cursor when the pointer is over
     // it.
     return DoNotSetCursor;
@@ -301,6 +310,7 @@ void LayoutPart::updateOnWidgetChange() {
   if (!needsLayout())
     updateGeometryInternal();
 
+  // TODO(joelhockey): Does this need special handling for plugin?

[dcheng, 2017/03/23 07:21:42]

I think this also needs to be handled: the main subclass of PluginView is
WebPluginContainerImpl, which forwards the visibility change to the plugin
itself. =/

[joelhockey, 2017/03/27 06:42:21]

Thanks.  I just noticed how the impl forwards the change through to itself. 
Well worthy of a wry smily =/.  As per comment above, for now I will get either
PluginView or FrameView here, but my longer plan is to duplicate
updateOnWidgetChange to have separate versions for Frame and Plugin.

   if (style()->visibility() != EVisibility::kVisible) {
     frameViewBase->hide();
   } else {
@@ -319,6 +329,7 @@ void LayoutPart::updateGeometry() {
 
   LayoutRect newFrame = replacedContentRect();
   DCHECK(newFrame.size() == roundedIntSize(newFrame.size()));
+  // TODO(joelhockey): Does this need special handling for plugin?
   bool boundsWillChange =
       LayoutSize(frameViewBase->frameRect().size()) != newFrame.size();
 
@@ -372,6 +383,7 @@ void LayoutPart::updateGeometryInternal() {
 
   // Why is the protector needed?
   RefPtr<LayoutPart> protector(this);
+  // TODO(joelhockey): Does this need special handling for plugin?
   frameViewBase->setFrameRect(frameRect);
 }