Use either /RECT or /QuadPoints for annotation coordinates, depending on /AP

On Acrobat, if "/AP" is present on a text markup definition,
the coordinates used to draw the annotation come from "/Rect
values, whereas if "/AP" is not defined, the array defined
in /QuadPoints is used to grab the annotation coordinates from.
PDFium, on the other hand, uses "/Rect" regardless of
presence or absence of "/AP".

CL fixes PDFium to work similarly to Acrobat, in this case.

TEST=testing/resources/pixel/bug_585_*.in
BUG=pdfium:585

Review-Url: https://codereview.chromium.org/2289293005

3 files changed, 91 insertions, 25 deletions

diff --git a/core/fpdfdoc/cpdf_annot.cpp b/core/fpdfdoc/cpdf_annot.cpp
index bee96a62ff..2cc767a066 100644
--- a/core/fpdfdoc/cpdf_annot.cpp
+++ b/core/fpdfdoc/cpdf_annot.cpp
@@ -20,6 +20,15 @@
 
 namespace {
 
+char kPDFiumKey_HasGeneratedAP[] = "PDFIUM_HasGeneratedAP";
+
+bool IsTextMarkupAnnotation(CPDF_Annot::Subtype type) {
+  return type == CPDF_Annot::Subtype::HIGHLIGHT ||
+         type == CPDF_Annot::Subtype::SQUIGGLY ||
+         type == CPDF_Annot::Subtype::STRIKEOUT ||
+         type == CPDF_Annot::Subtype::UNDERLINE;
+}
+
 bool ShouldGenerateAPForAnnotation(CPDF_Dictionary* pAnnotDict) {
   // If AP dictionary exists, we use the appearance defined in the
   // existing AP dictionary.
@@ -40,6 +49,8 @@ CPDF_Annot::CPDF_Annot(CPDF_Dictionary* pDict,
       m_bOpenState(false),
       m_pPopupAnnot(nullptr) {
   m_nSubtype = StringToAnnotSubtype(m_pAnnotDict->GetStringFor("Subtype"));
+  m_bIsTextMarkupAnnotation = IsTextMarkupAnnotation(m_nSubtype);
+  m_bHasGeneratedAP = m_pAnnotDict->GetBooleanFor(kPDFiumKey_HasGeneratedAP);
   GenerateAPIfNeeded();
 }
 
@@ -53,24 +64,30 @@ void CPDF_Annot::GenerateAPIfNeeded() {
   if (!ShouldGenerateAPForAnnotation(m_pAnnotDict))
     return;
 
+  bool result = false;
   if (m_nSubtype == CPDF_Annot::Subtype::CIRCLE)
-    CPVT_GenerateAP::GenerateCircleAP(m_pDocument, m_pAnnotDict);
+    result = CPVT_GenerateAP::GenerateCircleAP(m_pDocument, m_pAnnotDict);
   else if (m_nSubtype == CPDF_Annot::Subtype::HIGHLIGHT)
-    CPVT_GenerateAP::GenerateHighlightAP(m_pDocument, m_pAnnotDict);
+    result = CPVT_GenerateAP::GenerateHighlightAP(m_pDocument, m_pAnnotDict);
   else if (m_nSubtype == CPDF_Annot::Subtype::INK)
-    CPVT_GenerateAP::GenerateInkAP(m_pDocument, m_pAnnotDict);
+    result = CPVT_GenerateAP::GenerateInkAP(m_pDocument, m_pAnnotDict);
   else if (m_nSubtype == CPDF_Annot::Subtype::POPUP)
-    CPVT_GenerateAP::GeneratePopupAP(m_pDocument, m_pAnnotDict);
+    result = CPVT_GenerateAP::GeneratePopupAP(m_pDocument, m_pAnnotDict);
   else if (m_nSubtype == CPDF_Annot::Subtype::SQUARE)
-    CPVT_GenerateAP::GenerateSquareAP(m_pDocument, m_pAnnotDict);
+    result = CPVT_GenerateAP::GenerateSquareAP(m_pDocument, m_pAnnotDict);
   else if (m_nSubtype == CPDF_Annot::Subtype::SQUIGGLY)
-    CPVT_GenerateAP::GenerateSquigglyAP(m_pDocument, m_pAnnotDict);
+    result = CPVT_GenerateAP::GenerateSquigglyAP(m_pDocument, m_pAnnotDict);
   else if (m_nSubtype == CPDF_Annot::Subtype::STRIKEOUT)
-    CPVT_GenerateAP::GenerateStrikeOutAP(m_pDocument, m_pAnnotDict);
+    result = CPVT_GenerateAP::GenerateStrikeOutAP(m_pDocument, m_pAnnotDict);
   else if (m_nSubtype == CPDF_Annot::Subtype::TEXT)
-    CPVT_GenerateAP::GenerateTextAP(m_pDocument, m_pAnnotDict);
+    result = CPVT_GenerateAP::GenerateTextAP(m_pDocument, m_pAnnotDict);
   else if (m_nSubtype == CPDF_Annot::Subtype::UNDERLINE)
-    CPVT_GenerateAP::GenerateUnderlineAP(m_pDocument, m_pAnnotDict);
+    result = CPVT_GenerateAP::GenerateUnderlineAP(m_pDocument, m_pAnnotDict);
+
+  if (result) {
+    m_pAnnotDict->SetBooleanFor(kPDFiumKey_HasGeneratedAP, result);
+    m_bHasGeneratedAP = result;
+  }
 }
 
 bool CPDF_Annot::ShouldDrawAnnotation() {
@@ -91,11 +108,23 @@ CPDF_Annot::Subtype CPDF_Annot::GetSubtype() const {
   return m_nSubtype;
 }
 
+CFX_FloatRect CPDF_Annot::RectForDrawing() const {
+  if (!m_pAnnotDict)
+    return CFX_FloatRect();
+
+  bool bShouldUseQuadPointsCoords =
+      m_bIsTextMarkupAnnotation && m_bHasGeneratedAP;
+  if (bShouldUseQuadPointsCoords)
+    return RectFromQuadPoints(m_pAnnotDict);
+
+  return m_pAnnotDict->GetRectFor("Rect");
+}
+
 CFX_FloatRect CPDF_Annot::GetRect() const {
   if (!m_pAnnotDict)
     return CFX_FloatRect();
 
-  CFX_FloatRect rect = m_pAnnotDict->GetRectFor("Rect");
+  CFX_FloatRect rect = RectForDrawing();
   rect.Normalize();
   return rect;
 }
@@ -176,6 +205,26 @@ static CPDF_Form* FPDFDOC_Annot_GetMatrix(const CPDF_Page* pPage,
 }
 
 // Static.
+CFX_FloatRect CPDF_Annot::RectFromQuadPoints(CPDF_Dictionary* pAnnotDict) {
+  CPDF_Array* pArray = pAnnotDict->GetArrayFor("QuadPoints");
+  if (!pArray)
+    return CFX_FloatRect();
+
+  // QuadPoints are defined with 4 pairs of numbers
+  // ([ pair0, pair1, pair2, pair3 ]), where
+  // pair0 = top_left
+  // pair1 = top_right
+  // pair2 = bottom_left
+  // pair3 = bottom_right
+  //
+  // On the other hand, /Rect is define as 2 pairs [pair0, pair1] where:
+  // pair0 = bottom_left
+  // pair1 = top_right.
+  return CFX_FloatRect(pArray->GetNumberAt(4), pArray->GetNumberAt(5),
+                       pArray->GetNumberAt(2), pArray->GetNumberAt(3));
+}
+
+// Static.
 bool CPDF_Annot::IsAnnotationHidden(CPDF_Dictionary* pAnnotDict) {
   return !!(pAnnotDict->GetIntegerFor("F") & ANNOTFLAG_HIDDEN);
 }
diff --git a/core/fpdfdoc/cpvt_generateap.cpp b/core/fpdfdoc/cpvt_generateap.cpp
index decc408c33..d37aaf3acb 100644
--- a/core/fpdfdoc/cpvt_generateap.cpp
+++ b/core/fpdfdoc/cpvt_generateap.cpp
@@ -586,7 +586,8 @@ CPDF_Dictionary* GenerateResourceDict(CPDF_Dictionary* pExtGStateDict,
 void GenerateAndSetAPDict(CPDF_Document* pDoc,
                           CPDF_Dictionary* pAnnotDict,
                           const CFX_ByteTextBuf& sAppStream,
-                          CPDF_Dictionary* pResourceDict) {
+                          CPDF_Dictionary* pResourceDict,
+                          bool bIsTextMarkupAnnotation) {
   CPDF_Dictionary* pAPDict = new CPDF_Dictionary;
   pAnnotDict->SetFor("AP", pAPDict);
 
@@ -602,7 +603,9 @@ void GenerateAndSetAPDict(CPDF_Document* pDoc,
   pStreamDict->SetStringFor("Subtype", "Form");
   pStreamDict->SetMatrixFor("Matrix", CFX_Matrix());
 
-  CFX_FloatRect rect = pAnnotDict->GetRectFor("Rect");
+  CFX_FloatRect rect = bIsTextMarkupAnnotation
+                           ? CPDF_Annot::RectFromQuadPoints(pAnnotDict)
+                           : pAnnotDict->GetRectFor("Rect");
   pStreamDict->SetRectFor("BBox", rect);
 
   pStreamDict->SetFor("Resources", pResourceDict);
@@ -783,7 +786,8 @@ bool CPVT_GenerateAP::GenerateCircleAP(CPDF_Document* pDoc,
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
   CPDF_Dictionary* pResourceDict =
       GenerateResourceDict(pExtGStateDict, nullptr);
-  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+                       false /*IsTextMarkupAnnotation*/);
   return true;
 }
 
@@ -797,7 +801,7 @@ bool CPVT_GenerateAP::GenerateHighlightAP(CPDF_Document* pDoc,
                                           CPVT_Color(CPVT_Color::kRGB, 1, 1, 0),
                                           PaintOperation::FILL);
 
-  CFX_FloatRect rect = pAnnotDict->GetRectFor("Rect");
+  CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
   rect.Normalize();
 
   sAppStream << rect.left << " " << rect.top << " m " << rect.right << " "
@@ -809,7 +813,8 @@ bool CPVT_GenerateAP::GenerateHighlightAP(CPDF_Document* pDoc,
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Multiply");
   CPDF_Dictionary* pResourceDict =
       GenerateResourceDict(pExtGStateDict, nullptr);
-  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+                       true /*IsTextMarkupAnnotation*/);
 
   return true;
 }
@@ -863,7 +868,8 @@ bool CPVT_GenerateAP::GenerateInkAP(CPDF_Document* pDoc,
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
   CPDF_Dictionary* pResourceDict =
       GenerateResourceDict(pExtGStateDict, nullptr);
-  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+                       false /*IsTextMarkupAnnotation*/);
   return true;
 }
 
@@ -885,7 +891,8 @@ bool CPVT_GenerateAP::GenerateTextAP(CPDF_Document* pDoc,
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
   CPDF_Dictionary* pResourceDict =
       GenerateResourceDict(pExtGStateDict, nullptr);
-  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+                       false /*IsTextMarkupAnnotation*/);
   return true;
 }
 
@@ -899,7 +906,7 @@ bool CPVT_GenerateAP::GenerateUnderlineAP(CPDF_Document* pDoc,
                                           CPVT_Color(CPVT_Color::kRGB, 0, 0, 0),
                                           PaintOperation::STROKE);
 
-  CFX_FloatRect rect = pAnnotDict->GetRectFor("Rect");
+  CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
   rect.Normalize();
 
   FX_FLOAT fLineWidth = 1.0;
@@ -911,7 +918,8 @@ bool CPVT_GenerateAP::GenerateUnderlineAP(CPDF_Document* pDoc,
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
   CPDF_Dictionary* pResourceDict =
       GenerateResourceDict(pExtGStateDict, nullptr);
-  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+                       true /*IsTextMarkupAnnotation*/);
   return true;
 }
 
@@ -949,7 +957,8 @@ bool CPVT_GenerateAP::GeneratePopupAP(CPDF_Document* pDoc,
     return false;
 
   sAppStream << GetPopupContentsString(pDoc, *pAnnotDict, pDefFont, sFontName);
-  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+                       false /*IsTextMarkupAnnotation*/);
   return true;
 }
 
@@ -996,7 +1005,8 @@ bool CPVT_GenerateAP::GenerateSquareAP(CPDF_Document* pDoc,
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
   CPDF_Dictionary* pResourceDict =
       GenerateResourceDict(pExtGStateDict, nullptr);
-  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+                       false /*IsTextMarkupAnnotation*/);
   return true;
 }
 
