Changed the return type of FPDFAnnot_Get{Rect|AttachmentPoints}()

Instead of returning structs, changed FPDFAnnot_GetRect() and
FPDFAnnot_GetAttachmentPoints() to return a bool and take a struct
as an out parameter.

Change-Id: I380e76eb1566b2488150fb31e9dad564a3ee10d4
Reviewed-on: https://pdfium-review.googlesource.com/10470
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Jane Liu <janeliulwq@google.com>

4 files changed, 94 insertions, 71 deletions

diff --git a/fpdfsdk/fpdfannot.cpp b/fpdfsdk/fpdfannot.cpp
index 9213877b0b..fee99c844b 100644
--- a/fpdfsdk/fpdfannot.cpp
+++ b/fpdfsdk/fpdfannot.cpp
@@ -590,15 +590,16 @@ FPDFAnnot_SetAttachmentPoints(FPDF_ANNOTATION annot,
   return true;
 }
 
-FPDF_EXPORT FS_QUADPOINTSF FPDF_CALLCONV
-FPDFAnnot_GetAttachmentPoints(FPDF_ANNOTATION annot) {
-  if (!annot || !FPDFAnnot_HasAttachmentPoints(annot))
-    return FS_QUADPOINTSF();
+FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
+FPDFAnnot_GetAttachmentPoints(FPDF_ANNOTATION annot,
+                              FS_QUADPOINTSF* quadPoints) {
+  if (!annot || !FPDFAnnot_HasAttachmentPoints(annot) || !quadPoints)
+    return false;
 
   CPDF_Dictionary* pAnnotDict =
       CPDFAnnotContextFromFPDFAnnotation(annot)->GetAnnotDict();
   if (!pAnnotDict)
-    return FS_QUADPOINTSF();
+    return false;
 
   // If the annotation's appearance stream is defined, then retrieve the
   // quadpoints defined by the "BBox" entry in the AP dictionary, since its
@@ -606,41 +607,40 @@ FPDFAnnot_GetAttachmentPoints(FPDF_ANNOTATION annot) {
   // takes priority over "QuadPoints" when rendering. Otherwise, retrieve
   // the "Quadpoints" entry from the annotation dictionary.
   CPDF_Array* pArray;
-  FS_QUADPOINTSF quadPoints;
   CPDF_Stream* pStream =
       FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
   if (pStream) {
     pArray = pStream->GetDict()->GetArrayFor("BBox");
     if (!pArray)
-      return FS_QUADPOINTSF();
+      return false;
 
     // Convert the BBox array into quadpoint coordinates. BBox array follows the
     // order of a rectangle array: (left, bottom, right, up); and quadpoints
     // follows the following order: (top-left vertex, top-right vertex, bottom-
     // left vertex, bottom-right vertex).
-    quadPoints.x1 = pArray->GetNumberAt(0);
-    quadPoints.y1 = pArray->GetNumberAt(3);
-    quadPoints.x2 = pArray->GetNumberAt(2);
-    quadPoints.y2 = pArray->GetNumberAt(3);
-    quadPoints.x3 = pArray->GetNumberAt(0);
-    quadPoints.y3 = pArray->GetNumberAt(1);
-    quadPoints.x4 = pArray->GetNumberAt(2);
-    quadPoints.y4 = pArray->GetNumberAt(1);
+    quadPoints->x1 = pArray->GetNumberAt(0);
+    quadPoints->y1 = pArray->GetNumberAt(3);
+    quadPoints->x2 = pArray->GetNumberAt(2);
+    quadPoints->y2 = pArray->GetNumberAt(3);
+    quadPoints->x3 = pArray->GetNumberAt(0);
+    quadPoints->y3 = pArray->GetNumberAt(1);
+    quadPoints->x4 = pArray->GetNumberAt(2);
+    quadPoints->y4 = pArray->GetNumberAt(1);
   } else {
     pArray = pAnnotDict->GetArrayFor("QuadPoints");
     if (!pArray)
-      return FS_QUADPOINTSF();
-
-    quadPoints.x1 = pArray->GetNumberAt(0);
-    quadPoints.y1 = pArray->GetNumberAt(1);
-    quadPoints.x2 = pArray->GetNumberAt(2);
-    quadPoints.y2 = pArray->GetNumberAt(3);
-    quadPoints.x3 = pArray->GetNumberAt(4);
-    quadPoints.y3 = pArray->GetNumberAt(5);
-    quadPoints.x4 = pArray->GetNumberAt(6);
-    quadPoints.y4 = pArray->GetNumberAt(7);
+      return false;
+
+    quadPoints->x1 = pArray->GetNumberAt(0);
+    quadPoints->y1 = pArray->GetNumberAt(1);
+    quadPoints->x2 = pArray->GetNumberAt(2);
+    quadPoints->y2 = pArray->GetNumberAt(3);
+    quadPoints->x3 = pArray->GetNumberAt(4);
+    quadPoints->y3 = pArray->GetNumberAt(5);
+    quadPoints->x4 = pArray->GetNumberAt(6);
+    quadPoints->y4 = pArray->GetNumberAt(7);
   }
-  return quadPoints;
+  return true;
 }
 
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFAnnot_SetRect(FPDF_ANNOTATION annot,
@@ -673,14 +673,15 @@ FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFAnnot_SetRect(FPDF_ANNOTATION annot,
   return true;
 }
 
-FPDF_EXPORT FS_RECTF FPDF_CALLCONV FPDFAnnot_GetRect(FPDF_ANNOTATION annot) {
-  if (!annot)
-    return FS_RECTF();
+FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFAnnot_GetRect(FPDF_ANNOTATION annot,
+                                                      FS_RECTF* rect) {
+  if (!annot || !rect)
+    return false;
 
   CPDF_Dictionary* pAnnotDict =
       CPDFAnnotContextFromFPDFAnnotation(annot)->GetAnnotDict();
   if (!pAnnotDict)
-    return FS_RECTF();
+    return false;
 
   // If the annotation's appearance stream is defined and the annotation is of
   // a type that does not have quadpoints, then retrieve the rectangle defined
@@ -696,12 +697,11 @@ FPDF_EXPORT FS_RECTF FPDF_CALLCONV FPDFAnnot_GetRect(FPDF_ANNOTATION annot) {
   else
     rt = pStream->GetDict()->GetRectFor("BBox");
 
-  FS_RECTF rect;
-  rect.left = rt.left;
-  rect.bottom = rt.bottom;
-  rect.right = rt.right;
-  rect.top = rt.top;
-  return rect;
+  rect->left = rt.left;
+  rect->bottom = rt.bottom;
+  rect->right = rt.right;
+  rect->top = rt.top;
+  return true;
 }
 
 FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFAnnot_HasKey(FPDF_ANNOTATION annot,
diff --git a/fpdfsdk/fpdfannot_embeddertest.cpp b/fpdfsdk/fpdfannot_embeddertest.cpp
index 66260bd8ac..32edd7a503 100644
--- a/fpdfsdk/fpdfannot_embeddertest.cpp
+++ b/fpdfsdk/fpdfannot_embeddertest.cpp
@@ -108,7 +108,8 @@ TEST_F(FPDFAnnotEmbeddertest, ExtractHighlightLongContent) {
                    .c_str());
 
   // Check that the quadpoints are correct.
-  FS_QUADPOINTSF quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+  FS_QUADPOINTSF quadpoints;
+  ASSERT_TRUE(FPDFAnnot_GetAttachmentPoints(annot, &quadpoints));
   EXPECT_EQ(115.802643f, quadpoints.x1);
   EXPECT_EQ(718.913940f, quadpoints.y1);
   EXPECT_EQ(157.211182f, quadpoints.x4);
@@ -152,7 +153,8 @@ TEST_F(FPDFAnnotEmbeddertest, ExtractInkMultiple) {
 
   // Check that the rectange coordinates are correct.
   // Note that upon rendering, the rectangle coordinates will be adjusted.
-  FS_RECTF rect = FPDFAnnot_GetRect(annot);
+  FS_RECTF rect;
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &rect));
   EXPECT_EQ(351.820404f, rect.left);
   EXPECT_EQ(583.830688f, rect.bottom);
   EXPECT_EQ(475.336090f, rect.right);
@@ -223,7 +225,8 @@ TEST_F(FPDFAnnotEmbeddertest, AddFirstTextAnnotation) {
   EXPECT_EQ(51u, A);
 
   // Set the annotation rectangle.
-  FS_RECTF rect = FPDFAnnot_GetRect(annot);
+  FS_RECTF rect;
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &rect));
   EXPECT_EQ(0.f, rect.left);
   EXPECT_EQ(0.f, rect.right);
   rect.left = 35;
@@ -232,7 +235,7 @@ TEST_F(FPDFAnnotEmbeddertest, AddFirstTextAnnotation) {
   rect.top = 165;
   ASSERT_TRUE(FPDFAnnot_SetRect(annot, &rect));
   // Check that the annotation rectangle has been set correctly.
-  rect = FPDFAnnot_GetRect(annot);
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &rect));
   EXPECT_EQ(35.f, rect.left);
   EXPECT_EQ(150.f, rect.bottom);
   EXPECT_EQ(53.f, rect.right);
@@ -270,7 +273,8 @@ TEST_F(FPDFAnnotEmbeddertest, AddAndSaveUnderlineAnnotation) {
   EXPECT_EQ(1, FPDFPage_GetAnnotCount(page));
   FPDF_ANNOTATION annot = FPDFPage_GetAnnot(page, 0);
   ASSERT_TRUE(annot);
-  FS_QUADPOINTSF quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+  FS_QUADPOINTSF quadpoints;
+  ASSERT_TRUE(FPDFAnnot_GetAttachmentPoints(annot, &quadpoints));
   EXPECT_EQ(115.802643f, quadpoints.x1);
   EXPECT_EQ(718.913940f, quadpoints.y1);
   EXPECT_EQ(157.211182f, quadpoints.x4);
@@ -301,7 +305,8 @@ TEST_F(FPDFAnnotEmbeddertest, AddAndSaveUnderlineAnnotation) {
   FPDF_ANNOTATION new_annot = FPDFPage_GetAnnot(m_SavedPage, 1);
   ASSERT_TRUE(new_annot);
   EXPECT_EQ(FPDF_ANNOT_UNDERLINE, FPDFAnnot_GetSubtype(new_annot));
-  FS_QUADPOINTSF new_quadpoints = FPDFAnnot_GetAttachmentPoints(new_annot);
+  FS_QUADPOINTSF new_quadpoints;
+  ASSERT_TRUE(FPDFAnnot_GetAttachmentPoints(new_annot, &new_quadpoints));
   EXPECT_NEAR(quadpoints.x1, new_quadpoints.x1, 0.001f);
   EXPECT_NEAR(quadpoints.y1, new_quadpoints.y1, 0.001f);
   EXPECT_NEAR(quadpoints.x4, new_quadpoints.x4, 0.001f);
@@ -329,7 +334,8 @@ TEST_F(FPDFAnnotEmbeddertest, ModifyRectQuadpointsWithAP) {
 
   // Check that when getting the attachment points, bounding box points are
   // returned since this is a markup annotation with AP defined.
-  FS_QUADPOINTSF quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+  FS_QUADPOINTSF quadpoints;
+  ASSERT_TRUE(FPDFAnnot_GetAttachmentPoints(annot, &quadpoints));
   EXPECT_NEAR(0.f, quadpoints.x1, 0.001f);
   EXPECT_NEAR(16.9955f, quadpoints.y1, 0.001f);
   EXPECT_NEAR(68.5953f, quadpoints.x4, 0.001f);
@@ -340,7 +346,8 @@ TEST_F(FPDFAnnotEmbeddertest, ModifyRectQuadpointsWithAP) {
   quadpoints.x1 = 1.0f;
   quadpoints.x3 = 1.0f;
   ASSERT_TRUE(FPDFAnnot_SetAttachmentPoints(annot, &quadpoints));
-  FS_QUADPOINTSF new_quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+  FS_QUADPOINTSF new_quadpoints;
+  ASSERT_TRUE(FPDFAnnot_GetAttachmentPoints(annot, &new_quadpoints));
   EXPECT_NE(quadpoints.x1, new_quadpoints.x1);
 
   // Check that the bounding box gets updated successfully when valid attachment
@@ -352,7 +359,7 @@ TEST_F(FPDFAnnotEmbeddertest, ModifyRectQuadpointsWithAP) {
   quadpoints.x3 = 0.f;
   quadpoints.x4 = 133.055f;
   ASSERT_TRUE(FPDFAnnot_SetAttachmentPoints(annot, &quadpoints));
-  new_quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+  ASSERT_TRUE(FPDFAnnot_GetAttachmentPoints(annot, &new_quadpoints));
   EXPECT_EQ(quadpoints.x1, new_quadpoints.x1);
   EXPECT_EQ(quadpoints.y1, new_quadpoints.y1);
   EXPECT_EQ(quadpoints.x4, new_quadpoints.x4);
@@ -360,7 +367,8 @@ TEST_F(FPDFAnnotEmbeddertest, ModifyRectQuadpointsWithAP) {
 
   // Check that when getting the annotation rectangle, rectangle points are
   // returned, but not bounding box points.
-  FS_RECTF rect = FPDFAnnot_GetRect(annot);
+  FS_RECTF rect;
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &rect));
   EXPECT_NEAR(67.7299f, rect.left, 0.001f);
   EXPECT_NEAR(704.296f, rect.bottom, 0.001f);
   EXPECT_NEAR(136.325f, rect.right, 0.001f);
@@ -373,9 +381,10 @@ TEST_F(FPDFAnnotEmbeddertest, ModifyRectQuadpointsWithAP) {
   rect.right = 134.055f;
   rect.top = 722.792f;
   ASSERT_TRUE(FPDFAnnot_SetRect(annot, &rect));
-  FS_RECTF new_rect = FPDFAnnot_GetRect(annot);
+  FS_RECTF new_rect;
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &new_rect));
   EXPECT_EQ(rect.right, new_rect.right);
-  new_quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+  ASSERT_TRUE(FPDFAnnot_GetAttachmentPoints(annot, &new_quadpoints));
   EXPECT_NE(rect.right, new_quadpoints.x2);
 
   FPDFPage_CloseAnnot(annot);
@@ -387,10 +396,10 @@ TEST_F(FPDFAnnotEmbeddertest, ModifyRectQuadpointsWithAP) {
 
   // Check that the rectangle and the bounding box get updated successfully when
   // a valid rectangle is set, since this is not a markup annotation.
-  rect = FPDFAnnot_GetRect(annot);
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &rect));
   rect.right += 1.f;
   ASSERT_TRUE(FPDFAnnot_SetRect(annot, &rect));
-  new_rect = FPDFAnnot_GetRect(annot);
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &new_rect));
   EXPECT_EQ(rect.right, new_rect.right);
 
   FPDFPage_CloseAnnot(annot);
@@ -406,17 +415,18 @@ TEST_F(FPDFAnnotEmbeddertest, RemoveAnnotation) {
 
   // Check that the annotations have the expected rectangle coordinates.
   FPDF_ANNOTATION annot = FPDFPage_GetAnnot(page, 0);
-  FS_RECTF rect = FPDFAnnot_GetRect(annot);
+  FS_RECTF rect;
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &rect));
   EXPECT_NEAR(86.1971f, rect.left, 0.001f);
   FPDFPage_CloseAnnot(annot);
 
   annot = FPDFPage_GetAnnot(page, 1);
-  rect = FPDFAnnot_GetRect(annot);
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &rect));
   EXPECT_NEAR(149.8127f, rect.left, 0.001f);
   FPDFPage_CloseAnnot(annot);
 
   annot = FPDFPage_GetAnnot(page, 2);
