New issue
Advanced search Search tips

Issue 746212 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Remove CompositorWorker code

Project Member Reported by nhiroki@chromium.org, Jul 19 2017

Issue description

CompositorWorker is a kind of chimera of Worker and Worklet for historical reasons, and complicates code under core/workers.

CompositorWorker hasn't been shipped yet and will be superseded by AnimationWorklet, so we could remove it at some point.
 

Comment 1 by falken@chromium.org, Jul 19 2017

"we could remove it at some point"

Is anything stopping us from deleting it now? Or it's just that no one has had the time to delete it yet?
Description: Show this description
falken@: I don't know :)

flackr@, majidvp@: Do we still need to keep CompositorWorker? If it's no longer necessary, I'll try to remove it.

Comment 4 by flackr@chromium.org, Jul 19 2017

We have a few end to end layout tests which are ensuring that the CompositorMutatorClient is successfully hooked up from the compositor scheduler to the worker. I'd be a little worried if we removed that testing coverage before we hook this up and test it with AnimationWorklet. Majid, do you know if the AnimationWorklet mutator client connection is done and tested yet?
Owner: majidvp@chromium.org
Status: Assigned (was: Available)
(Let me tentatively assign this to majidvp@ for c#4)
Cc: nhiroki@chromium.org
We already have a test that ensure CompositorMutatorClient client is properly connecting compositor scheduler to animation worklet [1]. The reverse direction is no longer needed and we have started removing some parts of it already. 
So it is fine with me to start removing CompositorWorker. 

nhiroki@: I will take a stab at removing remaining parts of Compositor Worker once that is done perhaps you can remove any left over complications in core/worker code.


[1] virtual/threaded/fast/compositorworker/animation-worklet-animator-animate.html
majidvp@: Thank you! Please reassign this to me once your work is done :)
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 27 2017

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

commit b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8
Author: Peter Mayo <petermayo@chromium.org>
Date: Wed Sep 27 22:56:01 2017

Remove CompositorWorker

The implementing classes and the idl exposing them are removed.
Removed/elevated AbstractAnimationWorkletThread class into actual
AnimationWorkletThread since it is the only one now.

R=majidvp@chromium.org

Bug:  746212 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I0970b5a658fdcbfbb4d45f47be5fd33739778155
Reviewed-on: https://chromium-review.googlesource.com/673484
Reviewed-by: Rick Byers <rbyers@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Quinten Yearsley <qyearsley@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Peter Mayo <petermayo@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Peter Mayo <petermayo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504788}
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/cc/layers/layer.cc
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/cc/layers/layer_impl.cc
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/cc/trees/layer_tree_impl.h
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/cc/trees/proxy_impl.cc
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/cc/trees/proxy_main.cc
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/content/renderer/gpu/render_widget_compositor.cc
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/LayoutTests/SmokeTests
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/LayoutTests/media/track/track-delete-during-setup.html
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/LayoutTests/media/track/track-word-breaking.html
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/LayoutTests/resources/js-test.js
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/request-animation-frame-expected.txt
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/request-animation-frame.html
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/basic-plumbing-main-to-worker.js
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/request-animation-frame.js
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/visual-update.html
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-compositor-worker-expected.txt
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-compositor-worker.html
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializerTest.cpp
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/bindings/idl.gni
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/bindings/scripts/v8_utilities.py
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/animation/CompositorMutatorImpl.cpp
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/animation/CustomCompositorAnimationManager.cpp
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/animation/CustomCompositorAnimations.cpp
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/animation/ScrollTimeline.cpp
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/animation/ScrollTimeline.idl
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/dom/BUILD.gn
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/core/dom/CompositorWorkerProxyClient.cpp
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/core/dom/CompositorWorkerProxyClient.h
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/dom/ExecutionContext.h
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/page/scrolling/SnapCoordinator.cpp
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/core/paint/compositing/CompositorWorkerTest.cpp
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/testing/InternalSettings.cpp
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/testing/InternalSettings.h
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/core/testing/InternalSettings.idl
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/modules/EventTargetModulesFactory.json5
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/modules/ModulesInitializer.cpp
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/AbstractAnimationWorkletThread.cpp
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/AbstractAnimationWorkletThread.h
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletGlobalScope.idl
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletThread.cpp
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletThread.h
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/modules/compositorworker/BUILD.gn
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorker.cpp
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorker.h
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorker.idl
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerGlobalScope.cpp
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerGlobalScope.h
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerGlobalScope.idl
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerMessagingProxy.cpp
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerMessagingProxy.h
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerProxyClientImpl.cpp
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerProxyClientImpl.h
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThread.cpp
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThread.h
[delete] https://crrev.com/f31387ab92b8aa2b3560afc829d370e112b79140/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/modules/compositorworker/WindowAnimationWorklet.idl
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/modules/compositorworker/WorkletAnimation.idl
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/modules/modules_idl_files.gni
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/platform/graphics/CompositorMutator.h
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.cpp
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/platform/runtime_enabled_features.json5
[modify] https://crrev.com/b75ee38fa6267ad54ee638e5d11c49ea4d1cb8f8/third_party/WebKit/Source/platform/testing/RuntimeEnabledFeaturesTestHelpers.h

