cc: Optimise shared raster tile handling in raster tile priority queue.

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 e0591b3d2a228a07db43edbdd28b371aa372d4c9..57982cf0949f8175ed88014bf5b940dcf0bfb844 100644
--- a/cc/resources/raster_tile_priority_queue.cc
+++ b/cc/resources/raster_tile_priority_queue.cc
@@ -122,7 +122,8 @@ RasterTilePriorityQueue::PairedPictureLayerQueue::PairedPictureLayerQueue(
                            ? PictureLayerImpl::LayerRasterTileIterator(
                                  layer_pair.pending,
                                  tree_priority == SMOOTHNESS_TAKES_PRIORITY)
-                           : PictureLayerImpl::LayerRasterTileIterator()) {
+                           : PictureLayerImpl::LayerRasterTileIterator()),
+      has_both_layers(layer_pair.active && layer_pair.pending) {
 }
 
 RasterTilePriorityQueue::PairedPictureLayerQueue::~PairedPictureLayerQueue() {
@@ -141,9 +142,11 @@ Tile* RasterTilePriorityQueue::PairedPictureLayerQueue::Top(
       next_tree == ACTIVE_TREE ? &active_iterator : &pending_iterator;
   DCHECK(*next_iterator);
   Tile* tile = **next_iterator;
+#if DCHECK_IS_ON
   DCHECK(std::find(returned_shared_tiles.begin(),
                    returned_shared_tiles.end(),
                    tile) == returned_shared_tiles.end());
+#endif
   return tile;
 }
 
@@ -155,24 +158,57 @@ void RasterTilePriorityQueue::PairedPictureLayerQueue::Pop(
   PictureLayerImpl::LayerRasterTileIterator* next_iterator =
       next_tree == ACTIVE_TREE ? &active_iterator : &pending_iterator;
   DCHECK(*next_iterator);
+#if DCHECK_IS_ON
   returned_shared_tiles.push_back(**next_iterator);
+#endif
   ++(*next_iterator);
 
-  if (IsEmpty())
-    return;
-
-  next_tree = NextTileIteratorTree(tree_priority);
-  next_iterator =
-      next_tree == ACTIVE_TREE ? &active_iterator : &pending_iterator;
-  while (std::find(returned_shared_tiles.begin(),
-                   returned_shared_tiles.end(),
-                   **next_iterator) != returned_shared_tiles.end()) {
-    ++(*next_iterator);
-    if (IsEmpty())
-      break;
+  for (; !IsEmpty(); ++(*next_iterator)) {
     next_tree = NextTileIteratorTree(tree_priority);
     next_iterator =
         next_tree == ACTIVE_TREE ? &active_iterator : &pending_iterator;
+    if (has_both_layers) {

[vmpstr, 2014/09/04 18:26:01]

nit: do

if (!has_both_layers)
  break;

Tile* tile = **next_iterator;
if (!tile->is_shared())
  break;

So that it's not as indented. Also make a comment above the first if saying
something along the lines of "We have to do an explicit check that we have both
layers, since tile shared could mean that it is shared with a recycled tree"

[USE eero AT chromium.org, 2014/09/05 07:51:26]

Done. In addition, it is possible that a shared tile is shared between an active
and a pending layer and either or them does not have valid tile priorities.

+      Tile* tile = **next_iterator;
+      if (tile->is_shared()) {
+        switch (tree_priority) {
+          case SMOOTHNESS_TAKES_PRIORITY:
+            // If we reach a pending tile, all shared tiles have been returned
+            // once as active tiles and should not be returned again.
+            if (next_tree != ACTIVE_TREE)
+              continue;
+            break;
+          case NEW_CONTENT_TAKES_PRIORITY:
+            // If we reach an active tile, all shared tiles have been returned
+            // once as pending tiles and should not be returned again.
+            if (next_tree != PENDING_TREE)
+              continue;
+            break;
+          case SAME_PRIORITY_FOR_BOTH_TREES: {
+            // Skip the shared tile if its local priority in the next tree is
+            // lower than its local priority in the other tree in order to
+            // skip it on the second time but not on the first time it is
+            // encountered.
+            const TilePriority& active_priority = tile->priority(ACTIVE_TREE);
+            const TilePriority& pending_priority = tile->priority(PENDING_TREE);
+            WhichTree main_tree =

[vmpstr, 2014/09/04 18:26:01]

nit: Name this something like "higher_priority_tree" or something else. It's
hard to understand what main_tree means in the context.

[USE eero AT chromium.org, 2014/09/05 07:51:25]

Done.

+                pending_priority.IsHigherPriorityThan(active_priority)
+                    ? PENDING_TREE
+                    : ACTIVE_TREE;
+            if (next_tree != main_tree)
+              continue;
+            break;
+          }
+          default:
+            NOTREACHED();
+        }
+      }
+    }
+#if DCHECK_IS_ON
+    DCHECK(std::find(returned_shared_tiles.begin(),
+                     returned_shared_tiles.end(),
+                     **next_iterator) == returned_shared_tiles.end());
+#endif
+    break;
   }
 }