Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(157)

Issue 2753933005: cc: Move Layer Id to Node Map to Individual Property Tree Private (Closed)

Created:
9 months ago by weiliangc
Modified:
8 months, 4 weeks ago
Reviewers:
pdr., ajuma
CC:
ajuma+watch_chromium.org, blink-reviews, blink-reviews-platform-graphics_chromium.org, Rik, cc-bugs_chromium.org, chromium-reviews, danakj+watch_chromium.org, danakj, dshwang, drott+blinkwatch_chromium.org, krit, fmalita+watch_chromium.org, jaydasika, jbroman, Justin Novosad, kinuko+watch, pdr+graphicswatchlist_chromium.org, rwlbuis, Stephen Chennney
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

cc: Move Layer Id to Node Map to Individual Property Tree Private Move the layer id to node index map to individual property tree, and make this map private. This helps control of access to these maps. BUG=695681 R=ajuma CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Review-Url: https://codereview.chromium.org/2753933005 Cr-Commit-Position: refs/heads/master@{#457850} Committed: https://chromium.googlesource.com/chromium/src/+/42c074f058c39ca559d47e168dade668b5ec419e

Patch Set 1 #

Patch Set 2 : fix compile error and cc unittest error #

Patch Set 3 : add virtual to desctructor of ptree #

Total comments: 4

Patch Set 4 : address review comments #

Total comments: 2

Patch Set 5 : renaming to owning layer id #

Total comments: 1

Patch Set 6 : commtent change #

Patch Set 7 : renaming clean up #

Unified diffs Side-by-side diffs Delta from patch set Stats (+194 lines, -288 lines) Patch
M cc/input/scrollbar_animation_controller.cc View 1 2 3 4 5 1 chunk +9 lines, -7 lines 0 comments Download
M cc/layers/layer.cc View 1 2 3 4 7 chunks +25 lines, -53 lines 0 comments Download
M cc/layers/layer_impl.h View 1 chunk +1 line, -0 lines 0 comments Download
M cc/layers/layer_impl.cc View 1 2 3 4 6 chunks +16 lines, -34 lines 0 comments Download
M cc/layers/render_surface_impl.cc View 1 2 3 4 1 chunk +4 lines, -3 lines 0 comments Download
M cc/trees/layer_tree_host.cc View 1 2 3 4 2 chunks +7 lines, -12 lines 0 comments Download
M cc/trees/layer_tree_host_common_unittest.cc View 1 2 3 4 9 chunks +18 lines, -6 lines 0 comments Download
M cc/trees/layer_tree_host_unittest.cc View 1 2 3 4 2 chunks +42 lines, -72 lines 0 comments Download
M cc/trees/layer_tree_impl.cc View 1 2 3 4 3 chunks +6 lines, -10 lines 0 comments Download
M cc/trees/property_tree.h View 1 2 3 4 7 chunks +28 lines, -13 lines 0 comments Download
M cc/trees/property_tree.cc View 1 2 3 4 6 chunks +18 lines, -50 lines 0 comments Download
M cc/trees/property_tree_builder.cc View 1 2 3 4 4 chunks +8 lines, -9 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositorTest.cpp View 1 2 3 4 1 chunk +3 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/platform/graphics/compositing/PropertyTreeManager.cpp View 1 2 3 4 5 6 8 chunks +9 lines, -16 lines 0 comments Download

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 50 (35 generated)
weiliangc
8 months, 4 weeks ago (2017-03-16 14:50:09 UTC) #14
ajuma
lgtm, thanks for simplifying this logic! https://codereview.chromium.org/2753933005/diff/40001/cc/trees/layer_tree_host.cc File cc/trees/layer_tree_host.cc (right): https://codereview.chromium.org/2753933005/diff/40001/cc/trees/layer_tree_host.cc#newcode1243 cc/trees/layer_tree_host.cc:1243: if (property_trees_.effect_tree.FindNodeFromId(layer->id())) { ...
8 months, 4 weeks ago (2017-03-16 17:15:32 UTC) #17
weiliangc
+pdf for Blink addressed review comments.
8 months, 4 weeks ago (2017-03-16 20:20:54 UTC) #20
weiliangc
On 2017/03/16 20:20:54, weiliangc wrote: > +pdf for Blink > > addressed review comments. Oooooops ...
8 months, 4 weeks ago (2017-03-16 20:27:10 UTC) #21
pdr.
bikesheeeeddd https://codereview.chromium.org/2753933005/diff/80001/cc/trees/property_tree.h File cc/trees/property_tree.h (right): https://codereview.chromium.org/2753933005/diff/80001/cc/trees/property_tree.h#newcode103 cc/trees/property_tree.h:103: int FindNodeIndexFromOwningId(int id) const { I think we ...
8 months, 4 weeks ago (2017-03-16 20:30:28 UTC) #22
pdr.
On 2017/03/16 at 20:30:28, pdr. wrote: > bikesheeeeddd > > https://codereview.chromium.org/2753933005/diff/80001/cc/trees/property_tree.h > File cc/trees/property_tree.h (right): ...
8 months, 4 weeks ago (2017-03-16 20:31:39 UTC) #23
ajuma
On 2017/03/16 20:31:39, pdr. wrote: > On 2017/03/16 at 20:30:28, pdr. wrote: > > bikesheeeeddd ...
8 months, 4 weeks ago (2017-03-16 20:46:27 UTC) #24
pdr.
On 2017/03/16 at 20:46:27, ajuma wrote: > On 2017/03/16 20:31:39, pdr. wrote: > > On ...
8 months, 4 weeks ago (2017-03-16 21:36:04 UTC) #25
ajuma
On 2017/03/16 21:36:04, pdr. wrote: > On 2017/03/16 at 20:46:27, ajuma wrote: > > On ...
8 months, 4 weeks ago (2017-03-16 21:41:47 UTC) #26
weiliangc
PTAL
8 months, 4 weeks ago (2017-03-16 22:13:09 UTC) #28
pdr.
LGTM https://codereview.chromium.org/2753933005/diff/100001/cc/input/scrollbar_animation_controller.cc File cc/input/scrollbar_animation_controller.cc (right): https://codereview.chromium.org/2753933005/diff/100001/cc/input/scrollbar_animation_controller.cc#newcode349 cc/input/scrollbar_animation_controller.cc:349: // yet have valid layer_id_to_node_index entries in effect ...
8 months, 4 weeks ago (2017-03-16 22:17:41 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2753933005/120001
8 months, 4 weeks ago (2017-03-16 22:29:46 UTC) #34
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/375844)
8 months, 4 weeks ago (2017-03-16 22:38:58 UTC) #36
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2753933005/140001
8 months, 4 weeks ago (2017-03-17 19:17:47 UTC) #47
commit-bot: I haz the power
8 months, 4 weeks ago (2017-03-17 19:29:17 UTC) #50
Message was sent while issue was closed.
Committed patchset #7 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/42c074f058c39ca559d47e168dad...

Powered by Google App Engine
This is Rietveld 0eb02b776