cc: Fix Raster/Eviction order comparators to return stable values.

For make_heap et al, it is important that we return consistent values
representing strict ordering. For instance, if we Comp(a, b) returns
true, then it must be the case that Comp(b, a) returns false.

Currently, because of IsEmpty checks we can return true in both cases,
which makes stl throw an exception.

This fix, along with added tests fixes this issue.

R=reveman

Review URL: https://codereview.chromium.org/448363002

Cr-Commit-Position: refs/heads/master@{#288818}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288818 0039d316-1c4b-4281-b951-d872f2087c98

cc/layers/picture_layer_impl.h

@@ -143,7 +143,8 @@ class CC_EXPORT PictureLayerImpl
   // Functions used by tile manager.
   PictureLayerImpl* GetTwinLayer() { return twin_layer_; }
   bool IsOnActiveOrPendingTree() const;
-  bool HasValidTilePriorities() const;
+  // Virtual for testing.
+  virtual bool HasValidTilePriorities() const;
   bool AllTilesRequiredForActivationAreReadyToDraw() const;
 
  protected:

cc/resources/eviction_tile_priority_queue.cc

@@ -16,11 +16,11 @@ class EvictionOrderComparator {
   bool operator()(
       const EvictionTilePriorityQueue::PairedPictureLayerQueue* a,
       const EvictionTilePriorityQueue::PairedPictureLayerQueue* b) const {
-    if (a->IsEmpty())
-      return true;
-
-    if (b->IsEmpty())
-      return false;
+    // Note that in this function, we have to return true if and only if
+    // b is strictly lower priority than a. Note that for the sake of
+    // completeness, empty queue is considered to have lowest priority.
+    if (a->IsEmpty() || b->IsEmpty())
+      return b->IsEmpty() < a->IsEmpty();
 
     WhichTree a_tree = a->NextTileIteratorTree(tree_priority_);
     const PictureLayerImpl::LayerEvictionTileIterator* a_iterator =
@@ -39,8 +39,6 @@ class EvictionOrderComparator {
         b_tile->priority_for_tree_priority(tree_priority_);
     bool prioritize_low_res = tree_priority_ == SMOOTHNESS_TAKES_PRIORITY;
 
-    // Now we have to return true iff b is lower priority than a.
-
     // If the priority bin differs, b is lower priority if it has the higher
     // priority bin.
     if (a_priority.priority_bin != b_priority.priority_bin)
@@ -60,7 +58,6 @@ class EvictionOrderComparator {
 
       if (prioritize_low_res)
         return a_priority.resolution == LOW_RESOLUTION;
-
       return a_priority.resolution == HIGH_RESOLUTION;
     }
 

cc/resources/raster_tile_priority_queue.cc

@@ -16,11 +16,11 @@ class RasterOrderComparator {
   bool operator()(
       const RasterTilePriorityQueue::PairedPictureLayerQueue* a,
       const RasterTilePriorityQueue::PairedPictureLayerQueue* b) const {
-    if (a->IsEmpty())
-      return true;
-
-    if (b->IsEmpty())
-      return false;
+    // Note that in this function, we have to return true if and only if
+    // b is strictly lower priority than a. Note that for the sake of
+    // completeness, empty queue is considered to have lowest priority.
+    if (a->IsEmpty() || b->IsEmpty())
+      return b->IsEmpty() < a->IsEmpty();
 
     WhichTree a_tree = a->NextTileIteratorTree(tree_priority_);
     const PictureLayerImpl::LayerRasterTileIterator* a_iterator =
@@ -39,8 +39,6 @@ class RasterOrderComparator {
         b_tile->priority_for_tree_priority(tree_priority_);
     bool prioritize_low_res = tree_priority_ == SMOOTHNESS_TAKES_PRIORITY;
 
-    // Now we have to return true iff b is higher priority than a.
-
     // If the bin is the same but the resolution is not, then the order will be
     // determined by whether we prioritize low res or not.
     // TODO(vmpstr): Remove this when TilePriority is no longer a member of Tile
@@ -56,10 +54,8 @@ class RasterOrderComparator {
 
       if (prioritize_low_res)
         return b_priority.resolution == LOW_RESOLUTION;
-
       return b_priority.resolution == HIGH_RESOLUTION;
     }
-
     return b_priority.IsHigherPriorityThan(a_priority);
   }
 
@@ -86,7 +82,6 @@ void RasterTilePriorityQueue::Build(
     paired_queues_.push_back(
         make_scoped_ptr(new PairedPictureLayerQueue(*it, tree_priority_)));
   }
-
   paired_queues_.make_heap(RasterOrderComparator(tree_priority_));
 }
 

cc/resources/tile_manager_unittest.cc

@@ -1105,5 +1105,102 @@ TEST_F(TileManagerTilePriorityQueueTest,
       pending_child_high_res_tiles.size() + pending_child_low_res_tiles.size();
   EXPECT_EQ(expected_occluded_count, occluded_count);
 }
+
+TEST_F(TileManagerTilePriorityQueueTest, RasterTilePriorityQueueEmptyLayers) {
+  SetupDefaultTrees(gfx::Size(1000, 1000));
+
+  active_layer_->CreateDefaultTilingsAndTiles();
+  pending_layer_->CreateDefaultTilingsAndTiles();
+
+  RasterTilePriorityQueue queue;
+  host_impl_.BuildRasterQueue(&queue, SAME_PRIORITY_FOR_BOTH_TREES);
+  EXPECT_FALSE(queue.IsEmpty());
+
+  size_t tile_count = 0;
+  std::set<Tile*> all_tiles;
+  while (!queue.IsEmpty()) {
+    EXPECT_TRUE(queue.Top());
+    all_tiles.insert(queue.Top());
+    ++tile_count;
+    queue.Pop();
+  }
+
+  EXPECT_EQ(tile_count, all_tiles.size());
+  EXPECT_EQ(17u, tile_count);
+
+  queue.Reset();
+  for (int i = 1; i < 10; ++i) {
+    scoped_ptr<FakePictureLayerImpl> pending_layer =
+        FakePictureLayerImpl::Create(host_impl_.pending_tree(), id_ + i);
+    pending_layer->SetDrawsContent(true);
+    pending_layer->DoPostCommitInitializationIfNeeded();
+    pending_layer->set_has_valid_tile_priorities(true);
+    pending_layer_->AddChild(pending_layer.PassAs<LayerImpl>());
+  }
+
+  host_impl_.BuildRasterQueue(&queue, SAME_PRIORITY_FOR_BOTH_TREES);
+  EXPECT_FALSE(queue.IsEmpty());
+
+  tile_count = 0;
+  all_tiles.clear();
+  while (!queue.IsEmpty()) {
+    EXPECT_TRUE(queue.Top());
+    all_tiles.insert(queue.Top());
+    ++tile_count;
+    queue.Pop();
+  }
+  EXPECT_EQ(tile_count, all_tiles.size());
+  EXPECT_EQ(17u, tile_count);
+}
+
+TEST_F(TileManagerTilePriorityQueueTest, EvictionTilePriorityQueueEmptyLayers) {
+  SetupDefaultTrees(gfx::Size(1000, 1000));
+
+  active_layer_->CreateDefaultTilingsAndTiles();
+  pending_layer_->CreateDefaultTilingsAndTiles();
+
+  RasterTilePriorityQueue raster_queue;
+  host_impl_.BuildRasterQueue(&raster_queue, SAME_PRIORITY_FOR_BOTH_TREES);
+  EXPECT_FALSE(raster_queue.IsEmpty());
+
+  size_t tile_count = 0;
+  std::set<Tile*> all_tiles;
+  while (!raster_queue.IsEmpty()) {
+    EXPECT_TRUE(raster_queue.Top());
+    all_tiles.insert(raster_queue.Top());
+    ++tile_count;
+    raster_queue.Pop();
+  }
+  EXPECT_EQ(tile_count, all_tiles.size());
+  EXPECT_EQ(17u, tile_count);
+
+  std::vector<Tile*> tiles(all_tiles.begin(), all_tiles.end());
+  host_impl_.tile_manager()->InitializeTilesWithResourcesForTesting(tiles);
+
+  EvictionTilePriorityQueue queue;
+  for (int i = 1; i < 10; ++i) {
+    scoped_ptr<FakePictureLayerImpl> pending_layer =
+        FakePictureLayerImpl::Create(host_impl_.pending_tree(), id_ + i);
+    pending_layer->SetDrawsContent(true);
+    pending_layer->DoPostCommitInitializationIfNeeded();
+    pending_layer->set_has_valid_tile_priorities(true);
+    pending_layer_->AddChild(pending_layer.PassAs<LayerImpl>());
+  }
+
+  host_impl_.BuildEvictionQueue(&queue, SAME_PRIORITY_FOR_BOTH_TREES);
+  EXPECT_FALSE(queue.IsEmpty());
+
+  tile_count = 0;
+  all_tiles.clear();
+  while (!queue.IsEmpty()) {
+    EXPECT_TRUE(queue.Top());
+    all_tiles.insert(queue.Top());
+    ++tile_count;
+    queue.Pop();
+  }
+  EXPECT_EQ(tile_count, all_tiles.size());
+  EXPECT_EQ(17u, tile_count);
+}
+
 }  // namespace
 }  // namespace cc

cc/test/fake_picture_layer_impl.cc

@@ -15,7 +15,9 @@ FakePictureLayerImpl::FakePictureLayerImpl(LayerTreeImpl* tree_impl,
                                            scoped_refptr<PicturePileImpl> pile)
     : PictureLayerImpl(tree_impl, id),
       append_quads_count_(0),
-      did_become_active_call_count_(0) {
+      did_become_active_call_count_(0),
+      has_valid_tile_priorities_(false),
+      use_set_valid_tile_priorities_flag_(false) {
   pile_ = pile;
   SetBounds(pile_->tiling_size());
   SetContentBounds(pile_->tiling_size());
@@ -27,7 +29,9 @@ FakePictureLayerImpl::FakePictureLayerImpl(LayerTreeImpl* tree_impl,
                                            const gfx::Size& layer_bounds)
     : PictureLayerImpl(tree_impl, id),
       append_quads_count_(0),
-      did_become_active_call_count_(0) {
+      did_become_active_call_count_(0),
+      has_valid_tile_priorities_(false),
+      use_set_valid_tile_priorities_flag_(false) {
   pile_ = pile;
   SetBounds(layer_bounds);
   SetContentBounds(layer_bounds);
@@ -36,7 +40,9 @@ FakePictureLayerImpl::FakePictureLayerImpl(LayerTreeImpl* tree_impl,
 FakePictureLayerImpl::FakePictureLayerImpl(LayerTreeImpl* tree_impl, int id)
     : PictureLayerImpl(tree_impl, id),
       append_quads_count_(0),
-      did_become_active_call_count_(0) {
+      did_become_active_call_count_(0),
+      has_valid_tile_priorities_(false),
+      use_set_valid_tile_priorities_flag_(false) {
 }
 
 scoped_ptr<LayerImpl> FakePictureLayerImpl::CreateLayerImpl(
@@ -163,4 +169,10 @@ void FakePictureLayerImpl::DidBecomeActive() {
   ++did_become_active_call_count_;
 }
 
+bool FakePictureLayerImpl::HasValidTilePriorities() const {
+  return use_set_valid_tile_priorities_flag_
+             ? has_valid_tile_priorities_
+             : PictureLayerImpl::HasValidTilePriorities();
+}
+
 }  // namespace cc

cc/test/fake_picture_layer_impl.h

@@ -46,6 +46,12 @@ class FakePictureLayerImpl : public PictureLayerImpl {
     return did_become_active_call_count_;
   }
 
+  virtual bool HasValidTilePriorities() const OVERRIDE;
+  void set_has_valid_tile_priorities(bool has_valid_priorities) {
+    has_valid_tile_priorities_ = has_valid_priorities;
+    use_set_valid_tile_priorities_flag_ = true;
+  }
+
   using PictureLayerImpl::AddTiling;
   using PictureLayerImpl::CleanUpTilingsOnActiveLayer;
   using PictureLayerImpl::CanHaveTilings;
@@ -116,6 +122,8 @@ class FakePictureLayerImpl : public PictureLayerImpl {
 
   size_t append_quads_count_;
   size_t did_become_active_call_count_;
+  bool has_valid_tile_priorities_;
+  bool use_set_valid_tile_priorities_flag_;
 };
 
 }  // namespace cc