cc: Change TileManager iterators to be queues.

cc: Change TileManager iterators to be queues.

This patch is mostly just moving code around. It moves the iterators
from tile manager to be in separate files, renames them to be queues.

It also adds a tile manager client interface for building the queues.

The rest is just fix ups for the tests.

R=reveman

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

[vmpstr, 2014-07-18 19:54:24]

PTAL. Note that this is mostly moving code around. Let's delay the actual
changes to the code for another patch.

[reveman, 2014-07-18 21:08:06]

https://codereview.chromium.org/406543003/diff/1/cc/layers/picture_layer_impl.h
File cc/layers/picture_layer_impl.h (right):

https://codereview.chromium.org/406543003/diff/1/cc/layers/picture_layer_impl...
cc/layers/picture_layer_impl.h:32: PictureLayerImpl* pending_layer;
nit: don't think _layer suffix is necessary.

https://codereview.chromium.org/406543003/diff/1/cc/layers/picture_layer_impl...
cc/layers/picture_layer_impl.h:33: };
How about PictureLayerImpl::Pair? or simply std::pair?

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
File cc/resources/eviction_tile_priority_queue.cc (right):

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.cc:23:
queue_heap_.reserve(paired_layers.size());
both these reserve calls can be removed after my suggestion to the .h file.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.cc:44: }
Can we remove this and if worthwhile add it in a follow up instead? Looks like
the rest of the code in this file will handle the case when PeekTile returns
NULL just fine.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.cc:63: if
(paired_queue->PeekTile(tree_priority_) != NULL) {
remove this conditional and add it in a follow up if it turns out it's
important?

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.cc:70: return queue_heap_.empty();
With my other suggestions I think we need "||
!queue_heap_.front()->PeekTile(tree_priority_)" here.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
File cc/resources/eviction_tile_priority_queue.h (right):

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.h:36: void PopTile(TreePriority
tree_priority);
Pop/Top above but PeekTile/PopTile here. Is there a good reason for that or can
we make it consistent?

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.h:44: std::vector<Tile*>
returned_shared_tiles;
Can you add a TODO here about possibly removing this vector?

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.h:56: };
You can move this to .cc after removing |comparator_|. See comment below.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.h:59:
std::vector<PairedPictureLayerQueue*> queue_heap_;
I understand that this might be needed for good performance but let's just
heapify paired_queues_ for now and we can change this in a follow up with perf
data.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.h:61: EvictionOrderComparator
comparator_;
Not obvious to me that caching this comparator is worthwhile. Let's just create
this when needed for now and we can change it in a follow up if needed.

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
File cc/resources/raster_tile_priority_queue.cc (right):

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.cc:24:
queue_heap_.reserve(paired_layers.size());
remove "reserve" calls

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.cc:43: }
remove this chunk

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.cc:62: if
(paired_queue->PeekTile(tree_priority_) != NULL) {
remove this conditional

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.cc:69: return queue_heap_.empty();
|| !queue_heap_.front()->PeekTile(tree_priority_)

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
File cc/resources/raster_tile_priority_queue.h (right):

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.h:36: void PopTile(TreePriority
tree_priority);
Pop/Top?

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.h:44: std::vector<Tile*>
returned_shared_tiles;
TODO

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.h:56: };
move to .cc

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.h:59:
std::vector<PairedPictureLayerQueue*> queue_heap_;
heapify paired_queues_ instead

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.h:61: RasterOrderComparator comparator_;
remove |comparator_|

https://codereview.chromium.org/406543003/diff/1/cc/resources/tile_manager_pe...
File cc/resources/tile_manager_perftest.cc (right):

https://codereview.chromium.org/406543003/diff/1/cc/resources/tile_manager_pe...
cc/resources/tile_manager_perftest.cc:188: ASSERT_EQ(0, count);
can you change the loop above to something like:

while (count--) {
  ASSERT_FALSE(queue.IsEmpty());
  ASSERT_TRUE(queue.Pop());
}

