cc: Remove ScopedPtrVector and cc::remove_if.

cc: Remove ScopedPtrVector and cc::remove_if.

This patch removes ScopedPtrVector and cc::remove_if. It depends on
https://codereview.chromium.org/1441613002 for TakeBack.

R=danakj
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/a370ef52788ea0f6c410b05ec40a74ee9bf3a647
Cr-Commit-Position: refs/heads/master@{#360318}

[vmpstr, 2015-11-13 18:44:50]

Patchset #2 (id:20001) has been deleted

[vmpstr, 2015-11-13 18:45:01]

Patchset #2 (id:40001) has been deleted

[vmpstr, 2015-11-13 18:45:51]

Patchset #2 (id:60001) has been deleted

[vmpstr, 2015-11-13 19:11:43]

Patchset #2 (id:80001) has been deleted

[vmpstr, 2015-11-13 19:11:54]

Patchset #2 (id:90001) has been deleted

[ericrk, 2015-11-16 19:20:51]

ericrk@chromium.org changed reviewers:
+ ericrk@chromium.org

[ericrk, 2015-11-16 19:20:52]

one comment so far :D - still revieiwing

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
File cc/animation/layer_animation_controller.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:738: }));
missing animations.end()?

[danakj, 2015-11-16 19:42:13]

Can you make sure this builds on top of
https://codereview.chromium.org/1440593004/?

[ericrk, 2015-11-16 21:59:49]

lgtm

Looks good other than a couple minor comments!

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer_impl.c...
cc/layers/layer_impl.cc:280: for (auto it = requests->begin(); it !=
requests->end(); ++it)
not sure if we can use it, but a more C++11-y way to do this is:
copy_requests_.insert(copy_requests_.end(),
std::make_move_iterator(requests->begin()),
std::make_move_iterator(requests->end()));

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer_impl.c...
cc/layers/layer_impl.cc:296: for (auto it = copy_requests_.begin(); it !=
copy_requests_.end(); ++it)
same here.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1147: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
did you mean to use a different list?

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1156: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
same here?

[danakj, 2015-11-17 01:12:19]

The good thing about a CL that touches all the code, is that then the reviewer
gets to read all the code and find all the weird. ALL THE AUTOS!

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/animation...
File cc/animation/animation_player.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/animation...
cc/animation/animation_player.cc:108: for (auto it = animations_.begin(); it !=
animations_.end(); ++it)
This could be

