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

cc/resources/raster_tile_priority_queue.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/resources/raster_tile_priority_queue.h"
 
 namespace cc {
 
 namespace {
 
@@ skipping 134 matching lines @@
                           ? PictureLayerImpl::LayerRasterTileIterator(
                                 layer_pair.active,
                                 tree_priority == SMOOTHNESS_TAKES_PRIORITY)
                           : PictureLayerImpl::LayerRasterTileIterator()),
       pending_iterator(layer_pair.pending
                            ? PictureLayerImpl::LayerRasterTileIterator(
                                  layer_pair.pending,
                                  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() {
 }
 
 bool RasterTilePriorityQueue::PairedPictureLayerQueue::IsEmpty() const {
   return !active_iterator && !pending_iterator;
 }
 
 Tile* RasterTilePriorityQueue::PairedPictureLayerQueue::Top(
@@ skipping 13 matching lines @@
     TreePriority tree_priority) {
   DCHECK(!IsEmpty());
 
   WhichTree next_tree = NextTileIteratorTree(tree_priority);
   PictureLayerImpl::LayerRasterTileIterator* next_iterator =
       next_tree == ACTIVE_TREE ? &active_iterator : &pending_iterator;
   DCHECK(*next_iterator);
   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 {
   DCHECK(!IsEmpty());
 
   // If we only have one iterator with tiles, return it.
   if (!active_iterator)
     return PENDING_TREE;
   if (!pending_iterator)
     return ACTIVE_TREE;
 
   // Now both iterators have tiles, so we have to decide based on tree priority.
   return HigherPriorityTree(
       tree_priority, &active_iterator, &pending_iterator, NULL);
 }
 
 }  // namespace cc