?

[vmpstr, 2014-07-18 23:18:14]

PTAL. I think I've addressed everything. Let me know if I missed something.

https://codereview.chromium.org/406543003/diff/1/cc/layers/picture_layer_impl.h
File cc/layers/picture_layer_impl.h (right):

https://codereview.chromium.org/406543003/diff/1/cc/layers/picture_layer_impl...
cc/layers/picture_layer_impl.h:32: PictureLayerImpl* pending_layer;
On 2014/07/18 21:08:04, reveman wrote:
> nit: don't think _layer suffix is necessary.

Done.

https://codereview.chromium.org/406543003/diff/1/cc/layers/picture_layer_impl...
cc/layers/picture_layer_impl.h:33: };
On 2014/07/18 21:08:04, reveman wrote:
> How about PictureLayerImpl::Pair? or simply std::pair?

Done. I prefer PictureLayerImpl::Pair so we have explicit names for which one is
active and which one is pending.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
File cc/resources/eviction_tile_priority_queue.cc (right):

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.cc:23:
queue_heap_.reserve(paired_layers.size());
On 2014/07/18 21:08:04, reveman wrote:
> both these reserve calls can be removed after my suggestion to the .h file.

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.cc:44: }
On 2014/07/18 21:08:04, reveman wrote:
> Can we remove this and if worthwhile add it in a follow up instead? Looks like
> the rest of the code in this file will handle the case when PeekTile returns
> NULL just fine.

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.cc:63: if
(paired_queue->PeekTile(tree_priority_) != NULL) {
On 2014/07/18 21:08:04, reveman wrote:
> remove this conditional and add it in a follow up if it turns out it's
> important?

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.cc:70: return queue_heap_.empty();
On 2014/07/18 21:08:04, reveman wrote:
> With my other suggestions I think we need "||
> !queue_heap_.front()->PeekTile(tree_priority_)" here.

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
File cc/resources/eviction_tile_priority_queue.h (right):

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.h:36: void PopTile(TreePriority
tree_priority);
On 2014/07/18 21:08:05, reveman wrote:
> Pop/Top above but PeekTile/PopTile here. Is there a good reason for that or
can
> we make it consistent?

I've changed the name and added IsEmpty

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.h:44: std::vector<Tile*>
returned_shared_tiles;
On 2014/07/18 21:08:05, reveman wrote:
> Can you add a TODO here about possibly removing this vector?

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.h:56: };
On 2014/07/18 21:08:05, reveman wrote:
> You can move this to .cc after removing |comparator_|. See comment below.

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.h:59:
std::vector<PairedPictureLayerQueue*> queue_heap_;
On 2014/07/18 21:08:05, reveman wrote:
> I understand that this might be needed for good performance but let's just
> heapify paired_queues_ for now and we can change this in a follow up with perf
> data.

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/eviction_tile_p...
cc/resources/eviction_tile_priority_queue.h:61: EvictionOrderComparator
comparator_;
On 2014/07/18 21:08:05, reveman wrote:
> Not obvious to me that caching this comparator is worthwhile. Let's just
create
> this when needed for now and we can change it in a follow up if needed.

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
File cc/resources/raster_tile_priority_queue.cc (right):

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.cc:24:
queue_heap_.reserve(paired_layers.size());
On 2014/07/18 21:08:05, reveman wrote:
> remove "reserve" calls

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.cc:43: }
On 2014/07/18 21:08:05, reveman wrote:
> remove this chunk

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.cc:62: if
(paired_queue->PeekTile(tree_priority_) != NULL) {
On 2014/07/18 21:08:05, reveman wrote:
> remove this conditional

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.cc:69: return queue_heap_.empty();
On 2014/07/18 21:08:05, reveman wrote:
> || !queue_heap_.front()->PeekTile(tree_priority_)

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
File cc/resources/raster_tile_priority_queue.h (right):

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.h:36: void PopTile(TreePriority
tree_priority);
On 2014/07/18 21:08:06, reveman wrote:
> Pop/Top?

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.h:44: std::vector<Tile*>
returned_shared_tiles;
On 2014/07/18 21:08:05, reveman wrote:
> TODO

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.h:56: };
On 2014/07/18 21:08:05, reveman wrote:
> move to .cc

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.h:59:
std::vector<PairedPictureLayerQueue*> queue_heap_;
On 2014/07/18 21:08:05, reveman wrote:
> heapify paired_queues_ instead

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/raster_tile_pri...
cc/resources/raster_tile_priority_queue.h:61: RasterOrderComparator comparator_;
On 2014/07/18 21:08:06, reveman wrote:
> remove |comparator_|

