New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 686897 link

Starred by 2 users
Status: Fixed
Owner:
majidvp@chromium.org
Closed: Jun 2017
Cc:
flackr@chromium.org
pdr@chromium.org
ajuma@chromium.org
chrishtr@chromium.org
█ wkorman@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , All
Pri: 2
Type: Feature

Blocking:
issue 674317



Sign in to add a comment

Support CompositorWorker for SPv2.

Project Member Reported by wkorman@chromium.org, Jan 30 2017 
Breakout from  http://crbug.com/674317 . We've not yet ported some logic from CompositedLayerMapping::updateElementIdAndCompositorMutableProperties() around setting element id and CompositorMutableProperties in the presence of compositor worker enabled. I expect we'll need to add a field(s) for CompositorMutableProperty to paint property node(s), and add some logic to PaintPropertyTreeBuilder to populate.

Before doing this work I thought to checkpoint and make sure the Compositor Worker feature is still planned to build atop the existing implementation.

Should we do this porting work? Guessing a couple of days to port and write/update tests, potential for work discovery if for example changes we've made to when we call updateAnimations() for SPv2 somehow have deeper lifecycle ramifications for compositor worker.

Involved failing tests are:

virtual/threaded/fast/compositorworker/visual-update.html [ Timeout ]
virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html [ Crash Failure ]
virtual/threaded/fast/compositorworker/basic-plumbing-worker-to-main.html [ Crash Failure ]
virtual/threaded/fast/compositorworker/compositor-attribute-change-worker.html [ Timeout ]
virtual/threaded/fast/compositorworker/compositor-proxy-disconnect-worker.html [ Timeout ]

Comment 1 by flackr@chromium.org, Jan 30 2017 

Yes, AnimationWorklet is going to use roughly the same infrastructure to update composited properties. There are a few planned changes but I don't think they'll conflict with porting the existing code.
1. We are not going to have animation worklets directly mutate the properties, but instead record a bunch of mutations which are applied afterwards (they could be shipped to another thread).
2. I believe Majid plans to associate proxies with animations instead of elements.

Comment 2 by wkorman@chromium.org, Jan 30 2017 

OK, thanks. Do you have a sense of timeframe for AnimationWorklet, as in are we working on it right now, or when do we expect the work to happen roughly?

Note to self -- there is also some logic in PaintLayerCompositor::updateIfNeeded that reads as if we should set scrolling CompositorMutableProperties on a layer if it has a scroll node.
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 31 2017 

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

commit 42bd2b1a523f7c73cdade1e528703e3a1e9b768f
Author: wkorman <wkorman@chromium.org>
Date: Tue Jan 31 03:57:01 2017

Enable half of virtual/threaded tests for SPv2.

