Add utility methods to SurfaceRange & Fix SurfaceRange::IsInRangeExclusive

This CL fixes SurfaceRange::IsInRangeExclusive in the case when the embed
tokens are different by comparing with either ends of the range.
In addition, it adds 2 extra utility functions to SurfaceRange which are needed
to make the code cleaner for following work.

Bug: 867531
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: Ia542e94b13c566989f3764c32cad4016bfddd32b
Reviewed-on: https://chromium-review.googlesource.com/1159100
Commit-Queue: Andre Kaba <akaba@google.com>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580313}

components/viz/common/surfaces/surface_range.cc

@@ -34,9 +34,25 @@ bool SurfaceRange::operator<(const SurfaceRange& other) const {
 bool SurfaceRange::IsInRangeExclusive(const SurfaceId& surface_id) const {
   if (!start_)
     return end_.IsNewerThan(surface_id);
+
+  if (HasDifferentFrameSinkIds() ||
+      end_.local_surface_id().embed_token() !=
+          start_->local_surface_id().embed_token()) {
+    return surface_id.IsNewerThan(*start_) || end_.IsNewerThan(surface_id);
+  }
+
   return surface_id.IsNewerThan(*start_) && end_.IsNewerThan(surface_id);
 }
 
+bool SurfaceRange::IsInRangeInclusive(const SurfaceId& surface_id) const {
+  return IsInRangeExclusive(surface_id) || end_ == surface_id ||
+         start_ == surface_id;
+}
+
+bool SurfaceRange::HasDifferentFrameSinkIds() const {
+  return start_ && start_->frame_sink_id() != end_.frame_sink_id();
+}
+
 bool SurfaceRange::IsValid() const {
   if (!end_.is_valid())
     return false;

components/viz/common/surfaces/surface_range.h

@@ -35,10 +35,19 @@ class VIZ_COMMON_EXPORT SurfaceRange {
 
   bool operator<(const SurfaceRange& other) const;
 
-  // Check if |surface_id| is in the surface range and match the frame sink id
-  // of start and end.
+  // Check if |surface_id| falls within |this| SurfaceRange but is neither the
+  // start nor end of the range. The FrameSinkId of |surface_id| must match
+  // either the start or end of the range.
   bool IsInRangeExclusive(const SurfaceId& surface_id) const;
 
+  // Check if |surface_id| falls within |this| SurfaceRange inclusive of the
+  // start and end of the range. The FrameSinkId of |surface_id| must match
+  // either the start or end of the range.
+  bool IsInRangeInclusive(const SurfaceId& surface_id) const;
+
+  // Returns whether the start and end of the range have differing FrameSinkIds.
+  bool HasDifferentFrameSinkIds() const;
+
   bool IsValid() const;
 
   const base::Optional<SurfaceId>& start() const { return start_; }

components/viz/common/surfaces/surface_range_unittest.cc

@@ -8,8 +8,9 @@
 #include "base/logging.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-// Verifying that SurfaceId::IsInRangeExclusive works properly.
-TEST(SurfaceRangeTest, VerifyInRange) {
+// Verifies that SurfaceId::IsInRangeExclusive and SurfaceId::IsInRangeInclusive
+// works properly.
+TEST(SurfaceRangeTest, InRangeTest) {
   viz::FrameSinkId FrameSink1(65564, 0);
   viz::FrameSinkId FrameSink2(65565, 0);
   const base::UnguessableToken token1 = base::UnguessableToken::Create();
@@ -17,9 +18,12 @@ TEST(SurfaceRangeTest, VerifyInRange) {
 
   const viz::SurfaceId start(FrameSink1, viz::LocalSurfaceId(1, 1, token1));
   const viz::SurfaceId end(FrameSink1, viz::LocalSurfaceId(2, 2, token1));
+  const viz::SurfaceId end_token2(FrameSink2,
+                                  viz::LocalSurfaceId(2, 2, token2));
 
   const viz::SurfaceRange surface_range1(start, end);
   const viz::SurfaceRange surface_range2(base::nullopt, end);
+  const viz::SurfaceRange surface_range1_token2(start, end_token2);
 
   const viz::SurfaceId surface_id1(FrameSink1,
                                    viz::LocalSurfaceId(1, 2, token1));
@@ -31,22 +35,35 @@ TEST(SurfaceRangeTest, VerifyInRange) {
                                    viz::LocalSurfaceId(3, 3, token1));
 
   // |surface_id1| has the right embed token and is inside the range
-  // (Start,end).
+  // (start,end).
   EXPECT_TRUE(surface_range1.IsInRangeExclusive(surface_id1));
 
   // |surface_id1| has the right embed token and inside the range
   // (base::nullopt,end).
   EXPECT_TRUE(surface_range2.IsInRangeExclusive(surface_id1));
 
-  // |surface_id2| has the wrong embed token/FrameSinkId so should be refused.
+  // |surface_id2| has an unmatching token.
   EXPECT_FALSE(surface_range1.IsInRangeExclusive(surface_id2));
   EXPECT_FALSE(surface_range2.IsInRangeExclusive(surface_id2));
 
-  // |surface_id3| is not included in the range exclusively.
+  // |surface_id2| doesn't match any end point either.
+  EXPECT_FALSE(surface_range1.IsInRangeInclusive(surface_id2));
+  EXPECT_FALSE(surface_range2.IsInRangeInclusive(surface_id2));
+
+  // |surface_id2| has a matching token to |end_token2|.
+  EXPECT_TRUE(surface_range1_token2.IsInRangeExclusive(surface_id2));
+
+  // |surface_id3| is not included when end points are not considered.
   EXPECT_FALSE(surface_range1.IsInRangeExclusive(surface_id3));
   EXPECT_FALSE(surface_range2.IsInRangeExclusive(surface_id3));
 
-  // |surface_id4| is not included in the range.
+  // But |surface_id3| is included when end points are considered.
+  EXPECT_TRUE(surface_range1.IsInRangeInclusive(surface_id3));
+  EXPECT_TRUE(surface_range2.IsInRangeInclusive(surface_id3));
+
+  // |surface_id4| is not included in the range in all cases.
   EXPECT_FALSE(surface_range1.IsInRangeExclusive(surface_id4));
   EXPECT_FALSE(surface_range2.IsInRangeExclusive(surface_id4));
+  EXPECT_FALSE(surface_range1.IsInRangeInclusive(surface_id4));
+  EXPECT_FALSE(surface_range2.IsInRangeInclusive(surface_id4));
 }