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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 months, 1 week ago by weiliangc
Modified:
5 months 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
Commit queue not available (can’t edit this change).

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 50 (35 generated)
weiliangc
5 months 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())) { ...
5 months ago (2017-03-16 17:15:32 UTC) #17
weiliangc
+pdf for Blink addressed review comments.
5 months 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 ...
5 months 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 ...
5 months 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): ...
5 months 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 ...
5 months 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 ...
5 months 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 ...
5 months ago (2017-03-16 21:41:47 UTC) #26
weiliangc
PTAL
5 months 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 ...
5 months 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
5 months 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)
5 months 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
5 months ago (2017-03-17 19:17:47 UTC) #47
commit-bot: I haz the power
5 months 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...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b