@@ -1010,7 +1020,7 @@ bool CPVT_GenerateAP::GenerateSquigglyAP(CPDF_Document* pDoc,
                                           CPVT_Color(CPVT_Color::kRGB, 0, 0, 0),
                                           PaintOperation::STROKE);
 
-  CFX_FloatRect rect = pAnnotDict->GetRectFor("Rect");
+  CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
   rect.Normalize();
 
   FX_FLOAT fLineWidth = 1.0;
@@ -1044,7 +1054,8 @@ bool CPVT_GenerateAP::GenerateSquigglyAP(CPDF_Document* pDoc,
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
   CPDF_Dictionary* pResourceDict =
       GenerateResourceDict(pExtGStateDict, nullptr);
-  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+                       true /*IsTextMarkupAnnotation*/);
   return true;
 }
 
@@ -1058,7 +1069,7 @@ bool CPVT_GenerateAP::GenerateStrikeOutAP(CPDF_Document* pDoc,
                                           CPVT_Color(CPVT_Color::kRGB, 0, 0, 0),
                                           PaintOperation::STROKE);
 
-  CFX_FloatRect rect = pAnnotDict->GetRectFor("Rect");
+  CFX_FloatRect rect = CPDF_Annot::RectFromQuadPoints(pAnnotDict);
   rect.Normalize();
 
   FX_FLOAT fLineWidth = 1.0;
