Decouple scheduling animation of webview plugins from layout.

Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty.
This can lead to not running the webview lifecyle in some cases.
 https://codereview.chromium.org/1708923002 fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual animations,
such as issuing resize events to script. It is valid to queue up async script events
during a lifecycle update. (In one case in the referenced bug, FrameView::
 sendResizeEventIfNeeded does this when it notices that layout changed the size of the frame).
This CL fixes cases such as that, and builds up on the one fixing issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli,bbudge,fsamuel

Committed: https://crrev.com/78ea22a8529ec7ac2ea07d7e69ddd1446f0ed262
Cr-Commit-Position: refs/heads/master@{#379694}

[chrishtr, 2016-03-07 21:48:07]

Description was changed from

==========
none

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

to

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty. This can lead to not running
the webview lifecyle in some cases. https://codereview.chromium.org/1708923002
fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame). This CL fixes cases such as that, and builds up on the one fixing
issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
==========

[chrishtr, 2016-03-07 21:49:23]

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

[chrishtr, 2016-03-07 21:49:24]

https://codereview.chromium.org/1774653002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutEmbeddedObject.cpp (right):

https://codereview.chromium.org/1774653002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutEmbeddedObject.cpp:139:
toPluginView(widget)->updateAllLifecyclePhases();
I'm leaving this in here out of caution, since this CL will need to be merged.

[chrishtr, 2016-03-07 21:49:35]

Description was changed from

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty. This can lead to not running
the webview lifecyle in some cases. https://codereview.chromium.org/1708923002
fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame). This CL fixes cases such as that, and builds up on the one fixing
issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
==========

to

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty. This can lead to not running
the webview lifecyle in some cases. https://codereview.chromium.org/1708923002
fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame). This CL fixes cases such as that, and builds up on the one fixing
issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli
==========

[chrishtr, 2016-03-07 21:49:53]

Description was changed from

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty. This can lead to not running
the webview lifecyle in some cases. https://codereview.chromium.org/1708923002
fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame). This CL fixes cases such as that, and builds up on the one fixing
issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli
==========

to

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty. This can lead to not running
the webview lifecyle in some cases. https://codereview.chromium.org/1708923002
fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame).
This CL fixes cases such as that, and builds up on the one fixing issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli
==========

[chrishtr, 2016-03-07 21:50:08]

Description was changed from

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty. This can lead to not running
the webview lifecyle in some cases. https://codereview.chromium.org/1708923002
fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame).
This CL fixes cases such as that, and builds up on the one fixing issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli
==========

to

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty.
This can lead to not running the webview lifecyle in some cases.
https://codereview.chromium.org/1708923002 fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame).
This CL fixes cases such as that, and builds up on the one fixing issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli
==========

[chrishtr, 2016-03-07 21:50:17]

Description was changed from

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty.
This can lead to not running the webview lifecyle in some cases.
https://codereview.chromium.org/1708923002 fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame).
This CL fixes cases such as that, and builds up on the one fixing issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli
==========

to

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty.
This can lead to not running the webview lifecyle in some cases.
 https://codereview.chromium.org/1708923002 fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame).
This CL fixes cases such as that, and builds up on the one fixing issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli
==========

[chrishtr, 2016-03-07 21:51:53]

Description was changed from

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty.
This can lead to not running the webview lifecyle in some cases.
 https://codereview.chromium.org/1708923002 fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame).
This CL fixes cases such as that, and builds up on the one fixing issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli
==========

to

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty.
This can lead to not running the webview lifecyle in some cases.
 https://codereview.chromium.org/1708923002 fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame).
This CL fixes cases such as that, and builds up on the one fixing issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli,bbudge,fsamuel
==========

[chrishtr, 2016-03-07 21:52:17]

I'm no longer able to crash deepika.com with this CL.

[Xianzhu, 2016-03-07 21:53:04]

lgtm

https://codereview.chromium.org/1774653002/diff/1/components/plugins/renderer...
File components/plugins/renderer/webview_plugin.cc (right):

https://codereview.chromium.org/1774653002/diff/1/components/plugins/renderer...
components/plugins/renderer/webview_plugin.cc:137: // for layout again, and it
does help us in the race-condition situation.
Nit: does the above comment need update?

[chrishtr, 2016-03-07 22:23:43]

https://codereview.chromium.org/1774653002/diff/1/components/plugins/renderer...
File components/plugins/renderer/webview_plugin.cc (right):

https://codereview.chromium.org/1774653002/diff/1/components/plugins/renderer...
components/plugins/renderer/webview_plugin.cc:137: // for layout again, and it
does help us in the race-condition situation.
On 2016/03/07 at 21:53:04, Xianzhu wrote:
> Nit: does the above comment need update?

