New issue
Advanced search Search tips

Issue 872773 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 6
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Mac
Pri: 2
Type: Bug

Blocking:
issue 836897



Sign in to add a comment

[BGPT] Implement headless animation updates (http://bit.ly/bfc-animations) with --enable-blink-gen-property-trees

Project Member Reported by pdr@chromium.org, Aug 9

Issue description

Headless chrome has an update that runs layout and animations but not paint. This is described in http://bit.ly/bfc-animations. This does not currently work with --enable-blink-gen-property-trees because animations are updated during paint.

In cc/trees/layer_tree_host_client.h there's an enum "VisualStateUpdate { kPrePaint, kAll };" and a comment that says kPrePaint updates layout & animations. We will need to either remove the kPrePaint flag or find a way to do the animation update without paint.
 
Status: Available (was: Untriaged)
Labels: Test-Layout
Labels: -Test-Layout Hotlist-Experimental
Summary: [BGPT] Implement headless animation updates (http://bit.ly/bfc-animations) with --enable-blink-gen-property-trees (was: Implement headless animation updates (http://bit.ly/bfc-animations) with --enable-blink-gen-property-trees)
This isn't a layout test issue, it's a bug with not updating animations with --blink-gen-property-trees for headless chrome due to animation updates happening during paint.
Rob, would you be willing to look into this?

I can think of a few approaches to solving this but I am not sure which would be feasible:
1) Remove the animations-only update and do a full update. This requires better understanding the original motivation.
2) Make the animations-only update do a full lifecycle update but disable frame generation.
3) Add a special animations-only update to just tick animations for BlinkGenPropertyTrees.
Cc: pfeldman@chromium.org
Owner: flackr@chromium.org
Status: Started (was: Available)
Pavel, do you have tests which I can use to ensure that we don't break headless chrome? How significant is the performance impact of running full lifecycle updates? It seems likely to me that not doing full lifecycle updates may be incorrect in other ways, for example by never actually starting composited animations.

It seems like the safest thing would be to do a full lifecycle update, but given test cases that we can benchmark on I'm open to looking into alternatives.
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca

commit 3ef2ffbb5222d9074728217c0c6db9b4c07a69ca
Author: Robert Flack <flackr@chromium.org>
Date: Thu Sep 06 20:38:20 2018

Modify headless animate_only behavior to run paint.

Animations updates with BlinkGenPropertyTrees require paint in order to have animations
updated. This modifies the animate_only property to not skip paint but
only skip commit.

BlinkGenPropertyTrees changes the blink lifecycle such that painting produces the
property trees which are then directly passed to the compositor. As a result, the
animations update can only happen after we know the painted artifacts which means
that animation updates are now dependent on running paint.

Bug:  872773 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I0d2424b3b658a0908a08adfce20940610cdd9b09
Reviewed-on: https://chromium-review.googlesource.com/1199603
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Eric Seckler <eseckler@chromium.org>
Commit-Queue: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589281}
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/test/layer_tree_test.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/test/stub_layer_tree_host_client.h
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/test/test_hooks.h
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/trees/layer_tree_host.h
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/trees/layer_tree_host_client.h
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/trees/layer_tree_host_perftest.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/trees/layer_tree_host_unittest_animation.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/trees/layer_tree_host_unittest_scroll.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/trees/proxy_main.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/cc/trees/single_thread_proxy.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/content/renderer/gpu/layer_tree_view.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/content/renderer/gpu/layer_tree_view.h
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/content/renderer/gpu/layer_tree_view_delegate.h
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/content/renderer/render_widget.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/content/renderer/render_widget.h
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/content/test/stub_layer_tree_view_delegate.h
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/ui/compositor/compositor.cc
[modify] https://crrev.com/3ef2ffbb5222d9074728217c0c6db9b4c07a69ca/ui/compositor/compositor.h

Status: Fixed (was: Started)
Option 2 mentioned in #4 above has landed. Marking fixed.

Sign in to add a comment