Comment 9 by flackr@chromium.org, Sep 28 2017

Owner: petermayo@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 18 2017

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

commit 9850c8be9a90d831c089722ef6023bc7f06c9753
Author: Peter Mayo <petermayo@chromium.org>
Date: Wed Oct 18 02:31:16 2017

Remove SetNeedsMutate from CompositorMutatorClient

Removed from CompositorMutatorImpl::RegisterCompositorAnimator,
then it becomes uncalled, and can be cleaned up.  The plumbing
for AnimationWorklet is different, being driven by timelines.
Without SetNeedsMutate, LayerTreeMutatorClient is empty, and
we don't need to register them on AnimationHost s or
CompositorMutatorClient s any more.

BUG= 746212 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I87186826450351876045fd8fa9d0a2b3d1f242d4
Reviewed-on: https://chromium-review.googlesource.com/690805
Reviewed-by: Peter Mayo <petermayo@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: Peter Mayo <petermayo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509662}
[modify] https://crrev.com/9850c8be9a90d831c089722ef6023bc7f06c9753/cc/animation/animation_host.cc
[modify] https://crrev.com/9850c8be9a90d831c089722ef6023bc7f06c9753/cc/animation/animation_host.h
[modify] https://crrev.com/9850c8be9a90d831c089722ef6023bc7f06c9753/cc/trees/layer_tree_mutator.h
[modify] https://crrev.com/9850c8be9a90d831c089722ef6023bc7f06c9753/third_party/WebKit/Source/core/animation/CompositorAnimator.h
[modify] https://crrev.com/9850c8be9a90d831c089722ef6023bc7f06c9753/third_party/WebKit/Source/core/animation/CompositorMutatorImpl.cpp
[modify] https://crrev.com/9850c8be9a90d831c089722ef6023bc7f06c9753/third_party/WebKit/Source/core/animation/CompositorMutatorImpl.h
[modify] https://crrev.com/9850c8be9a90d831c089722ef6023bc7f06c9753/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletProxyClientImpl.cpp
[modify] https://crrev.com/9850c8be9a90d831c089722ef6023bc7f06c9753/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletProxyClientImpl.h
[modify] https://crrev.com/9850c8be9a90d831c089722ef6023bc7f06c9753/third_party/WebKit/Source/platform/graphics/CompositorMutator.h
[modify] https://crrev.com/9850c8be9a90d831c089722ef6023bc7f06c9753/third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.cpp
[modify] https://crrev.com/9850c8be9a90d831c089722ef6023bc7f06c9753/third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.h

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 24 2017

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

commit c588c32ec44034ae51e4a53cb5258b9feb5af853
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Tue Oct 24 17:33:40 2017

Remove unused parameter in AnimationWorklet Mutate callstack

Bug:  746212 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I5af6b63f67ce52468abe20b6438cef3a77d9801d
Reviewed-on: https://chromium-review.googlesource.com/735068
Reviewed-by: Ian Vollick <vollick@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Peter Mayo <petermayo@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511194}
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/cc/animation/animation_host.cc
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/cc/animation/worklet_animation_player_unittest.cc
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/cc/test/mock_layer_tree_mutator.h
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/cc/trees/layer_tree_mutator.h
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/third_party/WebKit/Source/core/animation/CompositorAnimator.h
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/third_party/WebKit/Source/core/animation/CompositorMutatorImpl.cpp
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/third_party/WebKit/Source/core/animation/CompositorMutatorImpl.h
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletProxyClientImpl.cpp
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletProxyClientImpl.h
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/third_party/WebKit/Source/platform/graphics/CompositorMutator.h
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.cpp
[modify] https://crrev.com/c588c32ec44034ae51e4a53cb5258b9feb5af853/third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.h

petermayo@: I think with  issue 776244  fixed there should not be anything left of CompositorWorker in the code. Should we close this?
Status: Fixed (was: Started)

Sign in to add a comment