Updated layout to animation.

[chrishtr, 2016-03-07 22:24:36]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2016-03-07 22:24:36]

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

[commit-bot: I haz the power, 2016-03-07 22:24:54]

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

[commit-bot: I haz the power, 2016-03-07 22:33:47]

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

[commit-bot: I haz the power, 2016-03-07 22:33:48]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[chrishtr, 2016-03-07 23:09:23]

The CQ bit was checked by chrishtr@chromium.org

[commit-bot: I haz the power, 2016-03-07 23:09:58]

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

[commit-bot: I haz the power, 2016-03-08 01:11:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-08 01:13:40]

Description was changed from

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty.
This can lead to not running the webview lifecyle in some cases.
 https://codereview.chromium.org/1708923002 fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView:: 
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame).
This CL fixes cases such as that, and builds up on the one fixing issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli,bbudge,fsamuel
==========

to

==========
Decouple scheduling animation of webview plugins from layout.

Previously, any time the webview plugin needed to schedule animation, it dirtied
layout in the containing frame. This was because it used to be the case that
LayoutEmbeddedObject would run the webview plugin's lifecycle as part of layout.
However, this is not right, for two reasons:

1. A  lifecycle update for later phases  may be needed for the plugin due to
changes in the parent, even if layout is not dirty.
This can lead to not running the webview lifecyle in some cases.
 https://codereview.chromium.org/1708923002 fixed
this by always running the lifecycle of the webview plugin when the parent does.

2. Animation is scheduled for two reasons: lifecyle update, and actual
animations,
such as issuing resize events to script. It is valid to queue up async script
events
during a lifecycle update. (In one case in the referenced bug, FrameView::
 sendResizeEventIfNeeded does this when it notices that layout changed the size
of the frame).
This CL fixes cases such as that, and builds up on the one fixing issue #1,
by scheduling animation of the parent frame but not dirtying layout.

BUG=590856
TBR=tommycli,bbudge,fsamuel

Committed: https://crrev.com/78ea22a8529ec7ac2ea07d7e69ddd1446f0ed262
Cr-Commit-Position: refs/heads/master@{#379694}
==========

[commit-bot: I haz the power, 2016-03-08 01:13:41]

Patchset 2 (id:??) landed as
https://crrev.com/78ea22a8529ec7ac2ea07d7e69ddd1446f0ed262
Cr-Commit-Position: refs/heads/master@{#379694}

[wkorman, 2016-03-08 01:38:49]

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

[wkorman, 2016-03-08 01:38:50]

Is there a feasible path to reducing a test case?

https://codereview.chromium.org/1774653002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutEmbeddedObject.cpp (right):

https://codereview.chromium.org/1774653002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutEmbeddedObject.cpp:137: //
TODO(chrishtr): remove this code. It's now called in
FrameView::updateStyleAndLayoutIfNeededRecursive.
What blocks removing this?

https://codereview.chromium.org/1774653002/diff/20001/third_party/WebKit/publ...
File third_party/WebKit/public/web/WebPluginContainer.h (right):

https://codereview.chromium.org/1774653002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/web/WebPluginContainer.h:69: virtual void
scheduleAnimation() = 0;
If this is the way a new frame is requested, perhaps we should consider renaming
scheduleAnimation at some point, since as currently named I suspect most would
guess it is solely css animation related.

[chrishtr, 2016-03-10 19:06:14]

https://codereview.chromium.org/1774653002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutEmbeddedObject.cpp (right):

https://codereview.chromium.org/1774653002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutEmbeddedObject.cpp:137: //
TODO(chrishtr): remove this code. It's now called in
FrameView::updateStyleAndLayoutIfNeededRecursive.
On 2016/03/08 at 01:38:50, wkorman wrote:
> What blocks removing this?

I mentioned it in the first patchset, but I left this in out of
conservatism for M49 and M50. Going to send a patch to remove it now.

https://codereview.chromium.org/1774653002/diff/20001/third_party/WebKit/publ...
File third_party/WebKit/public/web/WebPluginContainer.h (right):

https://codereview.chromium.org/1774653002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/web/WebPluginContainer.h:69: virtual void
scheduleAnimation() = 0;
On 2016/03/08 at 01:38:50, wkorman wrote:
> If this is the way a new frame is requested, perhaps we should consider
renaming scheduleAnimation at some point, since as currently named I suspect
most would guess it is solely css animation related.

In some cases it isn't even running a frame, it's running async
events for script such as onresize. Still not exactly sure what changes
should be made to clean up frames vs events, I am intending to ask
around to the scheduling team.

[chrishtr, 2016-03-10 19:08:03]

On 2016/03/08 at 01:38:50, wkorman wrote:
> Is there a feasible path to reducing a test case?
> 

Probably yes. I'll try.