Done.

https://codereview.chromium.org/406543003/diff/1/cc/resources/tile_manager_pe...
File cc/resources/tile_manager_perftest.cc (right):

https://codereview.chromium.org/406543003/diff/1/cc/resources/tile_manager_pe...
cc/resources/tile_manager_perftest.cc:188: ASSERT_EQ(0, count);
On 2014/07/18 21:08:06, reveman wrote:
> can you change the loop above to something like:
> 
> while (count--) {
>   ASSERT_FALSE(queue.IsEmpty());
>   ASSERT_TRUE(queue.Pop());
> }
> 
> ?

Done.

[reveman, 2014-07-19 00:45:50]

ok, this looks pretty good. I think this is my last set of comments.

https://codereview.chromium.org/406543003/diff/20001/cc/layers/picture_layer_...
File cc/layers/picture_layer_impl.h (right):

https://codereview.chromium.org/406543003/diff/20001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl.h:31: Pair();
I think adding a ctor that takes 2 layers as input will clean up
LayerTreeHostImpl::GetPictureLayerImplPairs a bit.

https://codereview.chromium.org/406543003/diff/20001/cc/resources/eviction_ti...
File cc/resources/eviction_tile_priority_queue.h (right):

https://codereview.chromium.org/406543003/diff/20001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.h:20: PairedPictureLayerQueue();
How about we pass a PictureLayerImpl::Pair ref and tree priority to this ctor? I
think that will clean up ::Build a bit.

https://codereview.chromium.org/406543003/diff/20001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.h:25: void Pop(TreePriority
tree_priority);
Please reorder these based on typical usage (IsEmpty,Top,Pop) and make it
consistent (the order below is different). Please make sure the order of
implementations in .cc files match too.

https://codereview.chromium.org/406543003/diff/20001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.h:46: Tile* Top();
reorder

https://codereview.chromium.org/406543003/diff/20001/cc/resources/raster_tile...
File cc/resources/raster_tile_priority_queue.h (right):

https://codereview.chromium.org/406543003/diff/20001/cc/resources/raster_tile...
cc/resources/raster_tile_priority_queue.h:20: PairedPictureLayerQueue();
Pass a PictureLayerImpl::Pair to ctor?

https://codereview.chromium.org/406543003/diff/20001/cc/resources/raster_tile...
cc/resources/raster_tile_priority_queue.h:25: void Pop(TreePriority
tree_priority);
reorder

https://codereview.chromium.org/406543003/diff/20001/cc/resources/raster_tile...
cc/resources/raster_tile_priority_queue.h:46: Tile* Top();
reorder

https://codereview.chromium.org/406543003/diff/20001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/406543003/diff/20001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:1253: layer_pairs->clear();
Let's make this the responsibility of the caller instead.
DCHECK(layer_pairs->is_empty())

https://codereview.chromium.org/406543003/diff/20001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:1255:
layer_pairs->reserve(picture_layers_.size());
Please have the caller reuse the same vector instead of using reserve() here.

https://codereview.chromium.org/406543003/diff/20001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:1274: PictureLayerImpl::Pair layer_pair;
This temporary variable seem unnecessary. How about with a ctor that takes 2
layers as input we do .push_back(PictureLayerImpl::Pair(layer, twin_layer))
below instead?