@@ -1070,7 +1081,8 @@ bool CPVT_GenerateAP::GenerateStrikeOutAP(CPDF_Document* pDoc,
       GenerateExtGStateDict(*pAnnotDict, sExtGSDictName, "Normal");
   CPDF_Dictionary* pResourceDict =
       GenerateResourceDict(pExtGStateDict, nullptr);
-  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict);
+  GenerateAndSetAPDict(pDoc, pAnnotDict, sAppStream, pResourceDict,
+                       true /*IsTextMarkupAnnotation*/);
   return true;
 }
 
diff --git a/core/fpdfdoc/include/cpdf_annot.h b/core/fpdfdoc/include/cpdf_annot.h
index c16decc7d6..43f58931d3 100644
--- a/core/fpdfdoc/include/cpdf_annot.h
+++ b/core/fpdfdoc/include/cpdf_annot.h
@@ -71,6 +71,7 @@ class CPDF_Annot {
   static CPDF_Annot::Subtype StringToAnnotSubtype(
       const CFX_ByteString& sSubtype);
   static CFX_ByteString AnnotSubtypeToString(CPDF_Annot::Subtype nSubtype);
+  static CFX_FloatRect RectFromQuadPoints(CPDF_Dictionary* pAnnotDict);
 
   CPDF_Annot(CPDF_Dictionary* pDict, CPDF_Document* pDocument, bool bToOwnDict);
   ~CPDF_Annot();
@@ -102,6 +103,8 @@ class CPDF_Annot {
   void GenerateAPIfNeeded();
   bool ShouldDrawAnnotation();
 
+  CFX_FloatRect RectForDrawing() const;
+
   // For regular annotations, |m_pAnnotDict| is not owned. For
   // our artificially created popup annotations, |m_pAnnotDict|
   // is owned by this class.
@@ -112,6 +115,8 @@ class CPDF_Annot {
   std::map<CPDF_Stream*, std::unique_ptr<CPDF_Form>> m_APMap;
   // |m_bOpenState| is only set for popup annotations.
   bool m_bOpenState;
+  bool m_bHasGeneratedAP;
+  bool m_bIsTextMarkupAnnotation;
   // Not owned. If there is a valid pointer in |m_pPopupAnnot|,
   // then this annot is never a popup.
   CPDF_Annot* m_pPopupAnnot;