Rename FrameView fields and methods *Widget* to ...FrameViewBase...

Rename FrameView fields and methods *Widget* to ...FrameViewBase...

BUG=697351
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

[joelhockey, 2017-03-03 06:10:52]

Description was changed from

==========
Rename fields and methods *Widget* to ...FrameViewBase...

BUG=697351
==========

to

==========
Rename fields and methods *Widget* to ...FrameViewBase...

BUG=697351
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[joelhockey, 2017-03-03 06:32:22]

Description was changed from

==========
Rename fields and methods *Widget* to ...FrameViewBase...

BUG=697351
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Rename FrameView fields and methods *Widget* to ...FrameViewBase...

BUG=697351
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[joelhockey, 2017-03-03 07:05:33]

joelhockey@chromium.org changed reviewers:
+ haraken@chromium.org, sashab@chromium.org, slangley@chromium.org

[joelhockey, 2017-03-03 07:06:18]

The CQ bit was checked by joelhockey@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-03 07:06:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-03 08:31:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-03 08:31:56]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-03-03 19:07:34]

https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.h (right):

https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.h:160: void
setNeedsUpdateFrameViewBaseGeometries() {

This sounds a bit too verbose. setNeedsUpdateGeometries?

https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.h:272: void
updateFrameViewBaseGeometries();

updateGeometries ?

https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.h:658: IntPoint
convertFromContainingFrameViewBaseToScrollbar(

Instead of renaming methods on a per-file basis, it might make more sense to
rename them on a per-functionality basis. e.g., you can rename all convert*
methods in one CL. Then you can rename all updateGeometry things in one CL etc.

[joelhockey, 2017-03-05 22:37:42]

https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.h (right):

https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.h:160: void
setNeedsUpdateFrameViewBaseGeometries() {
On 2017/03/03 at 19:07:34, haraken wrote:
> This sounds a bit too verbose. setNeedsUpdateGeometries?

done

https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.h:272: void
updateFrameViewBaseGeometries();
On 2017/03/03 at 19:07:34, haraken wrote:
> updateGeometries ?

done

https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.h:658: IntPoint
convertFromContainingFrameViewBaseToScrollbar(
On 2017/03/03 at 19:07:34, haraken wrote:
> Instead of renaming methods on a per-file basis, it might make more sense to
rename them on a per-functionality basis. e.g., you can rename all convert*
methods in one CL. Then you can rename all updateGeometry things in one CL etc.

Acknowledged.  I'll try and use the approach for other renames.  I went with the
per-file approach since I don't really know which per-functionality methods
exist and where they live.  At least with the per-file approach I can make sure
I get everything.  In the end, I figure that the renames will happen in very
quick succession and the code ends up in the same end-state.

[joelhockey, 2017-03-05 22:37:47]

The CQ bit was checked by joelhockey@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-05 22:37:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-05 23:58:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-05 23:58:47]

Dry run: This issue passed the CQ dry run.

[joelhockey, 2017-03-06 00:01:19]

The CQ bit was checked by joelhockey@chromium.org

[commit-bot: I haz the power, 2017-03-06 00:01:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-06 00:01:29]

This CL has an open dependency (Issue 2725273002 Patch 40001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[joelhockey, 2017-03-06 00:01:44]

The CQ bit was checked by joelhockey@chromium.org

[commit-bot: I haz the power, 2017-03-06 00:02:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-06 00:02:06]

This CL has an open dependency (Issue 2725273002 Patch 40001). Please resolve
the dependency and try again.
If you are sure that there is no real dependency, please use one of the options
listed in https://goo.gl/9Es4OR to land the CL.

[joelhockey, 2017-03-06 01:07:50]

The CQ bit was checked by joelhockey@chromium.org

[commit-bot: I haz the power, 2017-03-06 01:08:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-06 01:08:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-06 01:08:10]

No L-G-T-M from a valid reviewer yet. 
CQ run can only be started by full committers or once the patch has
received an L-G-T-M from a full committer.
Even if an L-G-T-M may have been provided, it was from a non-committer,
_not_ a full super star committer.
Committers are members of the group "project-chromium-committers".
Note that this has nothing to do with OWNERS files.

[joelhockey, 2017-03-06 01:39:20]

The CQ bit was checked by joelhockey@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-06 01:39:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[haraken, 2017-03-06 01:54:49]

On 2017/03/05 22:37:42, joelhockey wrote:
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/frame/FrameView.h (right):
> 
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/frame/FrameView.h:160: void
> setNeedsUpdateFrameViewBaseGeometries() {
> On 2017/03/03 at 19:07:34, haraken wrote:
> > This sounds a bit too verbose. setNeedsUpdateGeometries?
> 
> done
> 
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/frame/FrameView.h:272: void
> updateFrameViewBaseGeometries();
> On 2017/03/03 at 19:07:34, haraken wrote:
> > updateGeometries ?
> 
> done
> 
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/frame/FrameView.h:658: IntPoint
> convertFromContainingFrameViewBaseToScrollbar(
> On 2017/03/03 at 19:07:34, haraken wrote:
> > Instead of renaming methods on a per-file basis, it might make more sense to
> rename them on a per-functionality basis. e.g., you can rename all convert*
> methods in one CL. Then you can rename all updateGeometry things in one CL
etc.
> 
> Acknowledged.  I'll try and use the approach for other renames.  I went with
the
> per-file approach since I don't really know which per-functionality methods
> exist and where they live.  At least with the per-file approach I can make
sure
> I get everything.  In the end, I figure that the renames will happen in very
> quick succession and the code ends up in the same end-state.

But landing this CL would make the subsequent reviews harder. For example,
you're renaming FrameView::updateWidgetGeometries in this CL but not renaming
LayoutPart::updateWidgetGeometry. Then we need to be careful not to rename
LayoutPart::updateWidgetGeometry to LayoutPart::updateFrameBaseViewGeometry when
we rename it.

Note that what we want to do is not a simple mechanical renaming (then you can
do that in one go) but improving the names so that they make more sense in the
code base. That will need some code inspection on a case-by-case basis.

[joelhockey, 2017-03-06 02:01:41]

On 2017/03/06 at 01:54:49, haraken wrote:
> On 2017/03/05 22:37:42, joelhockey wrote:
> >
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/frame/FrameView.h (right):
> > 
> >
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/frame/FrameView.h:160: void
> > setNeedsUpdateFrameViewBaseGeometries() {
> > On 2017/03/03 at 19:07:34, haraken wrote:
> > > This sounds a bit too verbose. setNeedsUpdateGeometries?
> > 
> > done
> > 
> >
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/frame/FrameView.h:272: void
> > updateFrameViewBaseGeometries();
> > On 2017/03/03 at 19:07:34, haraken wrote:
> > > updateGeometries ?
> > 
> > done
> > 
> >
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/frame/FrameView.h:658: IntPoint
> > convertFromContainingFrameViewBaseToScrollbar(
> > On 2017/03/03 at 19:07:34, haraken wrote:
> > > Instead of renaming methods on a per-file basis, it might make more sense
to
> > rename them on a per-functionality basis. e.g., you can rename all convert*
> > methods in one CL. Then you can rename all updateGeometry things in one CL
etc.
> > 
> > Acknowledged.  I'll try and use the approach for other renames.  I went with
the
> > per-file approach since I don't really know which per-functionality methods
> > exist and where they live.  At least with the per-file approach I can make
sure
> > I get everything.  In the end, I figure that the renames will happen in very
> > quick succession and the code ends up in the same end-state.
> 
> But landing this CL would make the subsequent reviews harder. For example,
you're renaming FrameView::updateWidgetGeometries in this CL but not renaming
LayoutPart::updateWidgetGeometry. Then we need to be careful not to rename
LayoutPart::updateWidgetGeometry to LayoutPart::updateFrameBaseViewGeometry when
we rename it.
> 
> Note that what we want to do is not a simple mechanical renaming (then you can
do that in one go) but improving the names so that they make more sense in the
code base. That will need some code inspection on a case-by-case basis.

Do you want me to continue with this CL?  Should I add LayoutPart, or any other
files?  I have another CL just about ready which does include LayoutPart, and I
have kept the *Geometry renaming consistent with this CL.

[haraken, 2017-03-06 02:03:59]

On 2017/03/06 02:01:41, joelhockey wrote:
> On 2017/03/06 at 01:54:49, haraken wrote:
> > On 2017/03/05 22:37:42, joelhockey wrote:
> > >
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/core/frame/FrameView.h (right):
> > > 
> > >
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/core/frame/FrameView.h:160: void
> > > setNeedsUpdateFrameViewBaseGeometries() {
> > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > This sounds a bit too verbose. setNeedsUpdateGeometries?
> > > 
> > > done
> > > 
> > >
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/core/frame/FrameView.h:272: void
> > > updateFrameViewBaseGeometries();
> > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > updateGeometries ?
> > > 
> > > done
> > > 
> > >
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/core/frame/FrameView.h:658: IntPoint
> > > convertFromContainingFrameViewBaseToScrollbar(
> > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > Instead of renaming methods on a per-file basis, it might make more
sense
> to
> > > rename them on a per-functionality basis. e.g., you can rename all
convert*
> > > methods in one CL. Then you can rename all updateGeometry things in one CL
> etc.
> > > 
> > > Acknowledged.  I'll try and use the approach for other renames.  I went
with
> the
> > > per-file approach since I don't really know which per-functionality
methods
> > > exist and where they live.  At least with the per-file approach I can make
> sure
> > > I get everything.  In the end, I figure that the renames will happen in
very
> > > quick succession and the code ends up in the same end-state.
> > 
> > But landing this CL would make the subsequent reviews harder. For example,
> you're renaming FrameView::updateWidgetGeometries in this CL but not renaming
> LayoutPart::updateWidgetGeometry. Then we need to be careful not to rename
> LayoutPart::updateWidgetGeometry to LayoutPart::updateFrameBaseViewGeometry
when
> we rename it.
> > 
> > Note that what we want to do is not a simple mechanical renaming (then you
can
> do that in one go) but improving the names so that they make more sense in the
> code base. That will need some code inspection on a case-by-case basis.
> 
> Do you want me to continue with this CL?  Should I add LayoutPart, or any
other
> files?  I have another CL just about ready which does include LayoutPart, and
I
> have kept the *Geometry renaming consistent with this CL.

Is it the last part? If we still need to rename a bunch of more things, I'd
prefer switching to the per-functionality renaming now.

[joelhockey, 2017-03-06 02:30:12]

On 2017/03/06 at 02:03:59, haraken wrote:
> On 2017/03/06 02:01:41, joelhockey wrote:
> > On 2017/03/06 at 01:54:49, haraken wrote:
> > > On 2017/03/05 22:37:42, joelhockey wrote:
> > > >
> >
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > > File third_party/WebKit/Source/core/frame/FrameView.h (right):
> > > > 
> > > >
> >
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > > third_party/WebKit/Source/core/frame/FrameView.h:160: void
> > > > setNeedsUpdateFrameViewBaseGeometries() {
> > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > This sounds a bit too verbose. setNeedsUpdateGeometries?
> > > > 
> > > > done
> > > > 
> > > >
> >
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > > third_party/WebKit/Source/core/frame/FrameView.h:272: void
> > > > updateFrameViewBaseGeometries();
> > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > updateGeometries ?
> > > > 
> > > > done
> > > > 
> > > >
> >
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > > third_party/WebKit/Source/core/frame/FrameView.h:658: IntPoint
> > > > convertFromContainingFrameViewBaseToScrollbar(
> > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > Instead of renaming methods on a per-file basis, it might make more
sense
> > to
> > > > rename them on a per-functionality basis. e.g., you can rename all
convert*
> > > > methods in one CL. Then you can rename all updateGeometry things in one
CL
> > etc.
> > > > 
> > > > Acknowledged.  I'll try and use the approach for other renames.  I went
with
> > the
> > > > per-file approach since I don't really know which per-functionality
methods
> > > > exist and where they live.  At least with the per-file approach I can
make
> > sure
> > > > I get everything.  In the end, I figure that the renames will happen in
very
> > > > quick succession and the code ends up in the same end-state.
> > > 
> > > But landing this CL would make the subsequent reviews harder. For example,
> > you're renaming FrameView::updateWidgetGeometries in this CL but not
renaming
> > LayoutPart::updateWidgetGeometry. Then we need to be careful not to rename
> > LayoutPart::updateWidgetGeometry to LayoutPart::updateFrameBaseViewGeometry
when
> > we rename it.
> > > 
> > > Note that what we want to do is not a simple mechanical renaming (then you
can
> > do that in one go) but improving the names so that they make more sense in
the
> > code base. That will need some code inspection on a case-by-case basis.
> > 
> > Do you want me to continue with this CL?  Should I add LayoutPart, or any
other
> > files?  I have another CL just about ready which does include LayoutPart,
and I
> > have kept the *Geometry renaming consistent with this CL.
> 
> Is it the last part? If we still need to rename a bunch of more things, I'd
prefer switching to the per-functionality renaming now.

There are 19 more methods that I count to be changed.  I am currently planning
to put them all in the next, final CL

core/dom/ContainerNode:
 - suspendWidgetHierarychyUpdates -> suspendFrameViewBaseHierarchyUpdates

core/dom/Document:
 - widgetForElement -> frameViewBaseForElement

core/html/HTMLEmbedElement:
 - updateWidgetInternal -> updateFrameViewBaseInternal

core/html/HTMLFrameOwnerElement:
 - setWidget -> setFrameViewBase
 - releaseWidget -> releaseFrameViewBase
 - ownedWidget -> ownedFrameViewBase
 - widgetNewParentMap -> frameViewBaseNewParentMap

core/html/HTMLPlugInElement
 - pluginWidget -> pluginFrameViewBase
 - setNeedsWidgetUpdate -> setNeedsFrameViewBaseUpdate
 - setPersistedPluginWidget -> setPersistedPluginFrameViewBase
 - updateWidget -> updateFrameViewBase

core/html/PluginDocument:
 - pluginWidget -> pluginFrameViewBase

core/input/ScrollManager:
 - passScrollGestureEventToWidget -> passScrollGestureEventToFrameViewBase

core/layout/HitTestResult:
 - isOverWidget -> isOverFrameViewBase
 - setIsOverWidget -> setIsOverFrameViewBase

core/layout/LayoutPart:
 - updateOnWidgetChange -> updateOnFrameViewBaseChange
 - updateWidgetGeometry -> updateGeometry
 - updateWidgetGeometryInternal -> updateGeometryInternal

web/WebViewImpl
 - focusedLocalFrameInWidget -> focussedLocalFrameInFrameViewBase

[joelhockey, 2017-03-06 02:38:39]

I've changed my mind about ContainerNode, it is a different Widget.  So 18
more to change.

On Mon, Mar 6, 2017 at 1:30 PM, <joelhockey@chromium.org> wrote:

> On 2017/03/06 at 02:03:59, haraken wrote:
> > On 2017/03/06 02:01:41, joelhockey wrote:
> > > On 2017/03/06 at 01:54:49, haraken wrote:
> > > > On 2017/03/05 22:37:42, joelhockey wrote:
> > > > >
> > >
> https://codereview.chromium.org/2727913005/diff/40001/
> third_party/WebKit/Source/core/frame/FrameView.h
> > > > > File third_party/WebKit/Source/core/frame/FrameView.h (right):
> > > > >
> > > > >
> > >
> https://codereview.chromium.org/2727913005/diff/40001/
> third_party/WebKit/Source/core/frame/FrameView.h#newcode160
> > > > > third_party/WebKit/Source/core/frame/FrameView.h:160: void
> > > > > setNeedsUpdateFrameViewBaseGeometries() {
> > > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > > This sounds a bit too verbose. setNeedsUpdateGeometries?
> > > > >
> > > > > done
> > > > >
> > > > >
> > >
> https://codereview.chromium.org/2727913005/diff/40001/
> third_party/WebKit/Source/core/frame/FrameView.h#newcode272
> > > > > third_party/WebKit/Source/core/frame/FrameView.h:272: void
> > > > > updateFrameViewBaseGeometries();
> > > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > > updateGeometries ?
> > > > >
> > > > > done
> > > > >
> > > > >
> > >
> https://codereview.chromium.org/2727913005/diff/40001/
> third_party/WebKit/Source/core/frame/FrameView.h#newcode658
> > > > > third_party/WebKit/Source/core/frame/FrameView.h:658: IntPoint
> > > > > convertFromContainingFrameViewBaseToScrollbar(
> > > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > > Instead of renaming methods on a per-file basis, it might make
> more
> sense
> > > to
> > > > > rename them on a per-functionality basis. e.g., you can rename all
> convert*
> > > > > methods in one CL. Then you can rename all updateGeometry things
> in one
> CL
> > > etc.
> > > > >
> > > > > Acknowledged. I'll try and use the approach for other renames. I
> went
> with
> > > the
> > > > > per-file approach since I don't really know which per-functionality
> methods
> > > > > exist and where they live. At least with the per-file approach I
> can
> make
> > > sure
> > > > > I get everything. In the end, I figure that the renames will
> happen in
> very
> > > > > quick succession and the code ends up in the same end-state.
> > > >
> > > > But landing this CL would make the subsequent reviews harder. For
> example,
> > > you're renaming FrameView::updateWidgetGeometries in this CL but not
> renaming
> > > LayoutPart::updateWidgetGeometry. Then we need to be careful not to
> rename
> > > LayoutPart::updateWidgetGeometry to LayoutPart::
> updateFrameBaseViewGeometry
> when
> > > we rename it.
> > > >
> > > > Note that what we want to do is not a simple mechanical renaming
> (then you
> can
> > > do that in one go) but improving the names so that they make more
> sense in
> the
> > > code base. That will need some code inspection on a case-by-case basis.
> > >
> > > Do you want me to continue with this CL? Should I add LayoutPart, or
> any
> other
> > > files? I have another CL just about ready which does include
> LayoutPart,
> and I
> > > have kept the *Geometry renaming consistent with this CL.
> >
> > Is it the last part? If we still need to rename a bunch of more things,
> I'd
> prefer switching to the per-functionality renaming now.
>
> There are 19 more methods that I count to be changed. I am currently
> planning
> to put them all in the next, final CL
>
> core/dom/ContainerNode:
> - suspendWidgetHierarychyUpdates -> suspendFrameViewBaseHierarchyUpdates
>
> core/dom/Document:
> - widgetForElement -> frameViewBaseForElement
>
> core/html/HTMLEmbedElement:
> - updateWidgetInternal -> updateFrameViewBaseInternal
>
> core/html/HTMLFrameOwnerElement:
> - setWidget -> setFrameViewBase
> - releaseWidget -> releaseFrameViewBase
> - ownedWidget -> ownedFrameViewBase
> - widgetNewParentMap -> frameViewBaseNewParentMap
>
> core/html/HTMLPlugInElement
> - pluginWidget -> pluginFrameViewBase
> - setNeedsWidgetUpdate -> setNeedsFrameViewBaseUpdate
> - setPersistedPluginWidget -> setPersistedPluginFrameViewBase
> - updateWidget -> updateFrameViewBase
>
> core/html/PluginDocument:
> - pluginWidget -> pluginFrameViewBase
>
> core/input/ScrollManager:
> - passScrollGestureEventToWidget -> passScrollGestureEventToFrameViewBase
>
> core/layout/HitTestResult:
> - isOverWidget -> isOverFrameViewBase
> - setIsOverWidget -> setIsOverFrameViewBase
>
> core/layout/LayoutPart:
> - updateOnWidgetChange -> updateOnFrameViewBaseChange
> - updateWidgetGeometry -> updateGeometry
> - updateWidgetGeometryInternal -> updateGeometryInternal
>
> web/WebViewImpl
> - focusedLocalFrameInWidget -> focussedLocalFrameInFrameViewBase
>
>
>
> https://codereview.chromium.org/2727913005/
>

-- 
You received this message because you are subscribed to the Google Groups "Blink
Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[joelhockey, 2017-03-06 02:38:39]

I've changed my mind about ContainerNode, it is a different Widget.  So 18
more to change.

On Mon, Mar 6, 2017 at 1:30 PM, <joelhockey@chromium.org> wrote:

> On 2017/03/06 at 02:03:59, haraken wrote:
> > On 2017/03/06 02:01:41, joelhockey wrote:
> > > On 2017/03/06 at 01:54:49, haraken wrote:
> > > > On 2017/03/05 22:37:42, joelhockey wrote:
> > > > >
> > >
> https://codereview.chromium.org/2727913005/diff/40001/
> third_party/WebKit/Source/core/frame/FrameView.h
> > > > > File third_party/WebKit/Source/core/frame/FrameView.h (right):
> > > > >
> > > > >
> > >
> https://codereview.chromium.org/2727913005/diff/40001/
> third_party/WebKit/Source/core/frame/FrameView.h#newcode160
> > > > > third_party/WebKit/Source/core/frame/FrameView.h:160: void
> > > > > setNeedsUpdateFrameViewBaseGeometries() {
> > > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > > This sounds a bit too verbose. setNeedsUpdateGeometries?
> > > > >
> > > > > done
> > > > >
> > > > >
> > >
> https://codereview.chromium.org/2727913005/diff/40001/
> third_party/WebKit/Source/core/frame/FrameView.h#newcode272
> > > > > third_party/WebKit/Source/core/frame/FrameView.h:272: void
> > > > > updateFrameViewBaseGeometries();
> > > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > > updateGeometries ?
> > > > >
> > > > > done
> > > > >
> > > > >
> > >
> https://codereview.chromium.org/2727913005/diff/40001/
> third_party/WebKit/Source/core/frame/FrameView.h#newcode658
> > > > > third_party/WebKit/Source/core/frame/FrameView.h:658: IntPoint
> > > > > convertFromContainingFrameViewBaseToScrollbar(
> > > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > > Instead of renaming methods on a per-file basis, it might make
> more
> sense
> > > to
> > > > > rename them on a per-functionality basis. e.g., you can rename all
> convert*
> > > > > methods in one CL. Then you can rename all updateGeometry things
> in one
> CL
> > > etc.
> > > > >
> > > > > Acknowledged. I'll try and use the approach for other renames. I
> went
> with
> > > the
> > > > > per-file approach since I don't really know which per-functionality
> methods
> > > > > exist and where they live. At least with the per-file approach I
> can
> make
> > > sure
> > > > > I get everything. In the end, I figure that the renames will
> happen in
> very
> > > > > quick succession and the code ends up in the same end-state.
> > > >
> > > > But landing this CL would make the subsequent reviews harder. For
> example,
> > > you're renaming FrameView::updateWidgetGeometries in this CL but not
> renaming
> > > LayoutPart::updateWidgetGeometry. Then we need to be careful not to
> rename
> > > LayoutPart::updateWidgetGeometry to LayoutPart::
> updateFrameBaseViewGeometry
> when
> > > we rename it.
> > > >
> > > > Note that what we want to do is not a simple mechanical renaming
> (then you
> can
> > > do that in one go) but improving the names so that they make more
> sense in
> the
> > > code base. That will need some code inspection on a case-by-case basis.
> > >
> > > Do you want me to continue with this CL? Should I add LayoutPart, or
> any
> other
> > > files? I have another CL just about ready which does include
> LayoutPart,
> and I
> > > have kept the *Geometry renaming consistent with this CL.
> >
> > Is it the last part? If we still need to rename a bunch of more things,
> I'd
> prefer switching to the per-functionality renaming now.
>
> There are 19 more methods that I count to be changed. I am currently
> planning
> to put them all in the next, final CL
>
> core/dom/ContainerNode:
> - suspendWidgetHierarychyUpdates -> suspendFrameViewBaseHierarchyUpdates
>
> core/dom/Document:
> - widgetForElement -> frameViewBaseForElement
>
> core/html/HTMLEmbedElement:
> - updateWidgetInternal -> updateFrameViewBaseInternal
>
> core/html/HTMLFrameOwnerElement:
> - setWidget -> setFrameViewBase
> - releaseWidget -> releaseFrameViewBase
> - ownedWidget -> ownedFrameViewBase
> - widgetNewParentMap -> frameViewBaseNewParentMap
>
> core/html/HTMLPlugInElement
> - pluginWidget -> pluginFrameViewBase
> - setNeedsWidgetUpdate -> setNeedsFrameViewBaseUpdate
> - setPersistedPluginWidget -> setPersistedPluginFrameViewBase
> - updateWidget -> updateFrameViewBase
>
> core/html/PluginDocument:
> - pluginWidget -> pluginFrameViewBase
>
> core/input/ScrollManager:
> - passScrollGestureEventToWidget -> passScrollGestureEventToFrameViewBase
>
> core/layout/HitTestResult:
> - isOverWidget -> isOverFrameViewBase
> - setIsOverWidget -> setIsOverFrameViewBase
>
> core/layout/LayoutPart:
> - updateOnWidgetChange -> updateOnFrameViewBaseChange
> - updateWidgetGeometry -> updateGeometry
> - updateWidgetGeometryInternal -> updateGeometryInternal
>
> web/WebViewImpl
> - focusedLocalFrameInWidget -> focussedLocalFrameInFrameViewBase
>
>
>
> https://codereview.chromium.org/2727913005/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[haraken, 2017-03-06 02:40:54]

On 2017/03/06 02:30:12, joelhockey wrote:
> On 2017/03/06 at 02:03:59, haraken wrote:
> > On 2017/03/06 02:01:41, joelhockey wrote:
> > > On 2017/03/06 at 01:54:49, haraken wrote:
> > > > On 2017/03/05 22:37:42, joelhockey wrote:
> > > > >
> > >
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > > > File third_party/WebKit/Source/core/frame/FrameView.h (right):
> > > > > 
> > > > >
> > >
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > > > third_party/WebKit/Source/core/frame/FrameView.h:160: void
> > > > > setNeedsUpdateFrameViewBaseGeometries() {
> > > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > > This sounds a bit too verbose. setNeedsUpdateGeometries?
> > > > > 
> > > > > done
> > > > > 
> > > > >
> > >
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > > > third_party/WebKit/Source/core/frame/FrameView.h:272: void
> > > > > updateFrameViewBaseGeometries();
> > > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > > updateGeometries ?
> > > > > 
> > > > > done
> > > > > 
> > > > >
> > >
>
https://codereview.chromium.org/2727913005/diff/40001/third_party/WebKit/Sour...
> > > > > third_party/WebKit/Source/core/frame/FrameView.h:658: IntPoint
> > > > > convertFromContainingFrameViewBaseToScrollbar(
> > > > > On 2017/03/03 at 19:07:34, haraken wrote:
> > > > > > Instead of renaming methods on a per-file basis, it might make more
> sense
> > > to
> > > > > rename them on a per-functionality basis. e.g., you can rename all
> convert*
> > > > > methods in one CL. Then you can rename all updateGeometry things in
one
> CL
> > > etc.
> > > > > 
> > > > > Acknowledged.  I'll try and use the approach for other renames.  I
went
> with
> > > the
> > > > > per-file approach since I don't really know which per-functionality
> methods
> > > > > exist and where they live.  At least with the per-file approach I can
> make
> > > sure
> > > > > I get everything.  In the end, I figure that the renames will happen
in
> very
> > > > > quick succession and the code ends up in the same end-state.
> > > > 
> > > > But landing this CL would make the subsequent reviews harder. For
example,
> > > you're renaming FrameView::updateWidgetGeometries in this CL but not
> renaming
> > > LayoutPart::updateWidgetGeometry. Then we need to be careful not to rename
> > > LayoutPart::updateWidgetGeometry to
LayoutPart::updateFrameBaseViewGeometry
> when
> > > we rename it.
> > > > 
> > > > Note that what we want to do is not a simple mechanical renaming (then
you
> can
> > > do that in one go) but improving the names so that they make more sense in
> the
> > > code base. That will need some code inspection on a case-by-case basis.
> > > 
> > > Do you want me to continue with this CL?  Should I add LayoutPart, or any
> other
> > > files?  I have another CL just about ready which does include LayoutPart,
> and I
> > > have kept the *Geometry renaming consistent with this CL.
> > 
> > Is it the last part? If we still need to rename a bunch of more things, I'd
> prefer switching to the per-functionality renaming now.
> 
> There are 19 more methods that I count to be changed.  I am currently planning
> to put them all in the next, final CL

Then I'd prefer splitting the CLs into pieces. Also let's try to rename the
methods in a more meaningful way rather than doing simple renaming -- this is an
opportunity to improve the code base :)

> 
> core/dom/ContainerNode:
>  - suspendWidgetHierarychyUpdates -> suspendFrameViewBaseHierarchyUpdates
> 
> core/dom/Document:
>  - widgetForElement -> frameViewBaseForElement
> 

As one example, this should be renamed with LayoutObject::widget(). Also you'll
notice that widgetForElement doesn't need to be a method of Document. It should
be moved to Element::frameViewBase().

> core/html/HTMLEmbedElement:
>  - updateWidgetInternal -> updateFrameViewBaseInternal
> 
> core/html/HTMLFrameOwnerElement:
>  - setWidget -> setFrameViewBase
>  - releaseWidget -> releaseFrameViewBase
>  - ownedWidget -> ownedFrameViewBase
>  - widgetNewParentMap -> frameViewBaseNewParentMap
> 
> core/html/HTMLPlugInElement
>  - pluginWidget -> pluginFrameViewBase
>  - setNeedsWidgetUpdate -> setNeedsFrameViewBaseUpdate
>  - setPersistedPluginWidget -> setPersistedPluginFrameViewBase
>  - updateWidget -> updateFrameViewBase
> 
> core/html/PluginDocument:
>  - pluginWidget -> pluginFrameViewBase
> 
> core/input/ScrollManager:
>  - passScrollGestureEventToWidget -> passScrollGestureEventToFrameViewBase
> 
> core/layout/HitTestResult:
>  - isOverWidget -> isOverFrameViewBase
>  - setIsOverWidget -> setIsOverFrameViewBase
> 
> core/layout/LayoutPart:
>  - updateOnWidgetChange -> updateOnFrameViewBaseChange
>  - updateWidgetGeometry -> updateGeometry
>  - updateWidgetGeometryInternal -> updateGeometryInternal
> 
> web/WebViewImpl
>  - focusedLocalFrameInWidget -> focussedLocalFrameInFrameViewBase

[commit-bot: I haz the power, 2017-03-06 03:07:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-06 03:07:43]

Dry run: This issue passed the CQ dry run.

[joelhockey, 2017-03-13 04:36:26]

Description was changed from

==========
Rename FrameView fields and methods *Widget* to ...FrameViewBase...

BUG=697351
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Rename FrameView fields and methods *Widget* to ...FrameViewBase...

BUG=697351
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========