From 570a5436c41a7a39f4b4a32cf68530d51a789129 Mon Sep 17 00:00:00 2001
From: Ting-Yu Lin <tlin@mozilla.com>
Date: Mon, 22 Nov 2021 03:26:19 +0000
Subject: [PATCH] Bug 1742006 - Fix documents of rect types' Union and
 SaturatingUnion, and add a test. r=gfx-reviewers,mstange

The documents of Union and SaturatingUnion promise that |this| will be return
when both rects are empty, but the current implementation returns aRect instead.

This patch fixes the documents and added a test case.

Differential Revision: https://phabricator.services.mozilla.com/D131568
---
 gfx/2d/BaseRect.h            |  5 ++---
 gfx/2d/RectAbsolute.h        |  5 ++---
 gfx/src/nsRect.h             |  3 +++
 gfx/tests/gtest/TestRect.cpp | 22 ++++++++++++++++++++++
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/gfx/2d/BaseRect.h b/gfx/2d/BaseRect.h
index 8d0a3f1db1c7..6d8fdaa0a6f7 100644
--- a/gfx/2d/BaseRect.h
+++ b/gfx/2d/BaseRect.h
@@ -174,9 +174,8 @@ struct BaseRect {
   }
 
   // Returns the smallest rectangle that contains both the area of both
-  // this and aRect2.
-  // Thus, empty input rectangles are ignored.
-  // If both rectangles are empty, returns this.
+  // this and aRect. Thus, empty input rectangles are ignored.
+  // Note: if both rectangles are empty, returns aRect.
   // WARNING! This is not safe against overflow, prefer using SafeUnion instead
   // when dealing with int-based rects.
   [[nodiscard]] Sub Union(const Sub& aRect) const {
diff --git a/gfx/2d/RectAbsolute.h b/gfx/2d/RectAbsolute.h
index 68ac88a2af23..09da87c80ffc 100644
--- a/gfx/2d/RectAbsolute.h
+++ b/gfx/2d/RectAbsolute.h
@@ -142,9 +142,8 @@ struct BaseRectAbsolute {
   void SetEmpty() { left = right = top = bottom = 0; }
 
   // Returns the smallest rectangle that contains both the area of both
-  // this and aRect2.
-  // Thus, empty input rectangles are ignored.
-  // If both rectangles are empty, returns this.
+  // this and aRect. Thus, empty input rectangles are ignored.
+  // Note: if both rectangles are empty, returns aRect.
   // WARNING! This is not safe against overflow, prefer using SafeUnion instead
   // when dealing with int-based rects.
   [[nodiscard]] Sub Union(const Sub& aRect) const {
diff --git a/gfx/src/nsRect.h b/gfx/src/nsRect.h
index 606019e0b3c9..c901aee72cdf 100644
--- a/gfx/src/nsRect.h
+++ b/gfx/src/nsRect.h
@@ -54,6 +54,9 @@ struct nsRect : public mozilla::gfx::BaseRect<nscoord, nsRect, nsPoint, nsSize,
   // overflowing nscoord values in the 'width' and 'height' fields by
   // clamping the width and height values to nscoord_MAX if necessary.
 
+  // Returns the smallest rectangle that contains both the area of both
+  // this and aRect. Thus, empty input rectangles are ignored.
+  // Note: if both rectangles are empty, returns aRect.
   [[nodiscard]] nsRect SaturatingUnion(const nsRect& aRect) const {
     if (IsEmpty()) {
       return aRect;
diff --git a/gfx/tests/gtest/TestRect.cpp b/gfx/tests/gtest/TestRect.cpp
index 6c3d10288489..886f5cb031b9 100644
--- a/gfx/tests/gtest/TestRect.cpp
+++ b/gfx/tests/gtest/TestRect.cpp
@@ -9,6 +9,7 @@
 
 #include "gfxTypes.h"
 #include "nsRect.h"
+#include "nsRectAbsolute.h"
 #include "gfxRect.h"
 #include "mozilla/gfx/Rect.h"
 #include "mozilla/gfx/RectAbsolute.h"
@@ -435,6 +436,18 @@ static bool TestUnion() {
   return true;
 }
 
+template <class RectType>
+static void TestUnionEmptyRects() {
+  RectType rect1(10, 10, 0, 50);
+  RectType rect2(5, 5, 40, 0);
+  EXPECT_TRUE(rect1.IsEmpty() && rect2.IsEmpty());
+
+  RectType dest = rect1.Union(rect2);
+  EXPECT_TRUE(dest.IsEmpty() && dest.IsEqualEdges(rect2))
+      << "Test the case where both rects are empty, and the result is the "
+         "same value passing into Union()";
+}
+
 static bool TestFiniteGfx() {
   float posInf = std::numeric_limits<float>::infinity();
   float negInf = -std::numeric_limits<float>::infinity();
@@ -578,6 +591,7 @@ TEST(Gfx, nsRect)
   TestIntersects<nsRect>();
   TestIntersection<nsRect>();
   TestUnion<nsRect>();
+  TestUnionEmptyRects<nsRect>();
   TestBug1135677<nsRect>();
   TestSetWH<nsRect>();
   TestSwap<nsRect>();
@@ -591,6 +605,7 @@ TEST(Gfx, nsIntRect)
   TestIntersects<nsIntRect>();
   TestIntersection<nsIntRect>();
   TestUnion<nsIntRect>();
+  TestUnionEmptyRects<nsIntRect>();
   TestBug1135677<nsIntRect>();
   TestSetWH<nsIntRect>();
   TestSwap<nsIntRect>();
@@ -604,12 +619,19 @@ TEST(Gfx, gfxRect)
   TestIntersects<gfxRect>();
   TestIntersection<gfxRect>();
   TestUnion<gfxRect>();
+  TestUnionEmptyRects<gfxRect>();
   TestBug1135677<gfxRect>();
   TestFiniteGfx();
   TestSetWH<gfxRect>();
   TestSwap<gfxRect>();
 }
 
+TEST(Gfx, nsRectAbsolute)
+{ TestUnionEmptyRects<nsRectAbsolute>(); }
+
+TEST(Gfx, IntRectAbsolute)
+{ TestUnionEmptyRects<IntRectAbsolute>(); }
+
 static void TestMoveInsideAndClamp(IntRect aSrc, IntRect aTarget,
                                    IntRect aExpected) {
   // Test the implementation in BaseRect (x/y/width/height representation)