cc: Bump up pending tree now tiles order for smoothness mode.

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 51 matching lines @@
  private:
   TreePriority tree_priority_;
 };
 
 WhichTree HigherPriorityTree(
     TreePriority tree_priority,
     const PictureLayerImpl::LayerRasterTileIterator* active_iterator,
     const PictureLayerImpl::LayerRasterTileIterator* pending_iterator,
     const Tile* shared_tile) {
   switch (tree_priority) {
-    case SMOOTHNESS_TAKES_PRIORITY:
+    case SMOOTHNESS_TAKES_PRIORITY: {
+      const Tile* active_tile = shared_tile ? shared_tile : **active_iterator;
+      const Tile* pending_tile = shared_tile ? shared_tile : **pending_iterator;
+
+      const TilePriority& active_priority = active_tile->priority(ACTIVE_TREE);
+      const TilePriority& pending_priority =
+          pending_tile->priority(PENDING_TREE);
+
+      // If we've down to eventually bin tiles on the active tree, but the
+      // pending tree has needed now tiles, process the pending tree to avoid
+      // activation starvation.

[reveman, 2014/10/13 19:45:11]

I don't see how this used to starve activation differently than SOON tiles on
active tree would. It seems fine to change the priority in
SMOOTHNESS_TAKES_PRIORITY mode as this patch does here but I don't see it as
some kind of unique situation where activation is starved. This is just
prioritizing the active tree differently, lowers the priority of the active tree
a bit afaict.

Did this used to be prioritized differently? If there's indeed some unique
situation where activation is starved and this is not just a change in priority,
then we should be able to add a test for it. 

[vmpstr, 2014/10/13 20:20:13]

In the pre-iterator approach, tiles that are required for activation were always
non-never, which means that once a list of tiles to be considered was
constructed, the activation tiles were simply tacked on at the end (in
smoothness takes priority, of course). Since the iterator approach can't do this
type of a post-process step of tacking things on, we need to be mindful of the
initial order in which the tiles are returned.

What this means in this particular case is that, yes, this is just a change of
priority for tiles that are potentially required for activation. However, in a
prepaint-only memory policy, this also means that we'll actually get to the
required for activation tiles before we abort iteration due to memory policy
violation. In other words, this is the order of tiles in smoothness mode:

active now
---------------- bare minimum*
active soon
required for activation (regardless of active bin)
---------------- allow prepaint*
active eventually
---------------- allow anything*

Bare minimum mode would only cut off the active now tiles. That mode is
currently used for testing only and it's not really appropriate for smoothness
mode (ganesh might use this, but ganesh would also not have a pending tree in
general).

Allow prepaint is pretty common for low mem devices, as I understand it and we
want to ensure that activation tiles are not pruned by this mode.

Without this file's change, but with the rest of the changes in this patch, the
order would like like this:

active now
---------------- bare minimum*
active soon
---------------- allow prepaint*
active eventually
required for activation (regardless of active bin)
---------------- allow anything*

This would fix this for most cases and fix the jank on the bots, but anytime
we're in allow prepaint only mode, we'd run into the same issue.

[reveman, 2014/10/13 21:07:36]

The change looks fine to me. It's just the comment that I'm not a 100% sure
about. Instead of "activation starvation" can it say something along the lines
of "process the pending tree to allow tiles required for activation to be
initialized when memory policy only allows prepaint"?

[vmpstr, 2014/10/13 21:33:40]

Done.

+      if (active_priority.priority_bin == TilePriority::EVENTUALLY &&
+          pending_priority.priority_bin == TilePriority::NOW) {
+        return PENDING_TREE;
+      }
       return ACTIVE_TREE;
+    }
     case NEW_CONTENT_TAKES_PRIORITY:
       return PENDING_TREE;
     case SAME_PRIORITY_FOR_BOTH_TREES: {
       const Tile* active_tile = shared_tile ? shared_tile : **active_iterator;
       const Tile* pending_tile = shared_tile ? shared_tile : **pending_iterator;
 
       const TilePriority& active_priority = active_tile->priority(ACTIVE_TREE);
       const TilePriority& pending_priority =
           pending_tile->priority(PENDING_TREE);
 
@@ skipping 144 matching lines @@
     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, nullptr);
 }
 
 }  // namespace cc