Maybe these comments are helpful when reviewing.
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/frame/FrameOrPlugin.hFile third_party/WebKit/Source/core/frame/FrameOrPlugin.h (right):
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/frame/FrameOrPlugin.h#newcode20third_party/WebKit/Source/core/frame/FrameOrPlugin.h:20: class
CORE_EXPORT FrameOrPlugin : public GarbageCollectedMixin {
Methods SetParent, Parent, SetParentVisible, FrameRectsChanged moved
from FrameViewBase to FrameOrPlugin. These methods are applicable to
FrameView, RemoteFrameView and PluginView. Parent is now type FrameView
rather than FrameViewBase.
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/frame/FrameView.hFile third_party/WebKit/Source/core/frame/FrameView.h (left):
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/frame/FrameView.h#oldcode503third_party/WebKit/Source/core/frame/FrameView.h:503: void
RemovePlugin(PluginView*);
Scrollbar is still a special-case child of FrameView, but Plugin is not.
All of the FrameOrPlugin (FrameView, RemoteFrameView and PluginView)
types are combined into children_
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/frame/FrameView.hFile third_party/WebKit/Source/core/frame/FrameView.h (right):
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/frame/FrameView.h#newcode650third_party/WebKit/Source/core/frame/FrameView.h:650: IntPoint
ConvertSelfToChild(const IntPoint&, const IntPoint&) const;
The method was only calling FrameViewBase::Location, so it is easy for
clients to provide this IntPoint directly.
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/frame/FrameView.h#newcode666third_party/WebKit/Source/core/frame/FrameView.h:666: IntPoint
ConvertFromContainingFrameViewToScrollbar(
I changed a few Convert...FrameViewBase... methods to
Convert...FrameView...
This still doesn't feel like a perfect name, but overall an improvement.
I would be happy to shorten even more to something like:
ConvertFromContainerToScrollbar
Or in cases like this where Scrollbar is provided as an input type, I
also like:
ConvertFromContainer
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/frame/FrameView.h#newcode1065third_party/WebKit/Source/core/frame/FrameView.h:1065: //
Member<FrameView> instead.
I've tried to update comments to be more accurate. Some changes are a
guess, but even if not perfect, at least hopefully things are not worse.
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/frame/RemoteFrameView.hFile third_party/WebKit/Source/core/frame/RemoteFrameView.h (right):
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/frame/RemoteFrameView.h#newcode19third_party/WebKit/Source/core/frame/RemoteFrameView.h:19: class
RemoteFrameView final : public
GarbageCollectedFinalized<RemoteFrameView>,
The main point of this CL is to stop RemoteFrameView from inheriting
from FrameViewBase. It is nice to see that overall there is a slight
reduction in lines of code and number of methods in this class as a
result.
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.hFile third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.h
(right):
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.h#newcode64third_party/WebKit/Source/core/html/HTMLFrameOwnerElement.h:64: void
SetWidget(FrameOrPlugin*);
Widget type has changed from platform/FrameViewBase to
core/frame/FrameOrPlugin.
I still plan to change the name of this method to something other than
'Widget' once I think of something better. Right now 'FrameOrPlugin' is
the leading candidate.
Previously I moved PluginView to be stored on HTMLPlugInElement. I have
now moved it back to be part of the single 'widget_' field in
HTMLFrameOwnerElement. Having the single field simplifies the overall
code, particularly the attach and disposal.
I still make a distinction in LayoutPart class where callers can request
only the PluginView, or only the FrameView, or for FrameOrPlugin which
may be any of [Frame|RemoteFrame|Plugin]View.
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/html/HTMLPlugInElement.hFile third_party/WebKit/Source/core/html/HTMLPlugInElement.h (left):
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/html/HTMLPlugInElement.h#oldcode169third_party/WebKit/Source/core/html/HTMLPlugInElement.h:169: PluginView*
ReleasePlugin();
These specialized PluginView methods are no longer needed. PluginView
has gone back to being stored in HTMLFrameOwnerElement::widget_
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/html/PluginDocument.hFile third_party/WebKit/Source/core/html/PluginDocument.h (right):
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/core/html/PluginDocument.h#newcode59third_party/WebKit/Source/core/html/PluginDocument.h:59:
Member<HTMLPlugInElement> plugin_node_;
plugin_node_ is always HTMLPlugInElement
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/platform/FrameViewBase.hFile third_party/WebKit/Source/platform/FrameViewBase.h (left):
https://codereview.chromium.org/2845583002/diff/60001/third_party/WebKit/Source/platform/FrameViewBase.h#oldcode51third_party/WebKit/Source/platform/FrameViewBase.h:51: virtual void
SetParent(FrameViewBase*) = 0;
SetParent, SetParentVisible, FrameRectsChanged methods moved to
FrameOrPlugin interface.
https://codereview.chromium.org/2845583002/