for (auto& animation : animations_)
  eleme...->AddAnimation(animation.Pass());

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/animation...
File cc/animation/animation_player.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/animation...
cc/animation/animation_player.h:98: typedef std::vector<scoped_ptr<Animation>>
AnimationList;
drop the typedef, just use the concrete type?

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/keyframed...
File cc/animation/keyframed_animation_curve.h (left):

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/keyframed...
cc/animation/keyframed_animation_curve.h:13: #include
"cc/base/scoped_ptr_vector.h"
include vector tho, iwyu

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
File cc/animation/layer_animation_controller.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:64: for (const scoped_ptr<Animation>&
animation : animations_) {
I'd like this to be an auto& and I wrote as much to chromium-dev. We can see
what people say I guess.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:90: auto animations_to_remove =
std::stable_partition(
Can you leave a comment saying you used this instead of remove_if cuz we want to
use them before removing

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:114: auto animations_to_remove =
std::stable_partition(
ditto comment

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:116: [animation_id,
target_property](const scoped_ptr<Animation>& animation) {
in general i find it easier to read these types of things if you store the
lambda in a var (has_animation_id_and_property?). WDYT? this one isn't atrocious
but a general feeling.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:287: return
!animation->affects_active_observers() &&
kinda like having a lambda here too, similar to combining multiple bool
conditions into a single bool helper than explains them all

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:725:
std::vector<scoped_ptr<Animation>>& animations = controller_impl->animations_;
that's a lot of templatefu. consider auto&? do we want to fix the type here?

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
File cc/animation/layer_animation_controller.h (left):

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.h:17: #include
"cc/base/scoped_ptr_vector.h"
include vector

https://codereview.chromium.org/1437413002/diff/130001/cc/base/list_container...
File cc/base/list_container_helper.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/base/list_container...
cc/base/list_container_helper.h:8: #include "base/logging.h"
why?

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
File cc/debug/micro_benchmark_controller.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
cc/debug/micro_benchmark_controller.cc:78: for
(std::vector<scoped_ptr<MicroBenchmark>>::iterator it =
auto. also range-based?

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
cc/debug/micro_benchmark_controller.cc:117: return benchmark->IsDone();
do you mean !IsDone?

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
File cc/debug/micro_benchmark_controller_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
cc/debug/micro_benchmark_controller_impl.cc:29: for
(std::vector<scoped_ptr<MicroBenchmarkImpl>>::iterator it =
auto, ranged-based

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
cc/debug/micro_benchmark_controller_impl.cc:43: return benchmark->IsDone();
did you mean the inverse?

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/heads_up_dis...
File cc/layers/heads_up_display_layer_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/heads_up_dis...
cc/layers/heads_up_display_layer_impl.cc:94: for
(std::vector<scoped_ptr<ScopedResource>>::iterator it =
auto and ranged-based?

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/heads_up_dis...
cc/layers/heads_up_display_layer_impl.cc:115: [this](const
scoped_ptr<ScopedResource>& resource) {
wwooooaa you can do that? and then use member vars inside? cool.

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer.cc
File cc/layers/layer.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer.cc#new...
cc/layers/layer.cc:1306: scoped_ptr<CopyOutputRequest> original_request =
it->Pass();
while you're here, can you change any new Pass() calls you made into move()s?

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer_impl.c...
cc/layers/layer_impl.cc:142: return ret.Pass();
oops, shame on me

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer_impl.c...
cc/layers/layer_impl.cc:280: for (auto it = requests->begin(); it !=
requests->end(); ++it)
On 2015/11/16 21:59:49, ericrk wrote:
> not sure if we can use it, but a more C++11-y way to do this is:
> copy_requests_.insert(copy_requests_.end(),
> std::make_move_iterator(requests->begin()),
> std::make_move_iterator(requests->end()));

Cool! Someone should propose it to cxx@ now that move() is approved.

In the mean time this could be range-based and auto

https://codereview.chromium.org/1437413002/diff/130001/cc/output/direct_rende...
File cc/output/direct_renderer.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/output/direct_rende...
cc/output/direct_renderer.cc:251: for (const scoped_ptr<RenderPass>& pass :
*render_passes_in_draw_order) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/output/direct_rende...
cc/output/direct_renderer.cc:267: for (size_t i = 0; i <
render_passes_in_draw_order->size(); ++i) {
what have i done. iterating on size() and using at(). can you make this better?

https://codereview.chromium.org/1437413002/diff/130001/cc/output/direct_rende...
cc/output/direct_renderer.cc:271: for
(std::vector<scoped_ptr<CopyOutputRequest>>::iterator it =
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/output/gl_renderer.h
File cc/output/gl_renderer.h (left):

https://codereview.chromium.org/1437413002/diff/130001/cc/output/gl_renderer....
cc/output/gl_renderer.h:12: #include "cc/base/scoped_ptr_vector.h"
include vector

https://codereview.chromium.org/1437413002/diff/130001/cc/output/gl_renderer.h
File cc/output/gl_renderer.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/output/gl_renderer....
cc/output/gl_renderer.h:261: typedef
std::vector<scoped_ptr<ResourceProvider::ScopedReadLockGL>>
bleh, typedefs. at least make this using = if you're changing it.

https://codereview.chromium.org/1437413002/diff/130001/cc/output/overlay_proc...
File cc/output/overlay_processor.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/output/overlay_proc...
cc/output/overlay_processor.cc:32: for (const scoped_ptr<Strategy>& strategy :
strategies_) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/output/overlay_proc...
File cc/output/overlay_processor.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/output/overlay_proc...
cc/output/overlay_processor.h:34: typedef std::vector<scoped_ptr<Strategy>>
StrategyList;
<___<...  "using" at least

also only used once >______>

https://codereview.chromium.org/1437413002/diff/130001/cc/quads/render_pass.cc
File cc/quads/render_pass.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/quads/render_pass.c...
cc/quads/render_pass.cc:97: for (size_t i = 0; i < in.size(); ++i) {
ouch. should be range-based too. whyyyy does it have to be this way..

https://codereview.chromium.org/1437413002/diff/130001/cc/quads/render_pass.h
File cc/quads/render_pass.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/quads/render_pass.h...
cc/quads/render_pass.h:150: typedef std::vector<scoped_ptr<RenderPass>>
RenderPassList;
fun fact, this also is defined in renderer.h. wow.

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/display.cc
File cc/surfaces/display.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/display.cc...
cc/surfaces/display.cc:215: for (const scoped_ptr<RenderPass>& pass :
frame_data->render_pass_list) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/surface.cc
File cc/surfaces/surface.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/surface.cc...
cc/surfaces/surface.cc:122: std::make_pair(render_pass->id, request.release()));
can you put a TODO and a bug to make this multimap hold scoped_ptrs?

or send me a CL

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/surface_ag...
File cc/surfaces/surface_aggregator_unittest.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/surface_ag...
cc/surfaces/surface_aggregator_unittest.cc:142: for (const
scoped_ptr<RenderPass>& pass : frame_data->render_pass_list) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/surface_hi...
File cc/surfaces/surface_hittest.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/surface_hi...
cc/surfaces/surface_hittest.cc:236: for (const scoped_ptr<RenderPass>&
render_pass :
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/test/surface_aggreg...
File cc/test/surface_aggregator_test_helpers.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/test/surface_aggreg...
cc/test/surface_aggregator_test_helpers.h:22: typedef
std::vector<scoped_ptr<RenderPass>> RenderPassList;
oh cool, another copy of this typedef.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/eviction_tile...
File cc/tiles/eviction_tile_priority_queue.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/eviction_tile...
cc/tiles/eviction_tile_priority_queue.cc:106: const
std::vector<scoped_ptr<TilingSetEvictionQueue>>& next_queues =
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/eviction_tile...
cc/tiles/eviction_tile_priority_queue.cc:114:
std::vector<scoped_ptr<TilingSetEvictionQueue>>& next_queues =
auto?

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/eviction_tile...
cc/tiles/eviction_tile_priority_queue.cc:132: return
const_cast<std::vector<scoped_ptr<TilingSetEvictionQueue>>&>(
The number of casts in this function is approaching infinity.

Would some temp var&s help this be human-parseable?

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
File cc/tiles/picture_layer_tiling_set.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:75: for (const
scoped_ptr<PictureLayerTiling>& pending_twin_tiling :
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:114: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:146: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:166: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:178: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:209: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:245: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_)
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:280: return nullptr;
thx

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:339: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:364: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:376: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_)
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/raster_tile_p...
File cc/tiles/raster_tile_priority_queue_all.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/raster_tile_p...
cc/tiles/raster_tile_priority_queue_all.cc:97: const
std::vector<scoped_ptr<TilingSetRasterQueueAll>>& next_queues =
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/raster_tile_p...
cc/tiles/raster_tile_priority_queue_all.cc:105:
std::vector<scoped_ptr<TilingSetRasterQueueAll>>& next_queues =
auto?

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/raster_tile_p...
cc/tiles/raster_tile_priority_queue_all.cc:123: return
const_cast<std::vector<scoped_ptr<TilingSetRasterQueueAll>>&>(
heh. temp vars?

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host.cc:819: for (; it != info->swap_promises.end(); ++it) {
hm.. why isn't this range-based?

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host.cc:1034: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host.cc:1040: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_common.cc:2577: for (const scoped_ptr<LayerImpl>&
child_layer : layer->children()) {
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_common_unittest.cc:8: #include <set>
include vector

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_impl_unittest.cc:3117: for (const
scoped_ptr<LayerImpl>& child : root->children())
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_impl_unittest.cc:3210: for (const
scoped_ptr<LayerImpl>& child : root->children())
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:868: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1081: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1086: for (const scoped_ptr<SwapPromise>&
swap_promise : pinned_swap_promise_list_)
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1137: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1144: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1147: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/16 21:59:49, ericrk wrote:
> did you mean to use a different list?

also, auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1153: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1156: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/16 21:59:49, ericrk wrote:
> same here?

also, auto

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/tree_synchron...
File cc/trees/tree_synchronizer.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/tree_synchron...
cc/trees/tree_synchronizer.cc:28: for (OwnedLayerImplList::iterator it =
children.begin(); it != children.end();
range-based and auto?

https://codereview.chromium.org/1437413002/diff/130001/content/common/cc_mess...
File content/common/cc_messages.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/content/common/cc_mess...
content/common/cc_messages.cc:692: for (const scoped_ptr<cc::RenderPass>& pass :
p.render_pass_list) {
auto

https://codereview.chromium.org/1437413002/diff/130001/ui/compositor/layer.cc
File ui/compositor/layer.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/ui/compositor/layer.cc...
ui/compositor/layer.cc:1008: for (auto it =
pending_threaded_animations_.begin();
range-based and auto

[vmpstr, 2015-11-17 23:26:26]

PTAL

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/animation...
File cc/animation/animation_player.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/animation...
cc/animation/animation_player.cc:108: for (auto it = animations_.begin(); it !=
animations_.end(); ++it)
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> This could be
> 
> for (auto& animation : animations_)
>   eleme...->AddAnimation(animation.Pass());

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/animation...
File cc/animation/animation_player.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/animation...
cc/animation/animation_player.h:98: typedef std::vector<scoped_ptr<Animation>>
AnimationList;
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> drop the typedef, just use the concrete type?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/keyframed...
File cc/animation/keyframed_animation_curve.h (left):

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/keyframed...
cc/animation/keyframed_animation_curve.h:13: #include
"cc/base/scoped_ptr_vector.h"
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> include vector tho, iwyu

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
File cc/animation/layer_animation_controller.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:64: for (const scoped_ptr<Animation>&
animation : animations_) {
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> I'd like this to be an auto& and I wrote as much to chromium-dev. We can see
> what people say I guess.

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:90: auto animations_to_remove =
std::stable_partition(
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> Can you leave a comment saying you used this instead of remove_if cuz we want
to
> use them before removing

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:114: auto animations_to_remove =
std::stable_partition(
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> ditto comment

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:116: [animation_id,
target_property](const scoped_ptr<Animation>& animation) {
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> in general i find it easier to read these types of things if you store the
> lambda in a var (has_animation_id_and_property?). WDYT? this one isn't
atrocious
> but a general feeling.

Yeah I think that's very reasonable. Let me know of any other place you want
this changed.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:126:
!(*animations_to_remove)->is_finished())
This _looks_ like there can only be one of such animations. If that's the case,
this should be a find_if/erase. However, I don't know this code that much...
+ajuma for a second opinion.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:287: return
!animation->affects_active_observers() &&
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> kinda like having a lambda here too, similar to combining multiple bool
> conditions into a single bool helper than explains them all

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:725:
std::vector<scoped_ptr<Animation>>& animations = controller_impl->animations_;
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> that's a lot of templatefu. consider auto&? do we want to fix the type here?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/animation/layer_ani...
cc/animation/layer_animation_controller.cc:738: }));
On 2015/11/16 19:20:52, ericrk wrote:
> missing animations.end()?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/base/list_container...
File cc/base/list_container_helper.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/base/list_container...
cc/base/list_container_helper.h:8: #include "base/logging.h"
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> why?

That was meant to be in the .cc, moved it there. It's for all the DCHECKs that
are not not defined.

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
File cc/debug/micro_benchmark_controller.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
cc/debug/micro_benchmark_controller.cc:78: for
(std::vector<scoped_ptr<MicroBenchmark>>::iterator it =
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> auto. also range-based?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
cc/debug/micro_benchmark_controller.cc:117: return benchmark->IsDone();
On 2015/11/17 01:12:16, danakj (behind on reviews) wrote:
> do you mean !IsDone?

No, this should remove things that are done. The confusion comes from the fact
that partition and remove_if put opposite things at the front (partition puts
things with true first, remove_if puts things with false first).

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
File cc/debug/micro_benchmark_controller_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
cc/debug/micro_benchmark_controller_impl.cc:29: for
(std::vector<scoped_ptr<MicroBenchmarkImpl>>::iterator it =
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> auto, ranged-based

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/debug/micro_benchma...
cc/debug/micro_benchmark_controller_impl.cc:43: return benchmark->IsDone();
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> did you mean the inverse?

Nope :)

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/heads_up_dis...
File cc/layers/heads_up_display_layer_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/heads_up_dis...
cc/layers/heads_up_display_layer_impl.cc:94: for
(std::vector<scoped_ptr<ScopedResource>>::iterator it =
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> auto and ranged-based?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/heads_up_dis...
cc/layers/heads_up_display_layer_impl.cc:115: [this](const
scoped_ptr<ScopedResource>& resource) {
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> wwooooaa you can do that? and then use member vars inside? cool. 

hehe, yep! :)

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer.cc
File cc/layers/layer.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer.cc#new...
cc/layers/layer.cc:1306: scoped_ptr<CopyOutputRequest> original_request =
it->Pass();
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> while you're here, can you change any new Pass() calls you made into move()s?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer_impl.cc
File cc/layers/layer_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer_impl.c...
cc/layers/layer_impl.cc:142: return ret.Pass();
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> oops, shame on me

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer_impl.c...
cc/layers/layer_impl.cc:280: for (auto it = requests->begin(); it !=
requests->end(); ++it)
On 2015/11/16 21:59:49, ericrk wrote:
> not sure if we can use it, but a more C++11-y way to do this is:
> copy_requests_.insert(copy_requests_.end(),
> std::make_move_iterator(requests->begin()),
> std::make_move_iterator(requests->end()));

I could do it as 

std::move(requests->begin(), requests->end(),
std::back_inserter(copy_requests_));

but either of those approaches requires extra approval, so I would say it's ok
for now :) WDYT?

https://codereview.chromium.org/1437413002/diff/130001/cc/layers/layer_impl.c...
cc/layers/layer_impl.cc:280: for (auto it = requests->begin(); it !=
requests->end(); ++it)
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> On 2015/11/16 21:59:49, ericrk wrote:
> > not sure if we can use it, but a more C++11-y way to do this is:
> > copy_requests_.insert(copy_requests_.end(),
> > std::make_move_iterator(requests->begin()),
> > std::make_move_iterator(requests->end()));
> 
> Cool! Someone should propose it to cxx@ now that move() is approved.
> 
> In the mean time this could be range-based and auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/output/direct_rende...
File cc/output/direct_renderer.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/output/direct_rende...
cc/output/direct_renderer.cc:251: for (const scoped_ptr<RenderPass>& pass :
*render_passes_in_draw_order) {
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/output/direct_rende...
cc/output/direct_renderer.cc:267: for (size_t i = 0; i <
render_passes_in_draw_order->size(); ++i) {
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> what have i done. iterating on size() and using at(). can you make this
better?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/output/direct_rende...
cc/output/direct_renderer.cc:271: for
(std::vector<scoped_ptr<CopyOutputRequest>>::iterator it =
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/output/gl_renderer.h
File cc/output/gl_renderer.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/output/gl_renderer....
cc/output/gl_renderer.h:261: typedef
std::vector<scoped_ptr<ResourceProvider::ScopedReadLockGL>>
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> bleh, typedefs. at least make this using = if you're changing it.

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/output/overlay_proc...
File cc/output/overlay_processor.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/output/overlay_proc...
cc/output/overlay_processor.cc:32: for (const scoped_ptr<Strategy>& strategy :
strategies_) {
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/output/overlay_proc...
File cc/output/overlay_processor.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/output/overlay_proc...
cc/output/overlay_processor.h:34: typedef std::vector<scoped_ptr<Strategy>>
StrategyList;
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> <___<...  "using" at least
> 
> also only used once >______> 

There are other uses (it's a public typedef). Switched to "using"

https://codereview.chromium.org/1437413002/diff/130001/cc/quads/render_pass.cc
File cc/quads/render_pass.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/quads/render_pass.c...
cc/quads/render_pass.cc:97: for (size_t i = 0; i < in.size(); ++i) {
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> ouch. should be range-based too. whyyyy does it have to be this way..

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/quads/render_pass.h
File cc/quads/render_pass.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/quads/render_pass.h...
cc/quads/render_pass.h:150: typedef std::vector<scoped_ptr<RenderPass>>
RenderPassList;
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> fun fact, this also is defined in renderer.h. wow.

Twice for good measure?

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/display.cc
File cc/surfaces/display.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/display.cc...
cc/surfaces/display.cc:215: for (const scoped_ptr<RenderPass>& pass :
frame_data->render_pass_list) {
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/surface.cc
File cc/surfaces/surface.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/surface.cc...
cc/surfaces/surface.cc:122: std::make_pair(render_pass->id, request.release()));
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> can you put a TODO and a bug to make this multimap hold scoped_ptrs?
> 
> or send me a CL

I'll do this as a follow-up. Added a TODO.

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/surface_hi...
File cc/surfaces/surface_hittest.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/surfaces/surface_hi...
cc/surfaces/surface_hittest.cc:236: for (const scoped_ptr<RenderPass>&
render_pass :
On 2015/11/17 01:12:17, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/test/surface_aggreg...
File cc/test/surface_aggregator_test_helpers.h (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/test/surface_aggreg...
cc/test/surface_aggregator_test_helpers.h:22: typedef
std::vector<scoped_ptr<RenderPass>> RenderPassList;
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> oh cool, another copy of this typedef.

These are test helpers, so I think that's ok. We also define Quad below :)

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/eviction_tile...
File cc/tiles/eviction_tile_priority_queue.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/eviction_tile...
cc/tiles/eviction_tile_priority_queue.cc:106: const
std::vector<scoped_ptr<TilingSetEvictionQueue>>& next_queues =
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/eviction_tile...
cc/tiles/eviction_tile_priority_queue.cc:114:
std::vector<scoped_ptr<TilingSetEvictionQueue>>& next_queues =
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/eviction_tile...
cc/tiles/eviction_tile_priority_queue.cc:132: return
const_cast<std::vector<scoped_ptr<TilingSetEvictionQueue>>&>(
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> The number of casts in this function is approaching infinity.
> 
> Would some temp var&s help this be human-parseable?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
File cc/tiles/picture_layer_tiling_set.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:75: for (const
scoped_ptr<PictureLayerTiling>& pending_twin_tiling :
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:114: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:146: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:166: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:178: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:209: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:245: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_)
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:280: return nullptr;
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> thx

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:339: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:364: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_) {
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:376: for (const
scoped_ptr<PictureLayerTiling>& tiling : tilings_)
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/raster_tile_p...
File cc/tiles/raster_tile_priority_queue_all.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/raster_tile_p...
cc/tiles/raster_tile_priority_queue_all.cc:97: const
std::vector<scoped_ptr<TilingSetRasterQueueAll>>& next_queues =
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/raster_tile_p...
cc/tiles/raster_tile_priority_queue_all.cc:105:
std::vector<scoped_ptr<TilingSetRasterQueueAll>>& next_queues =
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/tiles/raster_tile_p...
cc/tiles/raster_tile_priority_queue_all.cc:123: return
const_cast<std::vector<scoped_ptr<TilingSetRasterQueueAll>>&>(
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> heh. temp vars?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host.cc:819: for (; it != info->swap_promises.end(); ++it) {
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> hm.. why isn't this range-based?

Hmm. I'm not sure. I changed it...

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host.cc:1034: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host.cc:1040: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/17 01:12:18, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_common.cc:2577: for (const scoped_ptr<LayerImpl>&
child_layer : layer->children()) {
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_common_unittest.cc:8: #include <set>
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> include vector

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_impl_unittest.cc:3117: for (const
scoped_ptr<LayerImpl>& child : root->children())
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_ho...
cc/trees/layer_tree_host_impl_unittest.cc:3210: for (const
scoped_ptr<LayerImpl>& child : root->children())
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:868: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1081: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1086: for (const scoped_ptr<SwapPromise>&
swap_promise : pinned_swap_promise_list_)
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1137: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1144: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1147: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/16 21:59:49, ericrk wrote:
> did you mean to use a different list?

Whoops. Thanks for catching this!

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1147: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> On 2015/11/16 21:59:49, ericrk wrote:
> > did you mean to use a different list?
> 
> also, auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1153: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1156: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/16 21:59:49, ericrk wrote:
> same here?

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/layer_tree_im...
cc/trees/layer_tree_impl.cc:1156: for (const scoped_ptr<SwapPromise>&
swap_promise : swap_promise_list_)
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> On 2015/11/16 21:59:49, ericrk wrote:
> > same here?
> 
> also, auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/tree_synchron...
File cc/trees/tree_synchronizer.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/cc/trees/tree_synchron...
cc/trees/tree_synchronizer.cc:28: for (OwnedLayerImplList::iterator it =
children.begin(); it != children.end();
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> range-based and auto?

Done.

https://codereview.chromium.org/1437413002/diff/130001/content/common/cc_mess...
File content/common/cc_messages.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/content/common/cc_mess...
content/common/cc_messages.cc:692: for (const scoped_ptr<cc::RenderPass>& pass :
p.render_pass_list) {
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/130001/ui/compositor/layer.cc
File ui/compositor/layer.cc (right):

https://codereview.chromium.org/1437413002/diff/130001/ui/compositor/layer.cc...
ui/compositor/layer.cc:1008: for (auto it =
pending_threaded_animations_.begin();
On 2015/11/17 01:12:19, danakj (behind on reviews) wrote:
> range-based and auto

Done.

[danakj, 2015-11-17 23:49:41]

LGTM

https://codereview.chromium.org/1437413002/diff/170001/cc/debug/micro_benchma...
File cc/debug/micro_benchmark_controller.cc (right):

https://codereview.chromium.org/1437413002/diff/170001/cc/debug/micro_benchma...
cc/debug/micro_benchmark_controller.cc:82: return it != benchmarks_.end() ?
(*it)->ProcessMessage(value.Pass()) : false;
can you do this with an early out instead on if it == end() return false?

?: with lots of stuff in it is hard to read

also, std::move()

https://codereview.chromium.org/1437413002/diff/170001/cc/layers/heads_up_dis...
File cc/layers/heads_up_display_layer_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/170001/cc/layers/heads_up_dis...
cc/layers/heads_up_display_layer_impl.cc:111:
std::vector<scoped_ptr<ScopedResource>>::iterator it_erase =
auto?

https://codereview.chromium.org/1437413002/diff/170001/cc/layers/layer.cc
File cc/layers/layer.cc (right):

https://codereview.chromium.org/1437413002/diff/170001/cc/layers/layer.cc#new...
cc/layers/layer.cc:1301: for
(std::vector<scoped_ptr<CopyOutputRequest>>::iterator it =
auto

https://codereview.chromium.org/1437413002/diff/170001/cc/tiles/picture_layer...
File cc/tiles/picture_layer_tiling_set.cc (right):

https://codereview.chromium.org/1437413002/diff/170001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:317:
std::vector<scoped_ptr<PictureLayerTiling>>::iterator iter =
auto

[vmpstr, 2015-11-18 00:08:46]

https://codereview.chromium.org/1437413002/diff/170001/cc/debug/micro_benchma...
File cc/debug/micro_benchmark_controller.cc (right):

https://codereview.chromium.org/1437413002/diff/170001/cc/debug/micro_benchma...
cc/debug/micro_benchmark_controller.cc:82: return it != benchmarks_.end() ?
(*it)->ProcessMessage(value.Pass()) : false;
On 2015/11/17 23:49:41, danakj (behind on reviews) wrote:
> can you do this with an early out instead on if it == end() return false?
> 
> ?: with lots of stuff in it is hard to read
> 
> also, std::move()

Done.

https://codereview.chromium.org/1437413002/diff/170001/cc/layers/heads_up_dis...
File cc/layers/heads_up_display_layer_impl.cc (right):

https://codereview.chromium.org/1437413002/diff/170001/cc/layers/heads_up_dis...
cc/layers/heads_up_display_layer_impl.cc:111:
std::vector<scoped_ptr<ScopedResource>>::iterator it_erase =
On 2015/11/17 23:49:41, danakj (behind on reviews) wrote:
> auto?

Done.

https://codereview.chromium.org/1437413002/diff/170001/cc/layers/layer.cc
File cc/layers/layer.cc (right):

https://codereview.chromium.org/1437413002/diff/170001/cc/layers/layer.cc#new...
cc/layers/layer.cc:1301: for
(std::vector<scoped_ptr<CopyOutputRequest>>::iterator it =
On 2015/11/17 23:49:41, danakj (behind on reviews) wrote:
> auto

Done.

https://codereview.chromium.org/1437413002/diff/170001/cc/tiles/picture_layer...
File cc/tiles/picture_layer_tiling_set.cc (right):

https://codereview.chromium.org/1437413002/diff/170001/cc/tiles/picture_layer...
cc/tiles/picture_layer_tiling_set.cc:317:
std::vector<scoped_ptr<PictureLayerTiling>>::iterator iter =
On 2015/11/17 23:49:41, danakj (behind on reviews) wrote:
> auto

Done.

[vmpstr, 2015-11-18 00:13:02]

vmpstr@chromium.org changed reviewers:
+ dcheng@chromium.org, sievers@chromium.org

[vmpstr, 2015-11-18 00:13:03]

+sievers for content/
+dcheng for content/common/cc_messages.cc

[dcheng, 2015-11-18 00:47:10]

https://codereview.chromium.org/1437413002/diff/190001/content/common/cc_mess...
File content/common/cc_messages.cc (right):

https://codereview.chromium.org/1437413002/diff/190001/content/common/cc_mess...
content/common/cc_messages.cc:683: const cc::RenderPass* pass =
p.render_pass_list[i].get();
Would this work instead?

for (const auto& pass : p.render_pass_list) {
  to_reserve += sizeof(size_t) * 2;
  to_reserve += ReserveSizeForRenderPassWrite(*pass);
}

[vmpstr, 2015-11-18 00:53:44]

PTAL

https://codereview.chromium.org/1437413002/diff/190001/content/common/cc_mess...
File content/common/cc_messages.cc (right):

https://codereview.chromium.org/1437413002/diff/190001/content/common/cc_mess...
content/common/cc_messages.cc:683: const cc::RenderPass* pass =
p.render_pass_list[i].get();
On 2015/11/18 00:47:10, dcheng wrote:
> Would this work instead?
> 
> for (const auto& pass : p.render_pass_list) {
>   to_reserve += sizeof(size_t) * 2;
>   to_reserve += ReserveSizeForRenderPassWrite(*pass);
> }

Yep. Done.

[dcheng, 2015-11-18 00:59:23]

cc_messages.cc lgtm

[vmpstr, 2015-11-18 01:00:19]

vmpstr@chromium.org changed reviewers:
+ piman@chromium.org

[vmpstr, 2015-11-18 01:00:20]

+piman for content as well.

[no sievers, 2015-11-18 01:06:06]

content lgtm

[vmpstr, 2015-11-18 01:06:36]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2015-11-18 01:06:36]

The patchset sent to the CQ was uploaded after l-g-t-m from ericrk@chromium.org,
danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/1437413002/#ps210001
(title: "dcheng's review")

[commit-bot: I haz the power, 2015-11-18 01:08:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1437413002/210001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1437413002/210001

[commit-bot: I haz the power, 2015-11-18 01:51:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-18 01:51:27]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/7...)

[vmpstr, 2015-11-18 03:46:25]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2015-11-18 03:46:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1437413002/210001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1437413002/210001

[commit-bot: I haz the power, 2015-11-18 04:54:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-18 04:54:01]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[vmpstr, 2015-11-18 05:56:09]

Patchset #8 (id:230001) has been deleted

[vmpstr, 2015-11-18 09:19:06]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2015-11-18 09:19:07]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
ericrk@chromium.org, sievers@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/1437413002/#ps290001
(title: " ")

[commit-bot: I haz the power, 2015-11-18 09:19:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1437413002/290001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1437413002/290001

[commit-bot: I haz the power, 2015-11-18 10:41:37]

Committed patchset #10 (id:290001)

[commit-bot: I haz the power, 2015-11-18 10:42:39]

Patchset 10 (id:??) landed as
https://crrev.com/a370ef52788ea0f6c410b05ec40a74ee9bf3a647
Cr-Commit-Position: refs/heads/master@{#360318}