cc: Fix size_t to int truncations in tiles/ and trees/

cc: Fix size_t to int truncations in tiles/ and trees/

This patch fixes size_t to int truncations for tiles/ and trees/
directories in cc.

BUG=167187
R=danakj
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/24b32b3ee27a1fa020cf1b263bc71fbc8f077b2d
Cr-Commit-Position: refs/heads/master@{#333598}

[vmpstr, 2015-06-09 01:42:18]

Please take a look.

[danakj, 2015-06-09 17:39:50]

https://codereview.chromium.org/1168903003/diff/20001/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/1168903003/diff/20001/cc/layers/layer.h#newco...
cc/layers/layer.h:638: size_t num_dependents_need_push_properties_;
How come? We just ++ and -- it, it's not related to vectors or something? Or
where is it?

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/picture_layer_...
File cc/tiles/picture_layer_tiling_set.cc (right):

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/picture_layer_...
cc/tiles/picture_layer_tiling_set.cc:391: for (ideal_tiling_ = 0; ideal_tiling_
< set_->tilings_.size();
can you cache the size up here so we only call it once?

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/picture_layer_...
cc/tiles/picture_layer_tiling_set.cc:553: DCHECK_LE(tilings_.size(),
Wat. You went to all the trouble to use size_t everywhere, then DCHECK the size
is small here? How come?

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:978: // particular operator-=) can cause internal
values to become negative. Thus,
Can we DCHECK that they won't become negative? Is it intended?

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/tile_manager.h
File cc/tiles/tile_manager.h (right):

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/tile_manager.h...
cc/tiles/tile_manager.h:233: int64 memory_bytes_;
why not size_ts here?

https://codereview.chromium.org/1168903003/diff/20001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1168903003/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:1364: for (size_t i = 0; i <
layer->children().size(); ++i) {
we could cache the size() for each of these and then we don't have to keep
calling it.

[vmpstr, 2015-06-09 18:37:17]

https://codereview.chromium.org/1168903003/diff/20001/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/1168903003/diff/20001/cc/layers/layer.h#newco...
cc/layers/layer.h:638: size_t num_dependents_need_push_properties_;
On 2015/06/09 17:39:49, danakj wrote:
> How come? We just ++ and -- it, it's not related to vectors or something? Or
> where is it?

Hmm. I think this might've been just a test thing that had a truncations, I
fixed that part instead.

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/picture_layer_...
File cc/tiles/picture_layer_tiling_set.cc (right):

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/picture_layer_...
cc/tiles/picture_layer_tiling_set.cc:391: for (ideal_tiling_ = 0; ideal_tiling_
< set_->tilings_.size();
On 2015/06/09 17:39:49, danakj wrote:
> can you cache the size up here so we only call it once?

Done.

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/picture_layer_...
cc/tiles/picture_layer_tiling_set.cc:553: DCHECK_LE(tilings_.size(),
On 2015/06/09 17:39:49, danakj wrote:
> Wat. You went to all the trouble to use size_t everywhere, then DCHECK the
size
> is small here? How come?

I have fixed this.

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:978: // particular operator-=) can cause internal
values to become negative. Thus,
On 2015/06/09 17:39:49, danakj wrote:
> Can we DCHECK that they won't become negative? Is it intended?

Yeah the size going negative is somewhat intentional. We can subtract a memory
usage that is larger that this.

So we can do a -= b; where b > a, in that case a will become negative. This just
means that a.Exceeds(c) will be false for any regularly formed c. 

I feel like this is just an implementation detail of this class. The alternative
would be to have a check in operator-= to make sure it doesn't go below 0. Do
you think that's preferable?

https://codereview.chromium.org/1168903003/diff/20001/cc/trees/layer_tree_imp...
File cc/trees/layer_tree_impl.cc (right):

https://codereview.chromium.org/1168903003/diff/20001/cc/trees/layer_tree_imp...
cc/trees/layer_tree_impl.cc:1364: for (size_t i = 0; i <
layer->children().size(); ++i) {
On 2015/06/09 17:39:49, danakj wrote:
> we could cache the size() for each of these and then we don't have to keep
> calling it.

Done.

[danakj, 2015-06-09 19:05:47]

lgtm

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/1168903003/diff/20001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:978: // particular operator-=) can cause internal
values to become negative. Thus,
On 2015/06/09 18:37:17, vmpstr wrote:
> On 2015/06/09 17:39:49, danakj wrote:
> > Can we DCHECK that they won't become negative? Is it intended?
> 
> Yeah the size going negative is somewhat intentional. We can subtract a memory
> usage that is larger that this.
> 
> So we can do a -= b; where b > a, in that case a will become negative. This
just
> means that a.Exceeds(c) will be false for any regularly formed c. 
> 
> I feel like this is just an implementation detail of this class. The
alternative
> would be to have a check in operator-= to make sure it doesn't go below 0. Do
> you think that's preferable?
> 

Ya lets just set to 0 if it would become negative.

[danakj, 2015-06-09 20:16:46]

https://codereview.chromium.org/1168903003/diff/60001/cc/tiles/tile_manager.h
File cc/tiles/tile_manager.h (right):

https://codereview.chromium.org/1168903003/diff/60001/cc/tiles/tile_manager.h...
cc/tiles/tile_manager.h:216: MemoryUsage(size_t memory_bytes, size_t
resource_count);
Why are these size_t then if the stuff inside won't be?

[vmpstr, 2015-06-09 20:27:33]

https://codereview.chromium.org/1168903003/diff/60001/cc/tiles/tile_manager.h
File cc/tiles/tile_manager.h (right):

https://codereview.chromium.org/1168903003/diff/60001/cc/tiles/tile_manager.h...
cc/tiles/tile_manager.h:216: MemoryUsage(size_t memory_bytes, size_t
resource_count);
On 2015/06/09 20:16:45, danakj wrote:
> Why are these size_t then if the stuff inside won't be?

The public interface to MemoryUsage uses size_t, since they represent a memory
usage. You can't construct something from negative values... Internally it needs
signness to make sure Exceeds works properly with memory usages that were
computed using subtraction. I mean it could use size_t with extra size_ts to
indicate how much more was subtracted than was available, but that's kind of
very convenient to represent as a signed number...

I could change the ctor to take int64 and int, but then on construction there
would be a cast? I'm not sure if that's better.

[danakj, 2015-06-09 20:29:43]

https://codereview.chromium.org/1168903003/diff/60001/cc/tiles/tile_manager.h
File cc/tiles/tile_manager.h (right):

https://codereview.chromium.org/1168903003/diff/60001/cc/tiles/tile_manager.h...
cc/tiles/tile_manager.h:226: size_t memory_bytes() const {
Can you make this return an int(64?) instead then?

[vmpstr, 2015-06-09 21:00:56]

PTAL

https://codereview.chromium.org/1168903003/diff/60001/cc/tiles/tile_manager.h
File cc/tiles/tile_manager.h (right):

https://codereview.chromium.org/1168903003/diff/60001/cc/tiles/tile_manager.h...
cc/tiles/tile_manager.h:226: size_t memory_bytes() const {
On 2015/06/09 20:29:42, danakj wrote:
> Can you make this return an int(64?) instead then?

Done.

[danakj, 2015-06-09 21:04:30]

LGTM !

[vmpstr, 2015-06-09 21:05:17]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2015-06-09 21:08:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1168903003/120001

[commit-bot: I haz the power, 2015-06-09 22:39:44]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-06-09 22:41:30]

Patchset 7 (id:??) landed as
https://crrev.com/24b32b3ee27a1fa020cf1b263bc71fbc8f077b2d
Cr-Commit-Position: refs/heads/master@{#333598}