Don't call LayoutPart::widgetPositionsUpdated unnecessarily.

Don't call LayoutPart::widgetPositionsUpdated unnecessarily.

This reduces unnecessary geometry recomputation in WebPluginContainerImpl
by a factor of two.

BUG=555201
Committed: https://crrev.com/a86973e6a92be4540c7d1229e7b3be0148bafd2f
Cr-Commit-Position: refs/heads/master@{#365340}

[chrishtr, 2015-12-12 01:45:25]

Description was changed from

==========
none


none


none

BUG=
==========

to

==========
Don't call LayoutPart::widgetPositionsUpdated unless they were.

This reduces unnecessary geometry recomputation in WebPluginContainerImpl
significantly when layout changes don't affect the plugin's geometry.

BUG=555201
==========

[chrishtr, 2015-12-12 05:08:25]

chrishtr@chromium.org changed reviewers:
+ wkorman@chromium.org

[chrishtr, 2015-12-12 05:08:26]

When loading
http://chrome.blogspot.com/2009/04/11-short-films-about-browser.html with this
patch, the plugin geometry is recomputed 5 times instead of 11. The other ones
were not
necessary.

[chrishtr, 2015-12-12 05:08:54]

https://codereview.chromium.org/1513573013/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/1513573013/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:1162: if (wasUpdated[i])
This is the money line in this patch. The rest is renames.

[wkorman, 2015-12-12 06:54:55]

lgtm

https://codereview.chromium.org/1513573013/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/1513573013/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:1156:
wasUpdated.resize(parts.size());
Can we initialize it with the right size from the get-go with:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

https://codereview.chromium.org/1513573013/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:1163:
parts[i]->widgetGeometriesUpdated();
Re: the comment at line 1154, just double-checking that this can't result in
losing our layoutView(), thus necessitating that we keep that check in the for
loop?

[chrishtr, 2015-12-14 22:45:48]

Description was changed from

==========
Don't call LayoutPart::widgetPositionsUpdated unless they were.

This reduces unnecessary geometry recomputation in WebPluginContainerImpl
significantly when layout changes don't affect the plugin's geometry.

BUG=555201
==========

to

==========
Don't call LayoutPart::widgetPositionsUpdated unnecessarily.

This reduces unnecessary geometry recomputation in WebPluginContainerImpl
by a factor of two.

BUG=555201
==========

[chrishtr, 2015-12-14 22:58:34]

The previous version of the CL was wrong. Even though the position of a widget
within its
frame doesn't change, the visual screen rect (which includes clipping and nested
frames)
may change.

Change the patch to just avoid updating geometry twice for every update.

As for checking layoutView() in the for loop, turns out this is there to avoid
crashes.
See:
https://chromium.googlesource.com/chromium/src/+/9f17f9d60ffde762f0e6483083bd...

[chrishtr, 2015-12-14 22:58:47]

PTAL.

[wkorman, 2015-12-14 23:22:10]

lgtm

https://codereview.chromium.org/1513573013/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebPluginContainerImpl.cpp (left):

https://codereview.chromium.org/1513573013/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebPluginContainerImpl.cpp:108: reportGeometry();
So this was just spurious over-reporting, and reportGeometry() always still ends
up being called through eventual calls into
WebPluginContainerImpl::widgetGeometryMayHaveChanged() instead?

[chrishtr, 2015-12-14 23:41:18]

On 2015/12/14 at 23:22:10, wkorman wrote:
> lgtm
> 
>
https://codereview.chromium.org/1513573013/diff/80001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/web/WebPluginContainerImpl.cpp (left):
> 
>
https://codereview.chromium.org/1513573013/diff/80001/third_party/WebKit/Sour...
> third_party/WebKit/Source/web/WebPluginContainerImpl.cpp:108:
reportGeometry();
> So this was just spurious over-reporting, and reportGeometry() always still
ends up being called through eventual calls into
WebPluginContainerImpl::widgetGeometryMayHaveChanged() instead?

Yes, I think so.

The only call sites are two places in FrameView. Just realized I forgot to add a
line to the second one (FrameView::updateWidgets). Added that.

[chrishtr, 2015-12-14 23:41:22]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2015-12-14 23:41:42]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2015-12-14 23:41:42]

The patchset sent to the CQ was uploaded after l-g-t-m from wkorman@chromium.org
Link to the patchset: https://codereview.chromium.org/1513573013/#ps100001
(title: " ")

[commit-bot: I haz the power, 2015-12-14 23:43:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1513573013/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1513573013/100001

[commit-bot: I haz the power, 2015-12-15 00:02:01]

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

[commit-bot: I haz the power, 2015-12-15 00:02:02]

Try jobs failed on following builders:
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)

[chrishtr, 2015-12-15 19:22:14]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2015-12-15 19:22:14]

The patchset sent to the CQ was uploaded after l-g-t-m from wkorman@chromium.org
Link to the patchset: https://codereview.chromium.org/1513573013/#ps120001
(title: " ")

[commit-bot: I haz the power, 2015-12-15 19:22:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1513573013/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1513573013/120001

[commit-bot: I haz the power, 2015-12-15 21:44:04]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-12-15 21:45:06]

Description was changed from

==========
Don't call LayoutPart::widgetPositionsUpdated unnecessarily.

This reduces unnecessary geometry recomputation in WebPluginContainerImpl
by a factor of two.

BUG=555201
==========

to

==========
Don't call LayoutPart::widgetPositionsUpdated unnecessarily.

This reduces unnecessary geometry recomputation in WebPluginContainerImpl
by a factor of two.

BUG=555201

Committed: https://crrev.com/a86973e6a92be4540c7d1229e7b3be0148bafd2f
Cr-Commit-Position: refs/heads/master@{#365340}
==========

[commit-bot: I haz the power, 2015-12-15 21:45:07]

Patchset 7 (id:??) landed as
https://crrev.com/a86973e6a92be4540c7d1229e7b3be0148bafd2f
Cr-Commit-Position: refs/heads/master@{#365340}