cc: Make tiles that are not required by trees NEVER_BIN

cc: Make tiles that are not required by trees NEVER_BIN

Currently if we have allow anything mode, we never have a tile
in the NEVER_BIN, even if both active and pending trees don't
own the tile anymore (ie it's being kept alive by the raster
callback). This cleans it up.

BUG=251938
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=208047

[vmpstr, 2013-06-20 17:57:39]

As per our discussion offline, you were right that these tiles are actually
being scheduled with a lower priority than visible once. However, if we have
memory they _are_ being scheduled, which shouldn't be the case. At the very
least this should reduce some memory usage. Please take a look.

[reveman, 2013-06-21 01:59:16]

https://codereview.chromium.org/16959022/diff/1/cc/resources/tile_manager.cc
File cc/resources/tile_manager.cc (right):

https://codereview.chromium.org/16959022/diff/1/cc/resources/tile_manager.cc#...
cc/resources/tile_manager.cc:30: if (!prio.required_by_tree)
is there a case where this is different than "distance_to_visible_in_pixels ==
std::numeric_limits<float>::infinity()"? I'd prefer to use that if possible but
I'm OK with this too.

nit: move this check below the constants. I don't see a reason for this to be
different than the other if statements.

[vmpstr, 2013-06-21 15:47:02]

On 2013/06/21 01:59:16, David Reveman wrote:
> https://codereview.chromium.org/16959022/diff/1/cc/resources/tile_manager.cc
> File cc/resources/tile_manager.cc (right):
> 
>
https://codereview.chromium.org/16959022/diff/1/cc/resources/tile_manager.cc#...
> cc/resources/tile_manager.cc:30: if (!prio.required_by_tree)
> is there a case where this is different than "distance_to_visible_in_pixels ==
> std::numeric_limits<float>::infinity()"? I'd prefer to use that if possible
but
> I'm OK with this too.
> 
> nit: move this check below the constants. I don't see a reason for this to be
> different than the other if statements.

Yeah that's a good point. I got too hung up on "time_to_visible" being infinite
for non-never tiles. Now the patch just adds an if. PTAL.

[reveman, 2013-06-21 15:59:09]

lgtm

[commit-bot: I haz the power, 2013-06-21 16:40:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vmpstr@chromium.org/16959022/5001

[commit-bot: I haz the power, 2013-06-21 17:24:28]

Retried try job too often on linux_rel for step(s) cc_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[vmpstr, 2013-06-21 18:16:15]

On 2013/06/21 17:24:28, I haz the power (commit-bot) wrote:
> Retried try job too often on linux_rel for step(s) cc_unittests
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

Please take another look. This exposed a bug that we had: In the case where we
take memory away from pending never active not-now tiles, we were not taking
memory if it was scheduled to run, but has not finished running yet. We haven't
noticed this, because in the allow anything case, a tile was never in the never
bin (regardless of tile_priority). So we were just getting lucky on the
unittests. I added another one that properly breaks without the NEVER_BIN patch.

[reveman, 2013-06-21 19:27:49]

lgtm

https://chromiumcodereview.appspot.com/16959022/diff/7004/cc/resources/tile_m...
File cc/resources/tile_manager.cc (right):

https://chromiumcodereview.appspot.com/16959022/diff/7004/cc/resources/tile_m...
cc/resources/tile_manager.cc:527: // then it means that we have allocated memory
for it.
nit: that we have assigned memory to it?

[vmpstr, 2013-06-21 20:10:27]

https://codereview.chromium.org/16959022/diff/7004/cc/resources/tile_manager.cc
File cc/resources/tile_manager.cc (right):

https://codereview.chromium.org/16959022/diff/7004/cc/resources/tile_manager....
cc/resources/tile_manager.cc:527: // then it means that we have allocated memory
for it.
On 2013/06/21 19:27:50, David Reveman wrote:
> nit: that we have assigned memory to it?

Done.

[commit-bot: I haz the power, 2013-06-21 20:10:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vmpstr@chromium.org/16959022/22001

[commit-bot: I haz the power, 2013-06-22 02:33:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vmpstr@chromium.org/16959022/22001

[commit-bot: I haz the power, 2013-06-22 03:01:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vmpstr@chromium.org/16959022/22001

[commit-bot: I haz the power, 2013-06-22 14:51:19]

Change committed as 208047