Handle overflow in cc RTree

Updates the cc RTree class to handle overflow:
  - GetBounds becomes GetBoundsOrDie and can only be called when
    has_valid_bounds().
  - All other functions are unchanged, operating correctly, but less
    efficiently, when !has_valid_bounds().

Updates DisplayItemList to handle cases when !has_valid_bounds().

Bug: 988590
Change-Id: I8612c5e219a7aa9774fc24cb69372df975562bbc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1777049
Commit-Queue: Eric Karl <ericrk@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692247}

cc/base/rtree.h

@@ -63,6 +63,11 @@ class RTree {
              const BoundsFunctor& bounds_getter,
              const PayloadFunctor& payload_getter);
 
+  // If false, this rtree does not have valid bounds and:
+  //  - GetBoundsOrDie will CHECK.
+  //  - Search* will have degraded performance.
+  bool has_valid_bounds() const { return has_valid_bounds_; }
+
   // Given a query rect, returns elements that intersect the rect. Elements are
   // returned in the order they appeared in the initial container.
   void Search(const gfx::Rect& query, std::vector<T>* results) const;
@@ -73,7 +78,8 @@ class RTree {
   void SearchRefs(const gfx::Rect& query, std::vector<const T*>* results) const;
 
   // Returns the total bounds of all items in this rtree.
-  gfx::Rect GetBounds() const;
+  // if !has_valid_bounds() this function will CHECK.
+  gfx::Rect GetBoundsOrDie() const;
 
   // Returns respective bounds of all items in this rtree in the order of items.
   // Production code except tracing should not use this method.
@@ -122,6 +128,15 @@ class RTree {
                            const gfx::Rect& query,
                            std::vector<const T*>* results) const;
 
+  // The following two functions are slow fallback versions of SearchRecursive
+  // and SearchRefsRecursive for when !has_valid_bounds().
+  void SearchRecursiveFallback(Node<T>* root,
+                               const gfx::Rect& query,
+                               std::vector<T>* results) const;
+  void SearchRefsRecursiveFallback(Node<T>* root,
+                                   const gfx::Rect& query,
+                                   std::vector<const T*>* results) const;
+
   // Consumes the input array.
   Branch<T> BuildRecursive(std::vector<Branch<T>>* branches, int level);
   Node<T>* AllocateNodeAtLevel(int level);
@@ -133,6 +148,9 @@ class RTree {
   size_t num_data_elements_ = 0u;
   Branch<T> root_;
   std::vector<Node<T>> nodes_;
+
+  // If false, the rtree encountered overflow does not have reliable bounds.
+  bool has_valid_bounds_ = true;
 };
 
 template <typename T>
@@ -275,6 +293,11 @@ auto RTree<T>::BuildRecursive(std::vector<Branch<T>>* branches, int level)
     branch.bounds.SetRect(x, y, base::ClampSub(right, x),
                           base::ClampSub(bottom, y));
 
+    // If we had to clamp right/bottom values, we've overflowed.
+    bool overflow =
+        branch.bounds.right() != right || branch.bounds.bottom() != bottom;
+    has_valid_bounds_ &= !overflow;
+
     DCHECK_LT(new_branch_index, current_branch);
     (*branches)[new_branch_index] = std::move(branch);
     ++new_branch_index;
@@ -286,16 +309,26 @@ auto RTree<T>::BuildRecursive(std::vector<Branch<T>>* branches, int level)
 template <typename T>
 void RTree<T>::Search(const gfx::Rect& query, std::vector<T>* results) const {
   results->clear();
-  if (num_data_elements_ > 0 && query.Intersects(root_.bounds))
+  if (num_data_elements_ == 0)
+    return;
+  if (!has_valid_bounds_) {
+    SearchRecursiveFallback(root_.subtree, query, results);
+  } else if (query.Intersects(root_.bounds)) {
     SearchRecursive(root_.subtree, query, results);
+  }
 }
 
 template <typename T>
 void RTree<T>::SearchRefs(const gfx::Rect& query,
                           std::vector<const T*>* results) const {
   results->clear();
-  if (num_data_elements_ > 0 && query.Intersects(root_.bounds))
+  if (num_data_elements_ == 0)
+    return;
+  if (!has_valid_bounds_) {
+    SearchRefsRecursiveFallback(root_.subtree, query, results);
+  } else if (query.Intersects(root_.bounds)) {
     SearchRefsRecursive(root_.subtree, query, results);
+  }
 }
 
 template <typename T>
@@ -326,8 +359,40 @@ void RTree<T>::SearchRefsRecursive(Node<T>* node,
   }
 }
 
