Make FPDFAnnot_SetAP() generate PDF standard compliant AP stream

FPDFAnnot_SetAP() only sets an appearance stream in annotation’s
dictionary and does not create it as a separate XObject.

Other viewers such as Adobe Reader fail to render such annotation as
these readers expect an XObject to be present with the appearance
stream.

This CL modifies FPDFAnnot_SetAP() implementation to create appearance
stream as a XObject following the steps below:
1. Create an indirect PDF stream object.
2. Store appearance stream data in newly created indirect object.
3. Set dictionary values(BBox, Type, SubType) of indirect object.
4. Store a reference to this indirect object in actual annotation
   object's dictionary.

The CL includes a newly added test case to validate creation of
appearance stream as a separate XObject.

Bug: pdfium:1404
Change-Id: I5054cb15ad8bad6ed55f4b952197249a9cc2dfad
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/61430
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/fpdfsdk/BUILD.gn b/fpdfsdk/BUILD.gn
index afdbcf0..73117e1 100644
--- a/fpdfsdk/BUILD.gn
+++ b/fpdfsdk/BUILD.gn
@@ -106,6 +106,7 @@
 
 pdfium_unittest_source_set("unittests") {
   sources = [
+    "fpdf_annot_unittest.cpp",
     "fpdf_catalog_unittest.cpp",
     "fpdf_doc_unittest.cpp",
     "fpdf_edit_unittest.cpp",
@@ -114,6 +115,7 @@
   deps = [
     ":fpdfsdk",
     "../:pdfium_public_headers",
+    "../constants",
     "../core/fpdfapi/page",
     "../core/fpdfapi/parser",
     "../core/fpdfapi/render",
diff --git a/fpdfsdk/fpdf_annot.cpp b/fpdfsdk/fpdf_annot.cpp
index 211c102..16ae2cd 100644
--- a/fpdfsdk/fpdf_annot.cpp
+++ b/fpdfsdk/fpdf_annot.cpp
@@ -18,6 +18,7 @@
 #include "core/fpdfapi/parser/cpdf_document.h"
 #include "core/fpdfapi/parser/cpdf_name.h"
 #include "core/fpdfapi/parser/cpdf_number.h"
+#include "core/fpdfapi/parser/cpdf_reference.h"
 #include "core/fpdfapi/parser/cpdf_stream.h"
 #include "core/fpdfapi/parser/cpdf_string.h"
 #include "core/fpdfdoc/cpdf_annot.h"
@@ -769,12 +770,37 @@
   // If value is null, we're in remove mode. Otherwise, we're in add/update
   // mode.
   if (value) {
+    // Annotation object's non-empty bounding rect will be used as the /BBox
+    // for the associated /XObject object
+    CFX_FloatRect rect = pAnnotDict->GetRectFor(pdfium::annotation::kRect);
+    constexpr float kMinSize = 0.000001f;
+    if (rect.Width() < kMinSize || rect.Height() < kMinSize)
+      return false;
+
+    CPDF_AnnotContext* pAnnotContext =
+        CPDFAnnotContextFromFPDFAnnotation(annot);
+
+    CPDF_Document* pDoc = pAnnotContext->GetPage()->GetDocument();
+    if (!pDoc)
+      return false;
+
+    CPDF_Stream* pNewIndirectStream = pDoc->NewIndirect<CPDF_Stream>();
+
+    ByteString newAPStream =
+        PDF_EncodeText(WideStringFromFPDFWideString(value));
+    pNewIndirectStream->SetData(newAPStream.raw_span());
+
+    CPDF_Dictionary* pStreamDict = pNewIndirectStream->GetDict();
+    pStreamDict->SetNewFor<CPDF_Name>("Type", "XObject");
+    pStreamDict->SetNewFor<CPDF_Name>("Subtype", "Form");
+    pStreamDict->SetRectFor("BBox", rect);
+
+    // Storing reference to indirect object in annotation's AP
     if (!pApDict)
       pApDict = pAnnotDict->SetNewFor<CPDF_Dictionary>(pdfium::annotation::kAP);
 
-    ByteString newValue = PDF_EncodeText(WideStringFromFPDFWideString(value));
-    auto* pNewApStream = pApDict->SetNewFor<CPDF_Stream>(modeKey);
-    pNewApStream->SetData(newValue.raw_span());
+    pApDict->SetNewFor<CPDF_Reference>(modeKey, pDoc,
+                                       pNewIndirectStream->GetObjNum());
   } else {
     if (pApDict) {
       if (appearanceMode == FPDF_ANNOT_APPEARANCEMODE_NORMAL)
diff --git a/fpdfsdk/fpdf_annot_embeddertest.cpp b/fpdfsdk/fpdf_annot_embeddertest.cpp
index db5b3c2..44518d2 100644
--- a/fpdfsdk/fpdf_annot_embeddertest.cpp
+++ b/fpdfsdk/fpdf_annot_embeddertest.cpp
@@ -1712,6 +1712,8 @@
   {
     ScopedFPDFAnnotation annot(FPDFPage_CreateAnnot(page, FPDF_ANNOT_STAMP));
     ASSERT_TRUE(annot);
+    FS_RECTF bounding_rect{206.0f, 753.0f, 339.0f, 709.0f};
+    EXPECT_TRUE(FPDFAnnot_SetRect(annot.get(), &bounding_rect));
     EXPECT_EQ(2, FPDFPage_GetAnnotCount(page));
     EXPECT_EQ(FPDF_ANNOT_STAMP, FPDFAnnot_GetSubtype(annot.get()));
     // Also do the same test for its appearance string.
diff --git a/fpdfsdk/fpdf_annot_unittest.cpp b/fpdfsdk/fpdf_annot_unittest.cpp
new file mode 100644
index 0000000..c804562
--- /dev/null
+++ b/fpdfsdk/fpdf_annot_unittest.cpp
@@ -0,0 +1,78 @@
+// Copyright 2019 PDFium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "public/fpdf_annot.h"
+
+#include <vector>
+
+#include "constants/annotation_common.h"
+#include "core/fpdfapi/page/cpdf_annotcontext.h"
+#include "core/fpdfapi/page/cpdf_pagemodule.h"
+#include "core/fpdfapi/parser/cpdf_dictionary.h"
+#include "fpdfsdk/cpdfsdk_helpers.h"
+#include "public/cpp/fpdf_scopers.h"
+#include "public/fpdf_edit.h"
+#include "testing/fx_string_testhelpers.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class PDFAnnotTest : public testing::Test {
+ protected:
+  PDFAnnotTest() = default;
+  ~PDFAnnotTest() override = default;
+
+  void SetUp() override { CPDF_PageModule::Create(); }
+  void TearDown() override { CPDF_PageModule::Destroy(); }
+};
+
+TEST_F(PDFAnnotTest, SetAP) {
+  ScopedFPDFDocument doc(FPDF_CreateNewDocument());
+  ScopedFPDFPage page(FPDFPage_New(doc.get(), 0, 100, 100));
+  const std::wstring kStreamData =
+      L"0.0 0.0 0.0 RG 4 w 211.8 747.6 m 211.8 744.8 "
+      L"212.6 743.0 214.2 740.8 "
+      L"c 215.4 739.0 216.8 737.1 218.9 736.1 c 220.8 735.1 221.4 733.0 "
+      L"223.7 732.4 c 232.6 729.9 242.0 730.8 251.2 730.8 c 257.5 730.8 "
+      L"263.0 732.9 269.0 734.4 c S";
+  ScopedFPDFWideString ap_stream = GetFPDFWideString(kStreamData);
+
+  ScopedFPDFAnnotation annot(FPDFPage_CreateAnnot(page.get(), FPDF_ANNOT_INK));
+  ASSERT_TRUE(annot);
+
+  // Negative case: FPDFAnnot_SetAP() should fail if bounding rect is not yet
+  // set on the annotation.
+  EXPECT_FALSE(FPDFAnnot_SetAP(annot.get(), FPDF_ANNOT_APPEARANCEMODE_NORMAL,
+                               ap_stream.get()));
+
+  FS_RECTF bounding_rect{206.f, 753.f, 339.f, 709.f};
+  EXPECT_TRUE(FPDFAnnot_SetRect(annot.get(), &bounding_rect));
+
+  EXPECT_TRUE(FPDFAnnot_SetAP(annot.get(), FPDF_ANNOT_APPEARANCEMODE_NORMAL,
+                              ap_stream.get()));
+
+  // Verify that appearance stream is created as form XObject
+  CPDF_AnnotContext* context = CPDFAnnotContextFromFPDFAnnotation(annot.get());
+  ASSERT_TRUE(context);
+  CPDF_Dictionary* annot_dict = context->GetAnnotDict();
+  ASSERT_TRUE(annot_dict);
+  CPDF_Dictionary* ap_dict = annot_dict->GetDictFor(pdfium::annotation::kAP);
+  ASSERT_TRUE(ap_dict);
+  CPDF_Dictionary* stream_dict = ap_dict->GetDictFor("N");
+  ASSERT_TRUE(stream_dict);
+  ByteString type = stream_dict->GetStringFor("Type");
+  EXPECT_EQ("XObject", type);
+  ByteString sub_type = stream_dict->GetStringFor("Subtype");
+  EXPECT_EQ("Form", sub_type);
+
+  // Check that the appearance stream is same as we just set.
+  const uint32_t kStreamDataSize =
+      (kStreamData.size() + 1) * sizeof(FPDF_WCHAR);
+  unsigned long normal_length_bytes = FPDFAnnot_GetAP(
+      annot.get(), FPDF_ANNOT_APPEARANCEMODE_NORMAL, nullptr, 0);
+  ASSERT_EQ(kStreamDataSize, normal_length_bytes);
+  std::vector<FPDF_WCHAR> buf = GetFPDFWideStringBuffer(normal_length_bytes);
+  EXPECT_EQ(kStreamDataSize,
+            FPDFAnnot_GetAP(annot.get(), FPDF_ANNOT_APPEARANCEMODE_NORMAL,
+                            buf.data(), normal_length_bytes));
+  EXPECT_EQ(kStreamData, GetPlatformWString(buf.data()));
+}