Unify of saving documents.chromium/3181

In the original code the method of writing of objects depends on a much unpredictable factors:
as:
1) Is there an updated version of the at least one object in the document.
2) The password is changed.
3) Was this object loaded earlier.
4) The Object is compressed and document have a password.

With these factors it is difficult to predict what will be the final file.
To reduce volatility use only one method that works in all cases mentioned.
This method is parse then serialize.

Change-Id: I3d7dcadd10abffbad68d1f993f2dd60b039ed989
Reviewed-on: https://pdfium-review.googlesource.com/9572
Commit-Queue: Art Snake <art-snake@yandex-team.ru>
Reviewed-by: dsinclair <dsinclair@chromium.org>

7 files changed, 57 insertions, 149 deletions

diff --git a/BUILD.gn b/BUILD.gn
index 7710a65885..881c3718aa 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -1988,6 +1988,7 @@ test("pdfium_unittests") {
 
 test("pdfium_embeddertests") {
   sources = [
+    "core/fpdfapi/edit/cpdf_creator_embeddertest.cpp",
     "core/fpdfapi/page/cpdf_stitchfunc_embeddertest.cpp",
     "core/fpdfapi/parser/cpdf_parser_embeddertest.cpp",
     "core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp",
diff --git a/core/fpdfapi/edit/cpdf_creator.cpp b/core/fpdfapi/edit/cpdf_creator.cpp
index b347d45dce..9735460f41 100644
--- a/core/fpdfapi/edit/cpdf_creator.cpp
+++ b/core/fpdfapi/edit/cpdf_creator.cpp
@@ -323,34 +323,15 @@ bool CPDF_Creator::WriteOldIndirectObject(uint32_t objnum) {
   m_ObjectOffsets[objnum] = m_Archive->CurrentOffset();
 
   bool bExistInMap = !!m_pDocument->GetIndirectObject(objnum);
-  const CPDF_Parser::ObjectType object_type = m_pParser->GetObjectType(objnum);
-  if (m_pParser->IsVersionUpdated() || m_bSecurityChanged || bExistInMap ||
-      (object_type == CPDF_Parser::ObjectType::kCompressed && m_pEncryptDict)) {
-    CPDF_Object* pObj = m_pDocument->GetOrParseIndirectObject(objnum);
-    if (!pObj) {
-      m_ObjectOffsets.erase(objnum);
-      return true;
-    }
-    if (!WriteIndirectObj(pObj->GetObjNum(), pObj))
-      return false;
-    if (!bExistInMap)
-      m_pDocument->DeleteIndirectObject(objnum);
-  } else {
-    std::vector<uint8_t> buffer = m_pParser->GetIndirectBinary(objnum);
-    if (buffer.empty())
-      return true;
-    if (object_type == CPDF_Parser::ObjectType::kCompressed) {
-      if (!m_Archive->WriteDWord(objnum) ||
-          !m_Archive->WriteString(" 0 obj ") ||
-          !m_Archive->WriteBlock(buffer.data(), buffer.size()) ||
-          !m_Archive->WriteString("\r\nendobj\r\n")) {
-        return false;
-      }
-    } else {
-      if (!m_Archive->WriteBlock(buffer.data(), buffer.size()))
-        return false;
-    }
+  CPDF_Object* pObj = m_pDocument->GetOrParseIndirectObject(objnum);
+  if (!pObj) {
+    m_ObjectOffsets.erase(objnum);
+    return true;
   }
+  if (!WriteIndirectObj(pObj->GetObjNum(), pObj))
+    return false;
+  if (!bExistInMap)
+    m_pDocument->DeleteIndirectObject(objnum);
   return true;
 }
 
diff --git a/core/fpdfapi/edit/cpdf_creator_embeddertest.cpp b/core/fpdfapi/edit/cpdf_creator_embeddertest.cpp
new file mode 100644
index 0000000000..def7d50a97
--- /dev/null
+++ b/core/fpdfapi/edit/cpdf_creator_embeddertest.cpp
@@ -0,0 +1,43 @@
+// Copyright 2017 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 <memory>
+#include <string>
+#include <vector>
+
+#include "core/fxcrt/fx_system.h"
+#include "public/fpdf_annot.h"
+#include "public/fpdf_edit.h"
+#include "public/fpdfview.h"
+#include "testing/embedder_test.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class CPDF_CreatorEmbedderTest : public EmbedderTest {};
+
+TEST_F(CPDF_CreatorEmbedderTest, SavedDocsAreEqualAfterParse) {
+  ASSERT_TRUE(OpenDocument("annotation_stamp_with_ap.pdf"));
+  // Save without additional data reading.
+  EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
+  const std::string saved_doc_1 = GetString();
+  ClearString();
+
+  {
+    // Do some read only operations.
+    ASSERT_GE(1, FPDF_GetPageCount(document()));
+    FPDF_PAGE page = FPDF_LoadPage(document(), 0);
+    ASSERT_TRUE(page);
+    FPDF_BITMAP new_bitmap =
+        RenderPageWithFlags(page, form_handle(), FPDF_ANNOT);
+    FPDFBitmap_Destroy(new_bitmap);
+    UnloadPage(page);
+  }
+
+  // Save when we have additional loaded data.
+  EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
+  const std::string saved_doc_2 = GetString();
+  ClearString();
+
+  // The sizes of saved docs should be equal.
+  EXPECT_EQ(saved_doc_1.size(), saved_doc_2.size());
+}
diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp
index 32c616c1c6..da51eb1c83 100644
--- a/core/fpdfapi/parser/cpdf_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_parser.cpp
@@ -1258,122 +1258,6 @@ FX_FILESIZE CPDF_Parser::GetObjectSize(uint32_t objnum) const {
   return *it - offset;
 }
 
-std::vector<uint8_t> CPDF_Parser::GetIndirectBinary(uint32_t objnum) {
-  std::vector<uint8_t> buffer;
-  if (!IsValidObjectNumber(objnum))
-    return buffer;
-
-  if (GetObjectType(objnum) == ObjectType::kCompressed) {
-    CFX_RetainPtr<CPDF_StreamAcc> pObjStream =
-        GetObjectStream(m_ObjectInfo[objnum].pos);
-    if (!pObjStream)
-      return buffer;
-
-    int32_t offset = GetStreamFirst(pObjStream);
-    const uint8_t* pData = pObjStream->GetData();
-    uint32_t totalsize = pObjStream->GetSize();
-    auto file = pdfium::MakeRetain<CFX_MemoryStream>(
-        const_cast<uint8_t*>(pData), static_cast<size_t>(totalsize), false);
-    CPDF_SyntaxParser syntax;
-    syntax.InitParser(file, 0);
-
-    for (int i = GetStreamNCount(pObjStream); i > 0; --i) {
-      uint32_t thisnum = syntax.GetDirectNum();
-      uint32_t thisoff = syntax.GetDirectNum();
-      if (thisnum != objnum)
-        continue;
-
-      size_t size = 0;
-      if (i == 1) {
-        size = totalsize - (thisoff + offset);
-      } else {
-        syntax.GetDirectNum();  // Skip nextnum.
-        uint32_t nextoff = syntax.GetDirectNum();
-        size = nextoff - thisoff;
-      }
-
-      buffer.resize(size);
-      memcpy(buffer.data(), pData + thisoff + offset, size);
-      break;
-    }
-    return buffer;
-  }
-
-  if (GetObjectType(objnum) != ObjectType::kNotCompressed)
-    return buffer;
-
-  FX_FILESIZE pos = m_ObjectInfo[objnum].pos;
-  if (pos == 0)
-    return buffer;
-
-  FX_FILESIZE SavedPos = m_pSyntax->GetPos();
-  m_pSyntax->SetPos(pos);
-
-  bool bIsNumber;
-  CFX_ByteString word = m_pSyntax->GetNextWord(&bIsNumber);
-  if (!bIsNumber) {
-    m_pSyntax->SetPos(SavedPos);
-    return buffer;
-  }
-
-  uint32_t parser_objnum = FXSYS_atoui(word.c_str());
-  if (parser_objnum && parser_objnum != objnum) {
-    m_pSyntax->SetPos(SavedPos);
-    return buffer;
-  }
-
-  word = m_pSyntax->GetNextWord(&bIsNumber);
-  if (!bIsNumber) {
-    m_pSyntax->SetPos(SavedPos);
-    return buffer;
-  }
-
-  if (m_pSyntax->GetKeyword() != "obj") {
-    m_pSyntax->SetPos(SavedPos);
-    return buffer;
-  }
-
-  auto it = m_SortedOffset.find(pos);
-  if (it == m_SortedOffset.end() || ++it == m_SortedOffset.end()) {
-    m_pSyntax->SetPos(SavedPos);
-    return buffer;
-  }
-
-  FX_FILESIZE nextoff = *it;
-  bool bNextOffValid = false;
-  if (nextoff != pos) {
-    m_pSyntax->SetPos(nextoff);
-    word = m_pSyntax->GetNextWord(&bIsNumber);
-    if (word == "xref") {
-      bNextOffValid = true;
-    } else if (bIsNumber) {
-      word = m_pSyntax->GetNextWord(&bIsNumber);
-      if (bIsNumber && m_pSyntax->GetKeyword() == "obj") {
-        bNextOffValid = true;
-      }
-    }
-  }
-
-  if (!bNextOffValid) {
-    m_pSyntax->SetPos(pos);
-    while (1) {
-      if (m_pSyntax->GetKeyword() == "endobj")
-        break;
-
-      if (m_pSyntax->GetPos() == m_pSyntax->m_FileLen)
-        break;
-    }
-    nextoff = m_pSyntax->GetPos();
-  }
-
-  size_t size = (uint32_t)(nextoff - pos);
-  buffer.resize(size);
-  m_pSyntax->SetPos(pos);
-  m_pSyntax->ReadBlock(buffer.data(), size);
-  m_pSyntax->SetPos(SavedPos);
-  return buffer;
-}
-
 std::unique_ptr<CPDF_Object> CPDF_Parser::ParseIndirectObjectAt(
     CPDF_IndirectObjectHolder* pObjList,
     FX_FILESIZE pos,
diff --git a/core/fpdfapi/parser/cpdf_parser.h b/core/fpdfapi/parser/cpdf_parser.h
index c379905aed..fd74c5471e 100644
--- a/core/fpdfapi/parser/cpdf_parser.h
+++ b/core/fpdfapi/parser/cpdf_parser.h
@@ -91,7 +91,6 @@ class CPDF_Parser {
   FX_FILESIZE GetObjectOffset(uint32_t objnum) const;
   FX_FILESIZE GetObjectSize(uint32_t objnum) const;
 
-  std::vector<uint8_t> GetIndirectBinary(uint32_t objnum);
   int GetFileVersion() const { return m_FileVersion; }
   bool IsXRefStream() const { return m_bXRefStream; }
 
diff --git a/fpdfsdk/fpdfannot_embeddertest.cpp b/fpdfsdk/fpdfannot_embeddertest.cpp
index 58a00067a7..66260bd8ac 100644
--- a/fpdfsdk/fpdfannot_embeddertest.cpp
+++ b/fpdfsdk/fpdfannot_embeddertest.cpp
@@ -863,11 +863,11 @@ TEST_F(FPDFAnnotEmbeddertest, GetSetStringValue) {
 
   // Open the saved annotation.
 #if _FXM_PLATFORM_ == _FXM_PLATFORM_APPLE_
-  const char md5[] = "c35408717759562d1f8bf33d317483d2";
+  const char md5[] = "4d64e61c9c0f8c60ab3cc3234bb73b1c";
 #elif _FXM_PLATFORM_ == _FXM_PLATFORM_WINDOWS_
-  const char md5[] = "4f64add0190ede63f7bb9eb1e2e83edb";
+  const char md5[] = "0e3710ea6476f5bcba2cd39eb42d54e2";
 #else
-  const char md5[] = "02e1c6adff8fee4aeabd91c2c2e4be43";
+  const char md5[] = "831a3c465a56d2e0c89aef7bdf15306a";
 #endif
   TestSaved(595, 842, md5);
   FPDF_ANNOTATION new_annot = FPDFPage_GetAnnot(m_SavedPage, 0);
diff --git a/fpdfsdk/fpdfsave_embeddertest.cpp b/fpdfsdk/fpdfsave_embeddertest.cpp
index 260fe0c2a6..54eb347302 100644
--- a/fpdfsdk/fpdfsave_embeddertest.cpp
+++ b/fpdfsdk/fpdfsave_embeddertest.cpp
@@ -21,14 +21,14 @@ TEST_F(FPDFSaveEmbedderTest, SaveSimpleDoc) {
   EXPECT_TRUE(OpenDocument("hello_world.pdf"));
   EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
   EXPECT_THAT(GetString(), testing::StartsWith("%PDF-1.7\r\n"));
-  EXPECT_EQ(843u, GetString().length());
+  EXPECT_EQ(805u, GetString().length());
 }
 
 TEST_F(FPDFSaveEmbedderTest, SaveSimpleDocWithVersion) {
   EXPECT_TRUE(OpenDocument("hello_world.pdf"));
   EXPECT_TRUE(FPDF_SaveWithVersion(document(), this, 0, 14));
   EXPECT_THAT(GetString(), testing::StartsWith("%PDF-1.4\r\n"));
-  EXPECT_EQ(843u, GetString().length());
+  EXPECT_EQ(805u, GetString().length());
 }
 TEST_F(FPDFSaveEmbedderTest, SaveSimpleDocWithBadVersion) {
   EXPECT_TRUE(OpenDocument("hello_world.pdf"));