New issue
Advanced search Search tips

Issue 857479 link

Starred by 1 user
Status: Fixed
Owner:
majidvp@chromium.org
Closed: Jul 4
Cc:
petermayo@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment

[animationworklet] AnimationWorklet declared in child frame may override animations in parent

Project Member Reported by flackr@chromium.org, Jun 28 2018 
Chrome Version: 69.0.3471.0
OS: all

What steps will reproduce the problem?
(1) Run chrome with --experimental-web-platform-features
(2) Load attached webpage (served over https)

What is the expected result?
Expect to see both animated divs translated 200px to the right.

What happens instead?
Instead, sometimes the top one is only translated 100px because the included iframe defines the same animator and we only use one of the global scopes.

I had expected that if we weren't careful we might also fail to create test_animator2 but this works for some reason. This might also be problematic as it may mean we're running code from the main frame in the child frame.
worklet-test.zip
1.2 KB Download
Project Member

Comment 1 by sheriffbot@chromium.org, Jun 28 2018

Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 4 

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

commit c035b5fca9ea751e018c0d0026ee30374d104a84
Author: Majid Valipour <majidvp@chromium.org>
Date: Wed Jul 04 15:17:21 2018

[animation-worket] Ensure each worklet receives only its own input

Previously we would allow worklet to access and act on all of worklet inputs.
This is bad since it can leak information between different animation
worklet global scopes.

Summary of the fix:
- Introduce WorkletAnimationId which can uniquely identify both the worklet and animation.
- Give each worklet animation its own Id generated using its underlying worklet's scope.
- Plumb this new Id to cc.
- AnimationHost produces input state which is bucketed per scope.
- CompositorMutatorImpl only passes to each compositor animator the input.
  bucket that matches that compositor animator's scope Id.
- For additional safety added DCHECK to verify that each scope is receiving only its
  own input.

TEST: compositor_mutator_impl_test.cc
Bug:  857479 

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ib4b8f610ef661bdbf09c4c0252a2b9c4ba04db16
Reviewed-on: https://chromium-review.googlesource.com/1120436
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572572}
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/cc/animation/animation_host.cc
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/cc/animation/worklet_animation.cc
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/cc/animation/worklet_animation.h
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/cc/animation/worklet_animation_unittest.cc
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/cc/trees/layer_tree_mutator.cc
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/cc/trees/layer_tree_mutator.h
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/modules/animationworklet/animation_worklet.cc
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/modules/animationworklet/animation_worklet.h
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/modules/animationworklet/animation_worklet_global_scope.cc
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/modules/animationworklet/animation_worklet_global_scope.h
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/modules/animationworklet/animation_worklet_global_scope_test.cc
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/modules/animationworklet/animation_worklet_proxy_client_impl.cc
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/modules/animationworklet/animation_worklet_proxy_client_impl.h
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/modules/animationworklet/worklet_animation.cc
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/modules/animationworklet/worklet_animation.h
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/modules/animationworklet/worklet_animation.idl
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/modules/animationworklet/worklet_animation_test.cc
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/platform/BUILD.gn
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/platform/animation/compositor_animation.cc
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/platform/animation/compositor_animation.h
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/platform/graphics/compositor_animator.h
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/platform/graphics/compositor_animators_state.h
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/platform/graphics/compositor_mutator_client.h
[modify] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/platform/graphics/compositor_mutator_impl.cc
[add] https://crrev.com/c035b5fca9ea751e018c0d0026ee30374d104a84/third_party/blink/renderer/platform/graphics/compositor_mutator_impl_test.cc

Comment 3 by majidvp@chromium.org, Jul 4

Status: Fixed (was: Assigned)
Project Member

Comment 4 by sheriffbot@chromium.org, Jul 5

Labels: Restrict-View-SecurityNotify
Project Member

Comment 5 by sheriffbot@chromium.org, Oct 11

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
►

Sign in to add a comment