+// When !has_valid_bounds(), any non-leaf bounds may have overflowed and be
+// invalid. Iterate over the entire tree, checking bounds at each leaf.
+template <typename T>
+void RTree<T>::SearchRecursiveFallback(Node<T>* node,
+                                       const gfx::Rect& query,
+                                       std::vector<T>* results) const {
+  for (uint16_t i = 0; i < node->num_children; ++i) {
+    if (node->level == 0) {
+      if (query.Intersects(node->children[i].bounds))
+        results->push_back(node->children[i].payload);
+    } else {
+      SearchRecursive(node->children[i].subtree, query, results);
+    }
+  }
+}
+
+template <typename T>
+void RTree<T>::SearchRefsRecursiveFallback(
+    Node<T>* node,
+    const gfx::Rect& query,
+    std::vector<const T*>* results) const {
+  for (uint16_t i = 0; i < node->num_children; ++i) {
+    if (node->level == 0) {
+      if (query.Intersects(node->children[i].bounds))
+        results->push_back(&node->children[i].payload);
+    } else {
+      SearchRefsRecursive(node->children[i].subtree, query, results);
+    }
+  }
+}
+
 template <typename T>
-gfx::Rect RTree<T>::GetBounds() const {
+gfx::Rect RTree<T>::GetBoundsOrDie() const {
+  CHECK(has_valid_bounds_);
   return root_.bounds;
 }
 
@@ -355,6 +420,7 @@ void RTree<T>::Reset() {
   num_data_elements_ = 0;
   nodes_.clear();
   root_.bounds = gfx::Rect();
+  has_valid_bounds_ = true;
 }
 
 }  // namespace cc

cc/base/rtree_unittest.cc

