Made existing annotation APIs to work with APchromium/3143

1. Modified some existing annotation APIs to take into account the
effect of annotation's appearance streams.
    * Added an embedder test testing annotations with APs.

This CL is refactored out of 
https://pdfium-review.googlesource.com/c/6676/.

Bug=pdfium:737

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

3 files changed, 197 insertions, 41 deletions

diff --git a/fpdfsdk/fpdfannot.cpp b/fpdfsdk/fpdfannot.cpp
index 169c31c236..4a288b7df7 100644
--- a/fpdfsdk/fpdfannot.cpp
+++ b/fpdfsdk/fpdfannot.cpp
@@ -136,6 +136,10 @@ CPDF_AnnotContext* CPDFAnnotContextFromFPDFAnnotation(FPDF_ANNOTATION annot) {
   return static_cast<CPDF_AnnotContext*>(annot);
 }
 
+bool HasAPStream(const CPDF_Dictionary* pAnnotDict) {
+  return !!FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+}
+
 }  // namespace
 
 DLLEXPORT FPDF_BOOL STDCALL
@@ -225,6 +229,12 @@ DLLEXPORT FPDF_BOOL STDCALL FPDFAnnot_SetColor(FPDF_ANNOTATION annot,
   if (!pAnnotDict)
     return false;
 
+  // For annotations with their appearance streams already defined, the path
+  // stream's own color definitions take priority over the annotation color
+  // definitions set by this method, hence this method will simply fail.
+  if (HasAPStream(pAnnotDict))
+    return false;
+
   // Set the opacity of the annotation.
   pAnnotDict->SetNewFor<CPDF_Number>("CA", A / 255.f);
 
@@ -257,6 +267,12 @@ DLLEXPORT FPDF_BOOL STDCALL FPDFAnnot_GetColor(FPDF_ANNOTATION annot,
   if (!pAnnotDict)
     return false;
 
+  // For annotations with their appearance streams already defined, the path
+  // stream's own color definitions take priority over the annotation color
+  // definitions retrieved by this method, hence this method will simply fail.
+  if (HasAPStream(pAnnotDict))
+    return false;
+
   CPDF_Array* pColor = pAnnotDict->GetArrayFor(
       type == FPDFANNOT_COLORTYPE_InteriorColor ? "IC" : "C");
   *A =
@@ -315,8 +331,8 @@ FPDFAnnot_HasAttachmentPoints(FPDF_ANNOTATION annot) {
 
 DLLEXPORT FPDF_BOOL STDCALL
 FPDFAnnot_SetAttachmentPoints(FPDF_ANNOTATION annot,
-                              FS_QUADPOINTSF quadPoints) {
-  if (!annot || !FPDFAnnot_HasAttachmentPoints(annot))
+                              const FS_QUADPOINTSF* quadPoints) {
+  if (!annot || !quadPoints || !FPDFAnnot_HasAttachmentPoints(annot))
     return false;
 
   CPDF_Dictionary* pAnnotDict =
@@ -324,20 +340,33 @@ FPDFAnnot_SetAttachmentPoints(FPDF_ANNOTATION annot,
   if (!pAnnotDict)
     return false;
 
+  // Update the "QuadPoints" entry in the annotation dictionary.
   CPDF_Array* pQuadPoints = pAnnotDict->GetArrayFor("QuadPoints");
   if (pQuadPoints)
     pQuadPoints->Clear();
   else
     pQuadPoints = pAnnotDict->SetNewFor<CPDF_Array>("QuadPoints");
 
-  pQuadPoints->AddNew<CPDF_Number>(quadPoints.x1);
-  pQuadPoints->AddNew<CPDF_Number>(quadPoints.y1);
-  pQuadPoints->AddNew<CPDF_Number>(quadPoints.x2);
-  pQuadPoints->AddNew<CPDF_Number>(quadPoints.y2);
-  pQuadPoints->AddNew<CPDF_Number>(quadPoints.x3);
-  pQuadPoints->AddNew<CPDF_Number>(quadPoints.y3);
-  pQuadPoints->AddNew<CPDF_Number>(quadPoints.x4);
-  pQuadPoints->AddNew<CPDF_Number>(quadPoints.y4);
+  pQuadPoints->AddNew<CPDF_Number>(quadPoints->x1);
+  pQuadPoints->AddNew<CPDF_Number>(quadPoints->y1);
+  pQuadPoints->AddNew<CPDF_Number>(quadPoints->x2);
+  pQuadPoints->AddNew<CPDF_Number>(quadPoints->y2);
+  pQuadPoints->AddNew<CPDF_Number>(quadPoints->x3);
+  pQuadPoints->AddNew<CPDF_Number>(quadPoints->y3);
+  pQuadPoints->AddNew<CPDF_Number>(quadPoints->x4);
+  pQuadPoints->AddNew<CPDF_Number>(quadPoints->y4);
+
+  // If the annotation's appearance stream is defined, and the new quadpoints
+  // defines a bigger bounding box than the appearance stream currently
+  // specifies, then update the "BBox" entry in the AP dictionary too, since it
+  // comes from annotation dictionary's "QuadPoints" entry.
+  CPDF_Stream* pStream =
+      FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+  if (pStream) {
+    CFX_FloatRect newRect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
+    if (newRect.Contains(pStream->GetDict()->GetRectFor("BBox")))
+      pStream->GetDict()->SetRectFor("BBox", newRect);
+  }
   return true;
 }
 
@@ -351,25 +380,52 @@ FPDFAnnot_GetAttachmentPoints(FPDF_ANNOTATION annot) {
   if (!pAnnotDict)
     return FS_QUADPOINTSF();
 
-  CPDF_Array* pArray = pAnnotDict->GetArrayFor("QuadPoints");
-  if (!pArray)
-    return FS_QUADPOINTSF();
-
+  // If the annotation's appearance stream is defined, then retrieve the
+  // quadpoints defined by the "BBox" entry in the AP dictionary, since its
+  // "BBox" entry comes from annotation dictionary's "QuadPoints" entry, but
+  // takes priority over "QuadPoints" when rendering. Otherwise, retrieve
+  // the "Quadpoints" entry from the annotation dictionary.
+  CPDF_Array* pArray;
   FS_QUADPOINTSF quadPoints;
-  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);
+  CPDF_Stream* pStream =
+      FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+  if (pStream) {
+    pArray = pStream->GetDict()->GetArrayFor("BBox");
+    if (!pArray)
+      return FS_QUADPOINTSF();
+
+    // 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);
+  } 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 quadPoints;
 }
 
 DLLEXPORT FPDF_BOOL STDCALL FPDFAnnot_SetRect(FPDF_ANNOTATION annot,
-                                              FS_RECTF rect) {
-  if (!annot)
+                                              const FS_RECTF* rect) {
+  if (!annot || !rect)
     return false;
 
   CPDF_Dictionary* pAnnotDict =
@@ -377,16 +433,23 @@ DLLEXPORT FPDF_BOOL STDCALL FPDFAnnot_SetRect(FPDF_ANNOTATION annot,
   if (!pAnnotDict)
     return false;
 
-  CPDF_Array* pRect = pAnnotDict->GetArrayFor("Rect");
-  if (pRect)
-    pRect->Clear();
-  else
-    pRect = pAnnotDict->SetNewFor<CPDF_Array>("Rect");
+  CFX_FloatRect newRect(rect->left, rect->bottom, rect->right, rect->top);
+
+  // Update the "Rect" entry in the annotation dictionary.
+  pAnnotDict->SetRectFor("Rect", newRect);
 
-  pRect->AddNew<CPDF_Number>(rect.left);
-  pRect->AddNew<CPDF_Number>(rect.bottom);
-  pRect->AddNew<CPDF_Number>(rect.right);
-  pRect->AddNew<CPDF_Number>(rect.top);
+  // If the annotation's appearance stream is defined, the annotation is of a
+  // type that does not have quadpoints, and the new rectangle is bigger than
+  // the current bounding box, then update the "BBox" entry in the AP
+  // dictionary too, since its "BBox" entry comes from annotation dictionary's
+  // "Rect" entry.
+  if (FPDFAnnot_HasAttachmentPoints(annot))
+    return true;
+
+  CPDF_Stream* pStream =
+      FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+  if (pStream && newRect.Contains(pStream->GetDict()->GetRectFor("BBox")))
+    pStream->GetDict()->SetRectFor("BBox", newRect);
   return true;
 }
 
@@ -399,9 +462,19 @@ DLLEXPORT FS_RECTF STDCALL FPDFAnnot_GetRect(FPDF_ANNOTATION annot) {
   if (!pAnnotDict)
     return FS_RECTF();
 
-  CFX_FloatRect rt = pAnnotDict->GetRectFor("Rect");
-  if (rt.IsEmpty())
-    return FS_RECTF();
+  // 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
+  // by the "BBox" entry in the AP dictionary, since its "BBox" entry comes
+  // from annotation dictionary's "Rect" entry, but takes priority over "Rect"
+  // when rendering. Otherwise, retrieve the "Rect" entry from the annotation
+  // dictionary.
+  CFX_FloatRect rt;
+  CPDF_Stream* pStream =
+      FPDFDOC_GetAnnotAP(pAnnotDict, CPDF_Annot::AppearanceMode::Normal);
+  if (!pStream || FPDFAnnot_HasAttachmentPoints(annot))
+    rt = pAnnotDict->GetRectFor("Rect");
+  else
+    rt = pStream->GetDict()->GetRectFor("BBox");
 
   FS_RECTF rect;
   rect.left = rt.left;
diff --git a/fpdfsdk/fpdfannot_embeddertest.cpp b/fpdfsdk/fpdfannot_embeddertest.cpp
index 4d6ae1c6d8..9d9e2d365f 100644
--- a/fpdfsdk/fpdfannot_embeddertest.cpp
+++ b/fpdfsdk/fpdfannot_embeddertest.cpp
@@ -218,7 +218,7 @@ TEST_F(FPDFAnnotEmbeddertest, AddFirstTextAnnotation) {
   rect.bottom = 150;
   rect.right = 53;
   rect.top = 165;
-  ASSERT_TRUE(FPDFAnnot_SetRect(annot, rect));
+  ASSERT_TRUE(FPDFAnnot_SetRect(annot, &rect));
   // Check that the annotation rectangle has been set correctly.
   rect = FPDFAnnot_GetRect(annot);
   EXPECT_EQ(35.f, rect.left);
@@ -269,7 +269,7 @@ TEST_F(FPDFAnnotEmbeddertest, AddAndSaveUnderlineAnnotation) {
   ASSERT_TRUE(annot);
   quadpoints.x1 = 140.802643f;
   quadpoints.x3 = 140.802643f;
-  ASSERT_TRUE(FPDFAnnot_SetAttachmentPoints(annot, quadpoints));
+  ASSERT_TRUE(FPDFAnnot_SetAttachmentPoints(annot, &quadpoints));
   FPDFPage_CloseAnnot(annot);
 
   // Save the document, closing the page and document.
@@ -306,3 +306,89 @@ TEST_F(FPDFAnnotEmbeddertest, AddAndSaveUnderlineAnnotation) {
   FPDF_ClosePage(new_page);
   FPDF_CloseDocument(new_doc);
 }
+
+TEST_F(FPDFAnnotEmbeddertest, ModifyRectQuadpointsWithAP) {
+  // Open a file with four annotations and load its first page.
+  ASSERT_TRUE(OpenDocument("annotation_highlight_square_with_ap.pdf"));
+  FPDF_PAGE page = FPDF_LoadPage(document(), 0);
+  ASSERT_TRUE(page);
+  EXPECT_EQ(4, FPDFPage_GetAnnotCount(page));
+
+  // Retrieve the highlight annotation which has its AP stream already defined.
+  FPDF_ANNOTATION annot = FPDFPage_GetAnnot(page, 0);
+  ASSERT_TRUE(annot);
+  EXPECT_EQ(FPDF_ANNOT_HIGHLIGHT, FPDFAnnot_GetSubtype(annot));
+
+  // Check that color cannot be set when an AP stream is defined already.
+  EXPECT_FALSE(
+      FPDFAnnot_SetColor(annot, FPDFANNOT_COLORTYPE_Color, 51, 102, 153, 204));
+
+  // 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);
+  EXPECT_NEAR(0.f, quadpoints.x1, 0.001f);
+  EXPECT_NEAR(16.9955f, quadpoints.y1, 0.001f);
+  EXPECT_NEAR(68.5953f, quadpoints.x4, 0.001f);
+  EXPECT_NEAR(0.f, quadpoints.y4, 0.001f);
+
+  // Check that when new attachment points define a smaller bounding box, the
+  // bounding box does not get updated.
+  quadpoints.x1 = 1.0f;
+  quadpoints.x3 = 1.0f;
+  ASSERT_TRUE(FPDFAnnot_SetAttachmentPoints(annot, &quadpoints));
+  FS_QUADPOINTSF new_quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+  EXPECT_NE(quadpoints.x1, new_quadpoints.x1);
+
+  // Check that the bounding box gets updated successfully when valid attachment
+  // points are set.
+  quadpoints.x1 = 0.f;
+  quadpoints.y1 = 721.792f;
+  quadpoints.x2 = 133.055f;
+  quadpoints.y2 = 721.792f;
+  quadpoints.x3 = 0.f;
+  quadpoints.x4 = 133.055f;
+  ASSERT_TRUE(FPDFAnnot_SetAttachmentPoints(annot, &quadpoints));
+  new_quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+  EXPECT_EQ(quadpoints.x1, new_quadpoints.x1);
+  EXPECT_EQ(quadpoints.y1, new_quadpoints.y1);
+  EXPECT_EQ(quadpoints.x4, new_quadpoints.x4);
+  EXPECT_EQ(quadpoints.y4, new_quadpoints.y4);
+
+  // Check that when getting the annotation rectangle, rectangle points are
+  // returned, but not bounding box points.
+  FS_RECTF rect = FPDFAnnot_GetRect(annot);
+  EXPECT_NEAR(67.7299f, rect.left, 0.001f);
+  EXPECT_NEAR(704.296f, rect.bottom, 0.001f);
+  EXPECT_NEAR(136.325f, rect.right, 0.001f);
+  EXPECT_NEAR(721.292f, rect.top, 0.001f);
+
+  // Check that the rectangle gets updated successfully when a valid rectangle
+  // is set, and that the bounding box is not modified.
+  rect.left = 0.f;
+  rect.bottom = 0.f;
+  rect.right = 134.055f;
+  rect.top = 722.792f;
+  ASSERT_TRUE(FPDFAnnot_SetRect(annot, &rect));
+  FS_RECTF new_rect = FPDFAnnot_GetRect(annot);
+  EXPECT_EQ(rect.right, new_rect.right);
+  new_quadpoints = FPDFAnnot_GetAttachmentPoints(annot);
+  EXPECT_NE(rect.right, new_quadpoints.x2);
+
+  FPDFPage_CloseAnnot(annot);
+
+  // Retrieve the square annotation which has its AP stream already defined.
+  annot = FPDFPage_GetAnnot(page, 2);
+  ASSERT_TRUE(annot);
+  EXPECT_EQ(FPDF_ANNOT_SQUARE, FPDFAnnot_GetSubtype(annot));
+
+  // 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);
+  rect.right += 1.f;
+  ASSERT_TRUE(FPDFAnnot_SetRect(annot, &rect));
+  new_rect = FPDFAnnot_GetRect(annot);
+  EXPECT_EQ(rect.right, new_rect.right);
+
+  FPDFPage_CloseAnnot(annot);
+  UnloadPage(page);
+}
diff --git a/fpdfsdk/fsdk_define.h b/fpdfsdk/fsdk_define.h
index 1aec6cb963..12b9b4771c 100644
--- a/fpdfsdk/fsdk_define.h
+++ b/fpdfsdk/fsdk_define.h
@@ -62,9 +62,6 @@ FPDF_DOCUMENT FPDFDocumentFromCPDFDocument(CPDF_Document* doc);
 
 CPDF_Page* CPDFPageFromFPDFPage(FPDF_PAGE page);
 
-FPDF_ANNOTATION FPDFAnnotationFromCPDFDictionary(CPDF_Dictionary* pDict);
-CPDF_Dictionary* CPDFDictionaryFromFPDFAnnotation(FPDF_ANNOTATION annot);
-
 CPDF_PathObject* CPDFPathObjectFromFPDFPageObject(FPDF_PAGEOBJECT page_object);
 
 CFX_DIBitmap* CFXBitmapFromFPDFBitmap(FPDF_BITMAP bitmap);