https://codereview.chromium.org/406543003/diff/20001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:1275: WhichTree tree = layer->GetTree();
this temporary variable seem unnecessary too

[vmpstr, 2014-07-20 01:15:05]

PTAL. Let me know if I missed something.

https://codereview.chromium.org/406543003/diff/20001/cc/layers/picture_layer_...
File cc/layers/picture_layer_impl.h (right):

https://codereview.chromium.org/406543003/diff/20001/cc/layers/picture_layer_...
cc/layers/picture_layer_impl.h:31: Pair();
On 2014/07/19 00:45:50, reveman wrote:
> I think adding a ctor that takes 2 layers as input will clean up
> LayerTreeHostImpl::GetPictureLayerImplPairs a bit.

Done.

https://codereview.chromium.org/406543003/diff/20001/cc/resources/eviction_ti...
File cc/resources/eviction_tile_priority_queue.h (right):

https://codereview.chromium.org/406543003/diff/20001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.h:20: PairedPictureLayerQueue();
On 2014/07/19 00:45:50, reveman wrote:
> How about we pass a PictureLayerImpl::Pair ref and tree priority to this ctor?
I
> think that will clean up ::Build a bit.

Done.

https://codereview.chromium.org/406543003/diff/20001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.h:25: void Pop(TreePriority
tree_priority);
On 2014/07/19 00:45:50, reveman wrote:
> Please reorder these based on typical usage (IsEmpty,Top,Pop) and make it
> consistent (the order below is different). Please make sure the order of
> implementations in .cc files match too.

Done.

https://codereview.chromium.org/406543003/diff/20001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.h:46: Tile* Top();
On 2014/07/19 00:45:50, reveman wrote:
> reorder

Done.

https://codereview.chromium.org/406543003/diff/20001/cc/resources/raster_tile...
File cc/resources/raster_tile_priority_queue.h (right):

https://codereview.chromium.org/406543003/diff/20001/cc/resources/raster_tile...
cc/resources/raster_tile_priority_queue.h:20: PairedPictureLayerQueue();
On 2014/07/19 00:45:50, reveman wrote:
> Pass a PictureLayerImpl::Pair to ctor?

Done.

https://codereview.chromium.org/406543003/diff/20001/cc/resources/raster_tile...
cc/resources/raster_tile_priority_queue.h:25: void Pop(TreePriority
tree_priority);
On 2014/07/19 00:45:50, reveman wrote:
> reorder

Done.

https://codereview.chromium.org/406543003/diff/20001/cc/resources/raster_tile...
cc/resources/raster_tile_priority_queue.h:46: Tile* Top();
On 2014/07/19 00:45:50, reveman wrote:
> reorder

Done.

https://codereview.chromium.org/406543003/diff/20001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/406543003/diff/20001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:1253: layer_pairs->clear();
On 2014/07/19 00:45:50, reveman wrote:
> Let's make this the responsibility of the caller instead.
> DCHECK(layer_pairs->is_empty())

Done.

https://codereview.chromium.org/406543003/diff/20001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:1255:
layer_pairs->reserve(picture_layers_.size());
On 2014/07/19 00:45:50, reveman wrote:
> Please have the caller reuse the same vector instead of using reserve() here.

Done.

https://codereview.chromium.org/406543003/diff/20001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:1274: PictureLayerImpl::Pair layer_pair;
On 2014/07/19 00:45:50, reveman wrote:
> This temporary variable seem unnecessary. How about with a ctor that takes 2
> layers as input we do .push_back(PictureLayerImpl::Pair(layer, twin_layer))
> below instead?

Done.

https://codereview.chromium.org/406543003/diff/20001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:1275: WhichTree tree = layer->GetTree();
On 2014/07/19 00:45:50, reveman wrote:
> this temporary variable seem unnecessary too

Done.

[reveman, 2014-07-21 01:39:07]

lgtm with nits