BUG= 674317 , 686897 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2667743002
Cr-Commit-Position: refs/heads/master@{#447182}

[modify] https://crrev.com/42bd2b1a523f7c73cdade1e528703e3a1e9b768f/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2

Comment 4 by wkorman@chromium.org, May 11 2017

Cc: -chrishtr@chromium.org wkorman@chromium.org
Owner: chrishtr@chromium.org
Not yet 100% whether we need to port the Compositor Worker logic. Should Paint team do the port work, or given c#1 seems to be saying the existing infra will see some notable changes, should we pass to Majid? Passing to Chris for input.

Comment 5 by majidvp@chromium.org, Jun 5 2017

Cc: -majidvp@chromium.org chrishtr@chromium.org
Owner: majidvp@chromium.org
After recent changes to the AnimationWorklet API, its design is now heavily uses the existing animation
machinery in Blink and CC. In this new world, we will not necessarily need to use compositor proxy for
plumbing animations and mutations from Blink<=>CC<=>Worklet. So I think it is reasonable to just start
removing Compositor Proxies specially that I am told it is preventing certain clean up on paint side.
This means that CompositorWorker will not actually be usable anymore but I don't think that is a
big loss at this point given that it is being replaced with AnimationWorklet.



Note that we still plan to use the pre-existing CompositorWorker plumbing for sending a Mutate signal
from CC to threaded worklet scope but that does not depend on having Compositor Proxies.

Comment 6 by chrishtr@chromium.org, Jun 5 2017 

Are you planning on removing it now then? Or should I put up a patch?

Comment 7 by majidvp@chromium.org, Jun 5 2017 

I was planning to do it by EOW but feel free to put up a patch if you want it
done sooner.

Comment 8 by majidvp@chromium.org, Jun 7 2017

Status: Started (was: Assigned)
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 22 2017 

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

commit 09ec44387526f6a420044a749cdcb63bb00595c1
Author: Majid Valipour <majidvp@chromium.org>
Date: Thu Jun 22 21:16:25 2017

Remove CompositorProxy, CompositorMutableState and friends

CompositorProxy is a used by CompositorWorker which is being superseded by
AnimationWorklet. Given that AnimationWorklet is not going to use
CompositorProxy we are removing it and other related classes.

This also means removing tests that where mainly associated with  CompsitorProxy
functionality.


Bug:  686897 ,  619042 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I7fa749c83e74fa65e8271d2111ebc9ae2e872ebb
Reviewed-on: https://chromium-review.googlesource.com/527455
Commit-Queue: Majid Valipour <majidvp@chromium.org>
Reviewed-by: Ojan Vafai <ojan@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Chris harrelson <chrishtr@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481663}
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/LayoutTests/FlagExpectations/root-layer-scrolls
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/LayoutTests/SmokeTests
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/LayoutTests/TestExpectations
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/fast/dom/CompositorProxy/proxy-forces-layer.html
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/LayoutTests/virtual/service-worker-navigation-preload-disabled/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-worker-to-main.html
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/compositor-attribute-change-worker.html
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/compositor-attribute-change.html
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/compositor-proxy-disconnect-worker.html
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/compositor-proxy-disconnect.html
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/compositor-proxy-postmessage.html
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/compositor-proxy-supports.html
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/basic-plumbing-main-to-worker.js
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/basic-plumbing-worker-to-main.js
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/proxy-disconnect.js
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/proxy-echo.js
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/proxy-mutate.js
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-compositor-worker-expected.txt
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/bindings/core/v8/serialization/SerializationTag.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueDeserializer.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializer.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/bindings/core/v8/serialization/V8ScriptValueSerializerTest.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/animation/BUILD.gn
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/animation/CompositorAnimator.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/animation/CompositorMutatorImpl.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/animation/CompositorMutatorImpl.h
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/core/animation/CompositorProxyClientImpl.cpp
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/core/animation/CompositorProxyClientImpl.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/core_idl_files.gni
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/css/ComputedStyleExtraFields.json5
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/css/resolver/StyleAdjuster.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/dom/AnimationWorkletProxyClient.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/dom/BUILD.gn
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/core/dom/CompositorProxiedPropertySet.cpp
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/core/dom/CompositorProxiedPropertySet.h
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/core/dom/CompositorProxy.cpp
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/core/dom/CompositorProxy.h
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/core/dom/CompositorProxy.idl
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/core/dom/CompositorProxyClient.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/dom/CompositorWorkerProxyClient.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/dom/Element.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/dom/Element.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/dom/ElementRareData.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/dom/ElementRareData.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/dom/StyleChangeReason.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/dom/StyleChangeReason.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/layout/LayoutInline.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/layout/LayoutTableRow.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/layout/compositing/CompositedLayerMapping.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/layout/compositing/CompositingReasonFinder.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/core/paint/PaintLayerScrollableArea.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletProxyClientImpl.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletProxyClientImpl.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/modules/compositorworker/AnimationWorkletThreadTest.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerProxyClientImpl.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerProxyClientImpl.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/modules/compositorworker/CompositorWorkerThreadTest.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/platform/graphics/CompositingReasons.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/platform/graphics/CompositingReasons.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/platform/graphics/CompositorElementId.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/platform/graphics/CompositorElementId.h
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/platform/graphics/CompositorMutableState.cpp
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/platform/graphics/CompositorMutableState.h
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.cpp
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.h
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/platform/graphics/CompositorMutableStateTest.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/platform/graphics/CompositorMutator.h
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/platform/graphics/CompositorMutatorClientTest.cpp
[modify] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/web/tests/CompositorWorkerTest.cpp
[delete] https://crrev.com/3e164f76405e21777fae49bd221e8d86e7df4bfd/third_party/WebKit/Source/web/tests/data/compositor-proxy-basic.html
[add] https://crrev.com/09ec44387526f6a420044a749cdcb63bb00595c1/third_party/WebKit/Source/web/tests/data/compositor-worker-basic.html

Comment 10 by majidvp@chromium.org, Jun 26 2017

Status: Fixed (was: Started)
►

Sign in to add a comment