-  rect = FPDFAnnot_GetRect(annot);
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &rect));
   EXPECT_NEAR(351.8204f, rect.left, 0.001f);
   FPDFPage_CloseAnnot(annot);
 
@@ -454,12 +464,12 @@ TEST_F(FPDFAnnotEmbeddertest, RemoveAnnotation) {
   // Check that the remaining 2 annotations are the original 1st and 3rd ones by
   // verifying their rectangle coordinates.
   annot = FPDFPage_GetAnnot(new_page, 0);
-  rect = FPDFAnnot_GetRect(annot);
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &rect));
   EXPECT_NEAR(86.1971f, rect.left, 0.001f);
   FPDFPage_CloseAnnot(annot);
 
   annot = FPDFPage_GetAnnot(new_page, 1);
-  rect = FPDFAnnot_GetRect(annot);
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &rect));
   EXPECT_NEAR(351.8204f, rect.left, 0.001f);
   FPDFPage_CloseAnnot(annot);
   FPDF_ClosePage(new_page);
@@ -563,7 +573,8 @@ TEST_F(FPDFAnnotEmbeddertest, AddAndModifyPath) {
   EXPECT_EQ(1, FPDFAnnot_GetObjectCount(annot));
 
   // Check that the annotation's bounding box came from its rectangle.
-  FS_RECTF new_rect = FPDFAnnot_GetRect(annot);
+  FS_RECTF new_rect;
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &new_rect));
   EXPECT_EQ(rect.left, new_rect.left);
   EXPECT_EQ(rect.bottom, new_rect.bottom);
   EXPECT_EQ(rect.right, new_rect.right);