https://codereview.chromium.org/406543003/diff/40001/cc/resources/eviction_ti...
File cc/resources/eviction_tile_priority_queue.cc (right):

https://codereview.chromium.org/406543003/diff/40001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.cc:137: layer_pair.active,
tree_priority);
nit: consider moving this to the initialization list as that would be more
efficient. I would prefer that unless makes eviction or raster queue code less
clean.

https://codereview.chromium.org/406543003/diff/40001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.cc:142: layer_pair.pending,
tree_priority);
nit: consider moving this to the initialization list.

https://codereview.chromium.org/406543003/diff/40001/cc/resources/eviction_ti...
File cc/resources/eviction_tile_priority_queue.h (right):

https://codereview.chromium.org/406543003/diff/40001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.h:48: Tile* Top();
nit: IsEmpty,Top,Pop

https://codereview.chromium.org/406543003/diff/40001/cc/resources/raster_tile...
File cc/resources/raster_tile_priority_queue.cc (right):

https://codereview.chromium.org/406543003/diff/40001/cc/resources/raster_tile...
cc/resources/raster_tile_priority_queue.cc:125: layer_pair.active,
prioritize_low_res);
nit: consider moving this to the initialization list.

https://codereview.chromium.org/406543003/diff/40001/cc/resources/raster_tile...
cc/resources/raster_tile_priority_queue.cc:130: layer_pair.pending,
prioritize_low_res);
nit: consider moving this to the initialization list.

[vmpstr, 2014-07-21 16:33:49]

https://codereview.chromium.org/406543003/diff/40001/cc/resources/eviction_ti...
File cc/resources/eviction_tile_priority_queue.cc (right):

https://codereview.chromium.org/406543003/diff/40001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.cc:137: layer_pair.active,
tree_priority);
On 2014/07/21 01:39:06, reveman wrote:
> nit: consider moving this to the initialization list as that would be more
> efficient. I would prefer that unless makes eviction or raster queue code less
> clean.

Done.

https://codereview.chromium.org/406543003/diff/40001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.cc:142: layer_pair.pending,
tree_priority);
On 2014/07/21 01:39:06, reveman wrote:
> nit: consider moving this to the initialization list.

Done.

https://codereview.chromium.org/406543003/diff/40001/cc/resources/eviction_ti...
File cc/resources/eviction_tile_priority_queue.h (right):

https://codereview.chromium.org/406543003/diff/40001/cc/resources/eviction_ti...
cc/resources/eviction_tile_priority_queue.h:48: Tile* Top();
On 2014/07/21 01:39:06, reveman wrote:
> nit: IsEmpty,Top,Pop

Done.

https://codereview.chromium.org/406543003/diff/40001/cc/resources/raster_tile...
File cc/resources/raster_tile_priority_queue.cc (right):

https://codereview.chromium.org/406543003/diff/40001/cc/resources/raster_tile...
cc/resources/raster_tile_priority_queue.cc:125: layer_pair.active,
prioritize_low_res);
On 2014/07/21 01:39:06, reveman wrote:
> nit: consider moving this to the initialization list.

Done.

https://codereview.chromium.org/406543003/diff/40001/cc/resources/raster_tile...
cc/resources/raster_tile_priority_queue.cc:130: layer_pair.pending,
prioritize_low_res);
On 2014/07/21 01:39:06, reveman wrote:
> nit: consider moving this to the initialization list.

Done.

[vmpstr, 2014-07-21 16:33:53]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2014-07-21 16:34:57]

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

[commit-bot: I haz the power, 2014-07-21 21:45:48]

Change committed as 284506

[Kunihiko Sakamoto, 2014-07-22 02:34:42]

A revert of this CL has been created in
https://codereview.chromium.org/406183002/ by ksakamoto@chromium.org.

The reason for reverting is:
TileManagerTilePriorityQueueTest.EvictionTilePriorityQueueWithOcclusion
consistently times out on Win7 bot
http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%28...
.