@@ -9,6 +9,24 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace cc {
+namespace {
+// Helper function to use in place of rtree. Search that ensures that every
+// call to Search / SearchRefs produces the same results.
+template <typename T>
+void SearchAndVerifyRefs(const RTree<T>& rtree,
+                         const gfx::Rect& query,
+                         std::vector<T>* results) {
+  rtree.Search(query, results);
+
+  // Perform the same query with SearchRefs and make sure it matches Search.
+  std::vector<const T*> ref_results;
+  rtree.SearchRefs(query, &ref_results);
+  ASSERT_EQ(ref_results.size(), results->size());
+  for (size_t i = 0; i < results->size(); ++i) {
+    EXPECT_EQ(*ref_results[i], (*results)[i]);
+  }
+}
+}  // namespace
 
 TEST(RTreeTest, ReserveNodesDoesntDcheck) {
   // Make sure that anywhere between 0 and 1000 rects, our reserve math in rtree
@@ -36,20 +54,20 @@ TEST(RTreeTest, NoOverlap) {
   rtree.Build(rects);
 
   std::vector<size_t> results;
-  rtree.Search(gfx::Rect(0, 0, 50, 50), &results);
+  SearchAndVerifyRefs(rtree, gfx::Rect(0, 0, 50, 50), &results);
   ASSERT_EQ(2500u, results.size());
   // Note that the results have to be sorted.
   for (size_t i = 0; i < 2500; ++i) {
     ASSERT_EQ(results[i], i);
   }
 
-  rtree.Search(gfx::Rect(0, 0, 50, 49), &results);
+  SearchAndVerifyRefs(rtree, gfx::Rect(0, 0, 50, 49), &results);
   ASSERT_EQ(2450u, results.size());
   for (size_t i = 0; i < 2450; ++i) {
     ASSERT_EQ(results[i], i);
   }
 
-  rtree.Search(gfx::Rect(5, 6, 1, 1), &results);
+  SearchAndVerifyRefs(rtree, gfx::Rect(5, 6, 1, 1), &results);
   ASSERT_EQ(1u, results.size());
   EXPECT_EQ(6u * 50 + 5u, results[0]);
 }
@@ -66,14 +84,14 @@ TEST(RTreeTest, Overlap) {
   rtree.Build(rects);
 
   std::vector<size_t> results;
-  rtree.Search(gfx::Rect(0, 0, 1, 1), &results);
+  SearchAndVerifyRefs(rtree, gfx::Rect(0, 0, 1, 1), &results);
   ASSERT_EQ(2500u, results.size());
   // Both the checks for the elements assume elements are sorted.
   for (size_t i = 0; i < 2500; ++i) {
     ASSERT_EQ(results[i], i);
   }
 
-  rtree.Search(gfx::Rect(0, 49, 1, 1), &results);
+  SearchAndVerifyRefs(rtree, gfx::Rect(0, 49, 1, 1), &results);
   ASSERT_EQ(50u, results.size());
   for (size_t i = 0; i < 50; ++i) {
     EXPECT_EQ(results[i], 2450u + i);
@@ -103,11 +121,11 @@ TEST(RTreeTest, SortedResults) {
   for (int y = 0; y < 50; ++y) {
     for (int x = 0; x < 50; ++x) {
       std::vector<size_t> results;
-      rtree.Search(gfx::Rect(x, y, 1, 1), &results);
+      SearchAndVerifyRefs(rtree, gfx::Rect(x, y, 1, 1), &results);
       VerifySorted(results);
-      rtree.Search(gfx::Rect(x, y, 50, 1), &results);
+      SearchAndVerifyRefs(rtree, gfx::Rect(x, y, 50, 1), &results);
       VerifySorted(results);
-      rtree.Search(gfx::Rect(x, y, 1, 50), &results);
+      SearchAndVerifyRefs(rtree, gfx::Rect(x, y, 1, 50), &results);
       VerifySorted(results);
     }
   }
@@ -115,7 +133,7 @@ TEST(RTreeTest, SortedResults) {
 
 TEST(RTreeTest, GetBoundsEmpty) {
   RTree<size_t> rtree;
-  EXPECT_EQ(gfx::Rect(), rtree.GetBounds());
+  EXPECT_EQ(gfx::Rect(), rtree.GetBoundsOrDie());
   EXPECT_TRUE(rtree.GetAllBoundsForTracing().empty());
 }
 
@@ -127,7 +145,7 @@ TEST(RTreeTest, GetBoundsNonOverlapping) {
   RTree<size_t> rtree;
   rtree.Build(rects);
 
-  EXPECT_EQ(gfx::Rect(5, 6, 19, 20), rtree.GetBounds());
+  EXPECT_EQ(gfx::Rect(5, 6, 19, 20), rtree.GetBoundsOrDie());
   std::map<size_t, gfx::Rect> expected_all_bounds = {{0, rects[0]},
                                                      {1, rects[1]}};
   EXPECT_EQ(expected_all_bounds, rtree.GetAllBoundsForTracing());
@@ -141,7 +159,7 @@ TEST(RTreeTest, GetBoundsOverlapping) {
   RTree<size_t> rtree;
   rtree.Build(rects);
 
-  EXPECT_EQ(gfx::Rect(0, 0, 10, 10), rtree.GetBounds());
+  EXPECT_EQ(gfx::Rect(0, 0, 10, 10), rtree.GetBoundsOrDie());
   std::map<size_t, gfx::Rect> expected_all_bounds = {{0, rects[0]},
                                                      {1, rects[1]}};
   EXPECT_EQ(expected_all_bounds, rtree.GetAllBoundsForTracing());
@@ -155,7 +173,7 @@ TEST(RTreeTest, GetBoundsWithEmptyRect) {
   RTree<size_t> rtree;
   rtree.Build(rects);
 
-  EXPECT_EQ(gfx::Rect(5, 5, 5, 5), rtree.GetBounds());
+  EXPECT_EQ(gfx::Rect(5, 5, 5, 5), rtree.GetBoundsOrDie());
   std::map<size_t, gfx::Rect> expected_all_bounds = {{1, rects[1]}};
   EXPECT_EQ(expected_all_bounds, rtree.GetAllBoundsForTracing());
 }
@@ -172,12 +190,12 @@ TEST(RTreeTest, BuildAfterReset) {
 
   // Resetting should give the same as an empty rtree.
   rtree.Reset();
-  EXPECT_EQ(gfx::Rect(), rtree.GetBounds());
+  EXPECT_EQ(gfx::Rect(), rtree.GetBoundsOrDie());
   EXPECT_TRUE(rtree.GetAllBoundsForTracing().empty());
 
   // Should be able to rebuild from a reset rtree.
   rtree.Build(rects);
-  EXPECT_EQ(gfx::Rect(0, 0, 10, 10), rtree.GetBounds());
+  EXPECT_EQ(gfx::Rect(0, 0, 10, 10), rtree.GetBoundsOrDie());
   std::map<size_t, gfx::Rect> expected_all_bounds = {
       {0, rects[0]}, {1, rects[1]}, {2, rects[2]}, {3, rects[3]}};
   EXPECT_EQ(expected_all_bounds, rtree.GetAllBoundsForTracing());
@@ -198,11 +216,11 @@ TEST(RTreeTest, Payload) {
       [](const Container& items, size_t index) { return items[index].second; });
 
   std::vector<float> results;
-  rtree.Search(gfx::Rect(0, 0, 1, 1), &results);
+  SearchAndVerifyRefs(rtree, gfx::Rect(0, 0, 1, 1), &results);
   ASSERT_EQ(1u, results.size());
   EXPECT_FLOAT_EQ(10.f, results[0]);
 
-  rtree.Search(gfx::Rect(5, 5, 10, 10), &results);
+  SearchAndVerifyRefs(rtree, gfx::Rect(5, 5, 10, 10), &results);
   ASSERT_EQ(4u, results.size());
   // Items returned should be in the order they were inserted.
   EXPECT_FLOAT_EQ(40.f, results[0]);
@@ -211,4 +229,78 @@ TEST(RTreeTest, Payload) {
   EXPECT_FLOAT_EQ(20.f, results[3]);
 }
 
+TEST(RTreeTest, InvalidBounds) {
+  std::vector<gfx::Rect> rects;
+  rects.push_back(gfx::Rect(-INT_MAX, -INT_MAX, INT_MAX, INT_MAX));
+  rects.push_back(gfx::Rect(100, 100, 10, 10));
+
+  RTree<size_t> rtree;
+  rtree.Build(rects);
+
+  EXPECT_FALSE(rtree.has_valid_bounds());
+}
+
+TEST(RTreeTest, InvalidBoundsReset) {
+  std::vector<gfx::Rect> rects;
+  rects.push_back(gfx::Rect(-INT_MAX, -INT_MAX, INT_MAX, INT_MAX));
+  rects.push_back(gfx::Rect(100, 100, 10, 10));
+
+  RTree<size_t> rtree;
+  rtree.Build(rects);
+
+  EXPECT_FALSE(rtree.has_valid_bounds());
+
+  // Reset() should restore us to an empty (but valid) state.
+  rtree.Reset();
+  ASSERT_TRUE(rtree.has_valid_bounds());
+  EXPECT_EQ(rtree.GetBoundsOrDie(), gfx::Rect());
+}
+
+TEST(RTreeTest, InvalidBoundsSearch) {
+  std::vector<gfx::Rect> rects;
+  rects.push_back(gfx::Rect(-INT_MAX, -INT_MAX, INT_MAX, INT_MAX));
+  rects.push_back(gfx::Rect(100, 100, 10, 10));
+  rects.push_back(gfx::Rect(105, 105, 10, 10));
+  rects.push_back(gfx::Rect(-50, -50, 10, 10));
+  rects.push_back(gfx::Rect(INT_MAX - 100, INT_MAX - 100, 10, 10));
+
+  RTree<size_t> rtree;
+  rtree.Build(rects);
+
+  EXPECT_FALSE(rtree.has_valid_bounds());
+
+  // Searching should still work.
+  std::vector<size_t> found;
+  SearchAndVerifyRefs(rtree, gfx::Rect(0, 0, INT_MAX, INT_MAX), &found);
+  EXPECT_EQ(found, std::vector<size_t>({1, 2, 4}));
+  SearchAndVerifyRefs(rtree, gfx::Rect(-INT_MAX, -INT_MAX, INT_MAX, INT_MAX),
+                      &found);
+  EXPECT_EQ(found, std::vector<size_t>({0, 3}));
+  SearchAndVerifyRefs(rtree, gfx::Rect(-50, -50, INT_MAX, INT_MAX), &found);
+  EXPECT_EQ(found, std::vector<size_t>({0, 1, 2, 3, 4}));
+}
+
+TEST(RTreeTest, InvalidBoundsGetAllBounds) {
+  std::vector<gfx::Rect> rects;
+  rects.push_back(gfx::Rect(-INT_MAX, -INT_MAX, INT_MAX, INT_MAX));
+  rects.push_back(gfx::Rect(100, 100, 10, 10));
+  rects.push_back(gfx::Rect(105, 105, 10, 10));
+  rects.push_back(gfx::Rect(-50, -50, 10, 10));
+  rects.push_back(gfx::Rect(INT_MAX - 100, INT_MAX - 100, 10, 10));
+
+  RTree<size_t> rtree;
+  rtree.Build(rects);
+
+  EXPECT_FALSE(rtree.has_valid_bounds());
+
+  // Getting all bounds should still work.
+  std::map<size_t, gfx::Rect> all_bounds = rtree.GetAllBoundsForTracing();
+  std::map<size_t, gfx::Rect> expected_all_bounds = {{0, rects[0]},
+                                                     {1, rects[1]},
+                                                     {2, rects[2]},
+                                                     {3, rects[3]},
+                                                     {4, rects[4]}};
+  EXPECT_EQ(all_bounds, expected_all_bounds);
+}
+
 }  // namespace cc

cc/paint/display_item_list.cc

@@ -144,6 +144,15 @@ DisplayItemList::CreateTracedValue(bool include_items) const {
   auto state = std::make_unique<base::trace_event::TracedValue>();
   state->BeginDictionary("params");
 
+  gfx::Rect bounds;
+  if (rtree_.has_valid_bounds()) {
+    bounds = rtree_.GetBoundsOrDie();
+  } else {
+    // For tracing code, just use the entire positive quadrant if the |rtree_|
+    // has invalid bounds.
+    bounds = gfx::Rect(INT_MAX, INT_MAX);
+  }
+
   if (include_items) {
     state->BeginArray("items");
 
@@ -159,8 +168,7 @@ DisplayItemList::CreateTracedValue(bool include_items) const {
           state.get());
 
       SkPictureRecorder recorder;
-      SkCanvas* canvas =
-          recorder.beginRecording(gfx::RectToSkRect(rtree_.GetBounds()));
+      SkCanvas* canvas = recorder.beginRecording(gfx::RectToSkRect(bounds));
       op->Raster(canvas, params);
       sk_sp<SkPicture> picture = recorder.finishRecordingAsPicture();
 
@@ -176,12 +184,11 @@ DisplayItemList::CreateTracedValue(bool include_items) const {
     state->EndArray();  // "items".
   }
 
-  MathUtil::AddToTracedValue("layer_rect", rtree_.GetBounds(), state.get());
+  MathUtil::AddToTracedValue("layer_rect", bounds, state.get());
   state->EndDictionary();  // "params".
 
   {
     SkPictureRecorder recorder;
-    gfx::Rect bounds = rtree_.GetBounds();
     SkCanvas* canvas = recorder.beginRecording(gfx::RectToSkRect(bounds));
     canvas->translate(-bounds.x(), -bounds.y());
     canvas->clipRect(gfx::RectToSkRect(bounds));
@@ -197,7 +204,16 @@ DisplayItemList::CreateTracedValue(bool include_items) const {
 
 void DisplayItemList::GenerateDiscardableImagesMetadata() {
   DCHECK(usage_hint_ == kTopLevelDisplayItemList);
-  image_map_.Generate(&paint_op_buffer_, rtree_.GetBounds());
+
+  gfx::Rect bounds;
+  if (rtree_.has_valid_bounds()) {
+    bounds = rtree_.GetBoundsOrDie();
+  } else {
+    // Bounds are only used to size an SkNoDrawCanvas, pass INT_MAX.
+    bounds = gfx::Rect(INT_MAX, INT_MAX);
+  }
+
+  image_map_.Generate(&paint_op_buffer_, bounds);
 }
 
 void DisplayItemList::Reset() {
@@ -231,7 +247,7 @@ bool DisplayItemList::GetColorIfSolidInRect(const gfx::Rect& rect,
   DCHECK(usage_hint_ == kTopLevelDisplayItemList);
   std::vector<size_t>* offsets_to_use = nullptr;
   std::vector<size_t> offsets;
-  if (!rect.Contains(rtree_.GetBounds())) {
+  if (rtree_.has_valid_bounds() && !rect.Contains(rtree_.GetBoundsOrDie())) {
     rtree_.Search(rect, &offsets);
     offsets_to_use = &offsets;
   }