Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(202)

Issue 2041193005: [compositorworker] compositor proxy mutation updates underlying layers (Closed)

Created:
4 years, 6 months ago by majidvp
Modified:
4 years, 6 months ago
CC:
ajuma+watch_chromium.org, blink-reviews, blink-reviews-dom_chromium.org, blink-reviews-platform-graphics_chromium.org, Rik, cc-bugs_chromium.org, chromium-reviews, danakj+watch_chromium.org, dglazkov+blink, dshwang, drott+blinkwatch_chromium.org, krit, eae+blinkwatch, flackr, f(malita), jbroman, Justin Novosad, kinuko+watch, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney, sof, Ian Vollick
Base URL:
https://chromium.googlesource.com/chromium/src.git@compositor-worker-upstream-registration
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[compositorworker] compositor proxy mutation updates underlying layers The CL makes it so that changes to compositor proxies in compositor worker to actually mutate the underlying layers. This is achieved by passing a reference to active layer tree to LayerTreeMutator. This reference encapsulated by CompositorMutableStateProvider which is responsible for handing out individual CompositorMutableState for each proxy during rAF mutation. Any mutation on the proxy updates the mutable states which in turn is reflected on the underlying layers (main, scroll). major changes: - pass active layer tree impl to mutator - set mutable state for compositor proxy during rAF - add 'initialized' attribute to compositor worker to signal its activation - add layout tests covering visual updates from worker - update most of compositor worker layout tests to use a common pattern TEST=virtual/threaded/fast/compositorworker BUG=430155 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/ee4cbfea75629407af2b0377ca2567540081430d Cr-Commit-Position: refs/heads/master@{#400082}

Patch Set 1 #

Patch Set 2 : Add layout tests #

Patch Set 3 : rebase #

Total comments: 12

Patch Set 4 : rebase #

Patch Set 5 : Fix matrix #

Patch Set 6 : rebase #

Patch Set 7 : rebase #

Patch Set 8 : Replace map[id->set] with set #

Patch Set 9 : rebase #

Patch Set 10 : clean up visual-update test #

Patch Set 11 : Fix web-exposed layout test and minor update #

Patch Set 12 : Fix inlcudes #

Total comments: 16

Patch Set 13 : Address feedback #

Patch Set 14 : raise if not initialized #

Total comments: 2

Patch Set 15 : Fix layout test and use common style in all tests #

Total comments: 2

Patch Set 16 : Use RAII pattern #

Unified diffs Side-by-side diffs Delta from patch set Stats (+414 lines, -83 lines) Patch
M cc/animation/layer_tree_mutator.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M cc/trees/layer_tree_host_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +1 line, -1 line 0 comments Download
A third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +66 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-worker-to-main.html View 1 2 1 chunk +45 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/compositor-proxy-disconnect-worker-terminate.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +2 lines, -1 line 0 comments Download
A third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/basic-plumbing-main-to-worker.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +16 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/basic-plumbing-worker-to-main.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +17 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/proxy-disconnect.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +8 lines, -4 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/proxy-echo.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +7 lines, -7 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/proxy-idle.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +3 lines, -4 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/proxy-mutate.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +15 lines, -10 lines 0 comments Download
M third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/request-animation-frame.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -1 line 0 comments Download
A third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/visual-update.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +13 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/resources/worker-common.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +10 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/visual-update.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +56 lines, -0 lines 0 comments Download
A third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/visual-update-expected.html View 1 2 3 4 5 6 7 8 9 1 chunk +46 lines, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/global-interface-listing-compositor-worker-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/CompositorProxy.h View 1 2 3 4 5 6 5 chunks +5 lines, -7 lines 0 comments Download
M third_party/WebKit/Source/core/dom/CompositorProxy.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 8 chunks +21 lines, -17 lines 0 comments Download
M third_party/WebKit/Source/core/dom/CompositorProxy.idl View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DOMMatrix.h View 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/dom/DOMMatrix.cpp View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +4 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/CompositorMutableStateProvider.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +10 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/CompositorMutableStateTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 4 chunks +5 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/CompositorMutator.h View 2 chunks +3 lines, -1 line 0 comments Download
M third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.h View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/platform/graphics/CompositorMutatorClient.cpp View 1 2 3 4 5 6 7 8 9 10 2 chunks +7 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/CompositorMutatorClientTest.cpp View 1 chunk +5 lines, -1 line 0 comments Download
M third_party/WebKit/Source/web/CompositorMutatorImpl.h View 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/web/CompositorMutatorImpl.cpp View 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/web/CompositorProxyClientImpl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 2 chunks +2 lines, -1 line 0 comments Download
M third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 4 chunks +31 lines, -4 lines 0 comments Download

Depends on Patchset:

Messages

Total messages: 31 (12 generated)
majidvp
4 years, 6 months ago (2016-06-07 23:07:20 UTC) #5
jbroman
some initial comments, but this depends on the other CL, of course https://codereview.chromium.org/2041193005/diff/40001/third_party/WebKit/Source/core/dom/CompositorProxy.cpp File third_party/WebKit/Source/core/dom/CompositorProxy.cpp ...
4 years, 6 months ago (2016-06-08 14:59:38 UTC) #6
majidvp
https://codereview.chromium.org/2041193005/diff/40001/third_party/WebKit/Source/core/dom/CompositorProxy.h File third_party/WebKit/Source/core/dom/CompositorProxy.h (right): https://codereview.chromium.org/2041193005/diff/40001/third_party/WebKit/Source/core/dom/CompositorProxy.h#newcode15 third_party/WebKit/Source/core/dom/CompositorProxy.h:15: #include "wtf/PtrUtil.h" On 2016/06/08 14:59:38, jbroman wrote: > Unused ...
4 years, 6 months ago (2016-06-08 21:50:10 UTC) #7
jbroman
https://codereview.chromium.org/2041193005/diff/40001/third_party/WebKit/Source/core/dom/CompositorProxy.h File third_party/WebKit/Source/core/dom/CompositorProxy.h (right): https://codereview.chromium.org/2041193005/diff/40001/third_party/WebKit/Source/core/dom/CompositorProxy.h#newcode55 third_party/WebKit/Source/core/dom/CompositorProxy.h:55: void takeCompositorMutableState(std::unique_ptr<CompositorMutableState>); On 2016/06/08 at 21:50:09, majidvp wrote: > ...
4 years, 6 months ago (2016-06-08 21:54:56 UTC) #8
majidvp
jbroman@: PTAL On 2016/06/08 21:54:56, jbroman wrote: > https://codereview.chromium.org/2041193005/diff/40001/third_party/WebKit/Source/core/dom/CompositorProxy.h > File third_party/WebKit/Source/core/dom/CompositorProxy.h (right): > > ...
4 years, 6 months ago (2016-06-13 18:53:28 UTC) #9
jbroman
https://codereview.chromium.org/2041193005/diff/220001/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html File third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html (right): https://codereview.chromium.org/2041193005/diff/220001/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html#newcode27 third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html:27: var test = async_test('Tests that a change from the ...
4 years, 6 months ago (2016-06-13 19:45:17 UTC) #10
majidvp
https://codereview.chromium.org/2041193005/diff/220001/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html File third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html (right): https://codereview.chromium.org/2041193005/diff/220001/third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html#newcode27 third_party/WebKit/LayoutTests/virtual/threaded/fast/compositorworker/basic-plumbing-main-to-worker.html:27: var test = async_test('Tests that a change from the ...
4 years, 6 months ago (2016-06-13 21:52:51 UTC) #11
Ian Vollick
https://codereview.chromium.org/2041193005/diff/220001/third_party/WebKit/Source/core/dom/CompositorProxy.cpp File third_party/WebKit/Source/core/dom/CompositorProxy.cpp (right): https://codereview.chromium.org/2041193005/diff/220001/third_party/WebKit/Source/core/dom/CompositorProxy.cpp#newcode181 third_party/WebKit/Source/core/dom/CompositorProxy.cpp:181: if (!m_state.get()) On 2016/06/13 at 21:52:51, majidvp wrote: > ...
4 years, 6 months ago (2016-06-14 11:24:29 UTC) #13
majidvp
https://codereview.chromium.org/2041193005/diff/220001/third_party/WebKit/Source/core/dom/CompositorProxy.cpp File third_party/WebKit/Source/core/dom/CompositorProxy.cpp (right): https://codereview.chromium.org/2041193005/diff/220001/third_party/WebKit/Source/core/dom/CompositorProxy.cpp#newcode181 third_party/WebKit/Source/core/dom/CompositorProxy.cpp:181: if (!m_state.get()) On 2016/06/14 11:24:28, vollick wrote: > On ...
4 years, 6 months ago (2016-06-14 17:25:58 UTC) #15
jbroman
lgtm once you fix the trybot failures (which look real) https://codereview.chromium.org/2041193005/diff/260001/third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp File third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp (right): https://codereview.chromium.org/2041193005/diff/260001/third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp#newcode76 ...
4 years, 6 months ago (2016-06-15 08:55:50 UTC) #16
majidvp
Fixed failing test and refactor the layout tests to use a similar promise based pattern ...
4 years, 6 months ago (2016-06-15 15:46:53 UTC) #18
majidvp
aelias@: Please review changes in cc/ and Source/web
4 years, 6 months ago (2016-06-15 15:47:38 UTC) #20
aelias_OOO_until_Jul13
https://codereview.chromium.org/2041193005/diff/280001/third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp File third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp (right): https://codereview.chromium.org/2041193005/diff/280001/third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp#newcode57 third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp:57: updateMutableStateForCompositorProxies(stateProvider); Please make a small RAII class called like ...
4 years, 6 months ago (2016-06-15 18:40:54 UTC) #21
majidvp
https://codereview.chromium.org/2041193005/diff/280001/third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp File third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp (right): https://codereview.chromium.org/2041193005/diff/280001/third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp#newcode57 third_party/WebKit/Source/web/CompositorProxyClientImpl.cpp:57: updateMutableStateForCompositorProxies(stateProvider); On 2016/06/15 18:40:54, aelias wrote: > Please make ...
4 years, 6 months ago (2016-06-16 02:53:54 UTC) #22
aelias_OOO_until_Jul13
lgtm
4 years, 6 months ago (2016-06-16 03:09:08 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2041193005/300001
4 years, 6 months ago (2016-06-16 03:16:31 UTC) #26
commit-bot: I haz the power
Committed patchset #16 (id:300001)
4 years, 6 months ago (2016-06-16 04:10:20 UTC) #28
commit-bot: I haz the power
CQ bit was unchecked
4 years, 6 months ago (2016-06-16 04:10:25 UTC) #29
commit-bot: I haz the power
4 years, 6 months ago (2016-06-16 04:11:51 UTC) #31
Message was sent while issue was closed.
Patchset 16 (id:??) landed as
https://crrev.com/ee4cbfea75629407af2b0377ca2567540081430d
Cr-Commit-Position: refs/heads/master@{#400082}

Powered by Google App Engine
This is Rietveld 408576698