@@ -584,7 +595,7 @@ TEST_F(FPDFAnnotEmbeddertest, AddAndModifyPath) {
   EXPECT_EQ(1, FPDFAnnot_GetObjectCount(annot));
 
   // Check that the new annotation's rectangle is as defined.
-  new_rect = FPDFAnnot_GetRect(annot);
+  ASSERT_TRUE(FPDFAnnot_GetRect(annot, &new_rect));
   EXPECT_EQ(rect.left, new_rect.left);
   EXPECT_EQ(rect.bottom, new_rect.bottom);
   EXPECT_EQ(rect.right, new_rect.right);
diff --git a/public/fpdf_annot.h b/public/fpdf_annot.h
index 243896eec1..1de99453fc 100644
--- a/public/fpdf_annot.h
+++ b/public/fpdf_annot.h
@@ -299,10 +299,12 @@ FPDFAnnot_SetAttachmentPoints(FPDF_ANNOTATION annot,
 // with quadpoints, then return the bounding box it specifies instead.
 //
 //   annot      - handle to an annotation.
+//   quadPoints - receives the quadpoints; must not be NULL.
 //
-// Returns a quadpoints object, or an empty set of quadpoints on failure.
-FPDF_EXPORT FS_QUADPOINTSF FPDF_CALLCONV
-FPDFAnnot_GetAttachmentPoints(FPDF_ANNOTATION annot);
+// Returns true if successful.
+FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
+FPDFAnnot_GetAttachmentPoints(FPDF_ANNOTATION annot,
+                              FS_QUADPOINTSF* quadPoints);
 
 // Experimental API.
 // Set the annotation rectangle defining the location of the annotation. If the
@@ -322,9 +324,11 @@ FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFAnnot_SetRect(FPDF_ANNOTATION annot,
 // without quadpoints, then return the bounding box it specifies instead.
 //
 //   annot  - handle to an annotation.
+//   rect   - receives the rectangle; must not be NULL.
 //
-// Returns a rectangle object, or an empty rectangle on failure.
-FPDF_EXPORT FS_RECTF FPDF_CALLCONV FPDFAnnot_GetRect(FPDF_ANNOTATION annot);
+// Returns true if successful.
+FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV FPDFAnnot_GetRect(FPDF_ANNOTATION annot,
+                                                      FS_RECTF* rect);
 
 // Experimental API.
 // Check if |annot|'s dictionary has |key| as a key.
diff --git a/samples/pdfium_test.cc b/samples/pdfium_test.cc
index f8096abc50..31235f0eff 100644
--- a/samples/pdfium_test.cc
+++ b/samples/pdfium_test.cc
@@ -416,18 +416,26 @@ void WriteAnnot(FPDF_PAGE page, const char* pdf_name, int num) {
 
     // Retrieve the annotation's quadpoints if it is a markup annotation.
     if (FPDFAnnot_HasAttachmentPoints(annot)) {
-      FS_QUADPOINTSF quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
-      fprintf(fp,
-              "Quadpoints: (%.3f, %.3f), (%.3f, %.3f), (%.3f, %.3f), (%.3f, "
-              "%.3f)\n",
-              quadpoints.x1, quadpoints.y1, quadpoints.x2, quadpoints.y2,
-              quadpoints.x3, quadpoints.y3, quadpoints.x4, quadpoints.y4);
+      FS_QUADPOINTSF quadpoints;
+      if (FPDFAnnot_GetAttachmentPoints(annot, &quadpoints)) {
+        fprintf(fp,
+                "Quadpoints: (%.3f, %.3f), (%.3f, %.3f), (%.3f, %.3f), (%.3f, "
+                "%.3f)\n",
+                quadpoints.x1, quadpoints.y1, quadpoints.x2, quadpoints.y2,
+                quadpoints.x3, quadpoints.y3, quadpoints.x4, quadpoints.y4);
+      } else {
+        fprintf(fp, "Failed to retrieve quadpoints.\n");
+      }
     }
 
     // Retrieve the annotation's rectangle coordinates.
-    FS_RECTF rect = FPDFAnnot_GetRect(annot);
-    fprintf(fp, "Rectangle: l - %.3f, b - %.3f, r - %.3f, t - %.3f\n\n",
-            rect.left, rect.bottom, rect.right, rect.top);
+    FS_RECTF rect;
+    if (FPDFAnnot_GetRect(annot, &rect)) {
+      fprintf(fp, "Rectangle: l - %.3f, b - %.3f, r - %.3f, t - %.3f\n\n",
+              rect.left, rect.bottom, rect.right, rect.top);
+    } else {
+      fprintf(fp, "Failed to retrieve annotation rectangle.\n");
+    }
 
     FPDFPage_CloseAnnot(annot);
   }