Remove FrameViewBase as base class of PluginView.

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 97212dd02c9387c312db5285b7c524b777d7b0db..43ba2758548c39b61933c4f9e0c5551c6a06d04a 100644
--- a/third_party/WebKit/Source/core/layout/LayoutPart.cpp
+++ b/third_party/WebKit/Source/core/layout/LayoutPart.cpp
@@ -25,6 +25,7 @@
 #include "core/layout/LayoutPart.h"
 
 #include "core/dom/AXObjectCache.h"
+#include "core/frame/FrameOrPlugin.h"
 #include "core/frame/FrameView.h"
 #include "core/frame/LocalFrame.h"
 #include "core/html/HTMLFrameElementBase.h"
@@ -90,7 +91,7 @@ LayoutPart::~LayoutPart() {
 }
 
 FrameViewBase* LayoutPart::GetFrameViewBase() const {
-  // Plugin FrameViewBases are stored in their DOM node.
+  // FrameViews are stored in HTMLFrameOwnerElement node.
   Element* element = ToElement(GetNode());
 
   if (element && element->IsFrameOwnerElement())
@@ -106,8 +107,12 @@ PluginView* LayoutPart::Plugin() const {
              : nullptr;
 }
 
-FrameViewBase* LayoutPart::PluginOrFrame() const {
-  FrameViewBase* result = GetFrameViewBase();
+FrameOrPlugin* LayoutPart::GetFrameOrPlugin() const {
+  FrameOrPlugin* result = nullptr;
+  FrameViewBase* frame_view_base = GetFrameViewBase();
+  if (frame_view_base && frame_view_base->IsFrameView()) {
+    result = ToFrameView(frame_view_base);
+  }
   if (!result)
     result = Plugin();
   return result;
@@ -256,18 +261,21 @@ CompositingReasons LayoutPart::AdditionalCompositingReasons() const {
 void LayoutPart::StyleDidChange(StyleDifference diff,
                                 const ComputedStyle* old_style) {
   LayoutReplaced::StyleDidChange(diff, old_style);
-  FrameViewBase* frame_view_base = this->PluginOrFrame();
-  if (!frame_view_base)
+  FrameOrPlugin* frame_or_plugin = GetFrameOrPlugin();
+  if (!frame_or_plugin)
     return;
 
   // If the iframe has custom scrollbars, recalculate their style.
-  if (frame_view_base->IsFrameView())
-    ToFrameView(frame_view_base)->RecalculateCustomScrollbarStyle();
+  FrameViewBase* frame_view_base = GetFrameViewBase();
+  if (frame_view_base && frame_view_base->IsFrameView()) {

[dcheng, 2017/04/12 00:06:39]

Maybe it would be more natural to write this as a downcast from FrameOrPlugin
directly, rather than checking FrameViewBase?

[joelhockey, 2017/04/12 04:33:00]

My preferred solution is to make FrameView accessible directly from LayoutPart
on its own method.  I didn't want to mix up this CL with too many things though.
 I think I'll do it as a prequel CL right now.  Keep your eyes peeeled.

I don't like the explicit downcasts as a general pattern, so I don't want to
introduce it into the FrameOrPlugin class.

LayoutPart inherits indirectly from LayoutObject.  LayoutObject already has a
method GetFrameView which returns Document().Frame().View().  This is not the
same as the FrameViewBase which is stored on HTMLFrameOwnerElement.  So I need
to think of a good name for this method.  I will do my best and you will see it
in a CL coming soon.

+    FrameView* frame_view = ToFrameView(frame_view_base);
+    frame_view->RecalculateCustomScrollbarStyle();
+  }
 
   if (Style()->Visibility() != EVisibility::kVisible) {
-    frame_view_base->Hide();
+    frame_or_plugin->Hide();
   } else {
-    frame_view_base->Show();
+    frame_or_plugin->Show();
   }
 }
 
@@ -309,20 +317,20 @@ LayoutRect LayoutPart::ReplacedContentRect() const {
 }
 
 void LayoutPart::UpdateOnWidgetChange() {
-  FrameViewBase* frame_view_base = this->PluginOrFrame();
-  if (!frame_view_base)
+  FrameOrPlugin* frame_or_plugin = GetFrameOrPlugin();
+  if (!frame_or_plugin)
     return;
 
   if (!Style())
     return;
 
   if (!NeedsLayout())
-    UpdateGeometryInternal(*frame_view_base);
+    UpdateGeometryInternal(*frame_or_plugin);
 
   if (Style()->Visibility() != EVisibility::kVisible) {
-    frame_view_base->Hide();
+    frame_or_plugin->Hide();
   } else {
-    frame_view_base->Show();
+    frame_or_plugin->Show();
     // FIXME: Why do we issue a full paint invalidation in this case, but not
     // the other?
     SetShouldDoFullPaintInvalidation();
@@ -330,18 +338,21 @@ void LayoutPart::UpdateOnWidgetChange() {
 }
 
 void LayoutPart::UpdateGeometry() {
-  FrameViewBase* frame_view_base = this->PluginOrFrame();
-  if (!frame_view_base ||
+  FrameOrPlugin* frame_or_plugin = GetFrameOrPlugin();
+  if (!frame_or_plugin ||
       !GetNode())  // Check the node in case destroy() has been called.
     return;
 
   LayoutRect new_frame = ReplacedContentRect();
   DCHECK(new_frame.size() == RoundedIntSize(new_frame.size()));
   bool bounds_will_change =
-      LayoutSize(frame_view_base->FrameRect().size()) != new_frame.size();
+      LayoutSize(frame_or_plugin->FrameRect().size()) != new_frame.size();
 
-  FrameView* frame_view =
-      frame_view_base->IsFrameView() ? ToFrameView(frame_view_base) : nullptr;
+  FrameViewBase* frame_view_base = GetFrameViewBase();
+  FrameView* frame_view = nullptr;
+  if (frame_view_base && frame_view_base->IsFrameView()) {

[dcheng, 2017/04/12 00:06:39]

Ditto -- it seems a bit unusual to have this null check here.

[joelhockey, 2017/04/12 04:33:00]

I'll clean this up as a prequel CL.

+    frame_view = ToFrameView(frame_view_base);
+  }
 
   // If frame bounds are changing mark the view for layout. Also check the
   // frame's page to make sure that the frame isn't in the process of being
@@ -351,19 +362,20 @@ void LayoutPart::UpdateGeometry() {
       (bounds_will_change || frame_view->NeedsScrollbarReconstruction()))
     frame_view->SetNeedsLayout();
 
-  UpdateGeometryInternal(*frame_view_base);
+  UpdateGeometryInternal(*frame_or_plugin);
 
   // If view needs layout, either because bounds have changed or possibly
   // indicating content size is wrong, we have to do a layout to set the right
-  // FrameViewBase size.
+  // FrameView size.
   if (frame_view && frame_view->NeedsLayout() &&
       frame_view->GetFrame().GetPage())
     frame_view->Layout();
 
-  frame_view_base->GeometryMayHaveChanged();
+  if (PluginView* plugin = Plugin())
+    plugin->GeometryMayHaveChanged();
 }
 
-void LayoutPart::UpdateGeometryInternal(FrameViewBase& frame_view_base) {
+void LayoutPart::UpdateGeometryInternal(FrameOrPlugin& frame_or_plugin) {
   // Ignore transform here, as we only care about the sub-pixel accumulation.
   // TODO(trchen): What about multicol? Need a LayoutBox function to query
   // sub-pixel accumulation.
@@ -375,7 +387,7 @@ void LayoutPart::UpdateGeometryInternal(FrameViewBase& frame_view_base) {
                      PixelSnappedIntRect(absolute_replaced_rect).size());
   // Normally the location of the frame rect is ignored by the painter, but
   // currently it is still used by a family of coordinate conversion function in
-  // FrameViewBase/FrameView. This is incorrect because coordinate conversion
+  // FrameView. This is incorrect because coordinate conversion
   // needs to take transform and into account. A few callers still use the
   // family of conversion function, including but not exhaustive:
   // FrameView::updateViewportIntersectionIfNeeded()
@@ -388,7 +400,7 @@ void LayoutPart::UpdateGeometryInternal(FrameViewBase& frame_view_base) {
 
   // Why is the protector needed?
   RefPtr<LayoutPart> protector(this);
-  frame_view_base.SetFrameRect(frame_rect);
+  frame_or_plugin.SetFrameRect(frame_rect);
 }
 
 void LayoutPart::InvalidatePaintOfSubtreesIfNeeded(