cc: Ensure to skip the first tile that would be returned by twin.

cc/resources/raster_tile_priority_queue.cc

Index: cc/resources/raster_tile_priority_queue.cc
diff --git a/cc/resources/raster_tile_priority_queue.cc b/cc/resources/raster_tile_priority_queue.cc
index 64a4a913ebfa78cfd4444f2b9b62a52ccad50d96..5e076c39a7c8f4c63b69ca43586a3fef6658e0b6 100644
--- a/cc/resources/raster_tile_priority_queue.cc
+++ b/cc/resources/raster_tile_priority_queue.cc
@@ -152,6 +152,8 @@ RasterTilePriorityQueue::PairedPictureLayerQueue::PairedPictureLayerQueue(
                                  tree_priority == SMOOTHNESS_TAKES_PRIORITY)
                            : PictureLayerImpl::LayerRasterTileIterator()),
       has_both_layers(layer_pair.active && layer_pair.pending) {
+  if (has_both_layers)
+    SkipNextSharedTileReturnedByTwin(tree_priority);
 }
 
 RasterTilePriorityQueue::PairedPictureLayerQueue::~PairedPictureLayerQueue() {
@@ -185,31 +187,36 @@ void RasterTilePriorityQueue::PairedPictureLayerQueue::Pop(
   DCHECK(returned_tiles_for_debug.insert(**next_iterator).second);
   ++(*next_iterator);
 
-  if (has_both_layers) {
-    // We have both layers (active and pending) thus we can encounter shared
-    // tiles twice (from the active iterator and from the pending iterator).
-    for (; !IsEmpty(); ++(*next_iterator)) {
-      next_tree = NextTileIteratorTree(tree_priority);
-      next_iterator =
-          next_tree == ACTIVE_TREE ? &active_iterator : &pending_iterator;
-
-      // Accept all non-shared tiles.
-      const Tile* tile = **next_iterator;
-      if (!tile->is_shared())
-        break;
-
-      // Accept a shared tile if the next tree is the higher priority one
-      // corresponding the iterator (active or pending) which usually (but due
-      // to spiral iterators not always) returns the shared tile first.
-      if (next_tree == HigherPriorityTree(tree_priority, NULL, NULL, tile))
-        break;
-    }
-  }
+  if (has_both_layers)
+    SkipNextSharedTileReturnedByTwin(tree_priority);
 
   // If no empty, use Top to do DCHECK the next iterator.
   DCHECK(IsEmpty() || Top(tree_priority));
 }
 
+void RasterTilePriorityQueue::PairedPictureLayerQueue::
+    SkipNextSharedTileReturnedByTwin(TreePriority tree_priority) {
+  // We have both layers (active and pending) thus we can encounter shared
+  // tiles twice (from the active iterator and from the pending iterator).
+  PictureLayerImpl::LayerRasterTileIterator* next_iterator = NULL;

[reveman, 2014/10/10 14:27:22]

I find the use of next_iterator confusing. it's incremented in the for loop
statement but then assigned a new value right after that. what's going on here?

I think |next_iterator| being used as part of the for statement but set to null
initially is a bit confusing. Maybe change this to a "do while" loop to make it
more clear that we'll never try to increment it when null.

[reveman, 2014/10/10 14:40:47]

s/NULL/nullptr/

[vmpstr, 2014/10/10 15:24:05]

I made this into a while loop. FWIW, I just moved the code from above into a
separate function :)

And changed to nullptr.

+  for (; !IsEmpty(); ++(*next_iterator)) {
+    WhichTree next_tree = NextTileIteratorTree(tree_priority);
+    next_iterator =
+        next_tree == ACTIVE_TREE ? &active_iterator : &pending_iterator;
+
+    // Accept all non-shared tiles.
+    const Tile* tile = **next_iterator;
+    if (!tile->is_shared())
+      break;
+
+    // Accept a shared tile if the next tree is the higher priority one
+    // corresponding the iterator (active or pending) which usually (but due
+    // to spiral iterators not always) returns the shared tile first.
+    if (next_tree == HigherPriorityTree(tree_priority, NULL, NULL, tile))
+      break;
+  }
+}
+
 WhichTree
 RasterTilePriorityQueue::PairedPictureLayerQueue::NextTileIteratorTree(
     TreePriority tree_priority) const {