Revert "Avoid duplicate data buffering in CPDF_SyntaxParser::ReadStream()."

This reverts commit 77f15f7883638a4ced131d74c053af10a5970ce9.

Reason for revert: Causes crbug.com/860210
Bug: chromium:860210

Original change's description:
> Avoid duplicate data buffering in CPDF_SyntaxParser::ReadStream().
> 
> Allow sub-streams created from an IFX_SeekableReadStream to provide
> stream data without copying memory.
> The data will only reside in the top-level stream.
> 
> For example:
> For file
> http://www.major-landrover.ru/upload/attachments/f/9/f96aab07dab04ae89c8a509ec1ef2b31.pdf
> (18 Mb)
> 
> The memory usage is reduced by ~13 Mb.
> 
> Change-Id: I2595c014d0fbe1fdd181cc04965cfd7d901c2d88
> Reviewed-on: https://pdfium-review.googlesource.com/35930
> Commit-Queue: Art Snake <art-snake@yandex-team.ru>
> Reviewed-by: dsinclair <dsinclair@chromium.org>

TBR=tsepez@chromium.org,dsinclair@chromium.org,art-snake@yandex-team.ru

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I947fca17052765935a952a4f25ca48f6599c4af9
Reviewed-on: https://pdfium-review.googlesource.com/37210
Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
Commit-Queue: Henrique Nakashima <hnakashima@chromium.org>

5 files changed, 13 insertions, 65 deletions

diff --git a/core/fpdfapi/edit/cpdf_flateencoder.cpp b/core/fpdfapi/edit/cpdf_flateencoder.cpp
index 573c141ff2..a290da6dac 100644
--- a/core/fpdfapi/edit/cpdf_flateencoder.cpp
+++ b/core/fpdfapi/edit/cpdf_flateencoder.cpp
@@ -15,7 +15,7 @@
 
 CPDF_FlateEncoder::CPDF_FlateEncoder(const CPDF_Stream* pStream,
                                      bool bFlateEncode)
-    : m_pAcc(pdfium::MakeRetain<CPDF_StreamAcc>(pStream)), m_dwSize(0) {
+    : m_dwSize(0), m_pAcc(pdfium::MakeRetain<CPDF_StreamAcc>(pStream)) {
   m_pAcc->LoadAllDataRaw();
 
   bool bHasFilter = pStream && pStream->HasFilter();
diff --git a/core/fpdfapi/edit/cpdf_flateencoder.h b/core/fpdfapi/edit/cpdf_flateencoder.h
index 14ca7cec16..05633f6814 100644
--- a/core/fpdfapi/edit/cpdf_flateencoder.h
+++ b/core/fpdfapi/edit/cpdf_flateencoder.h
@@ -34,14 +34,14 @@ class CPDF_FlateEncoder {
   }
 
  private:
-  RetainPtr<CPDF_StreamAcc> m_pAcc;
-
   uint32_t m_dwSize;
   MaybeOwned<uint8_t, FxFreeDeleter> m_pData;
 
   // Only one of these two pointers is valid at any time.
   UnownedPtr<const CPDF_Dictionary> m_pDict;
   std::unique_ptr<CPDF_Dictionary> m_pClonedDict;
+
+  RetainPtr<CPDF_StreamAcc> m_pAcc;
 };
 
 #endif  // CORE_FPDFAPI_EDIT_CPDF_FLATEENCODER_H_
diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.cpp b/core/fpdfapi/parser/cpdf_syntax_parser.cpp
index f8f36aee11..ec5ac70a29 100644
--- a/core/fpdfapi/parser/cpdf_syntax_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_syntax_parser.cpp
@@ -34,37 +34,6 @@ namespace {
 
 enum class ReadStatus { Normal, Backslash, Octal, FinishOctal, CarriageReturn };
 
-class ReadableSubStream : public IFX_SeekableReadStream {
- public:
-  ReadableSubStream(const RetainPtr<IFX_SeekableReadStream>& pFileRead,
-                    FX_FILESIZE part_offset,
-                    FX_FILESIZE part_size)
-      : m_pFileRead(pFileRead),
-        m_PartOffset(part_offset),
-        m_PartSize(part_size) {}
-
-  ~ReadableSubStream() override = default;
-
-  // IFX_SeekableReadStream overrides:
-  bool ReadBlock(void* buffer, FX_FILESIZE offset, size_t size) override {
-    FX_SAFE_FILESIZE safe_end = offset;
-    safe_end += size;
-    // Check that requested range is valid, to prevent calling of ReadBlock
-    // of original m_pFileRead with incorrect params.
-    if (!safe_end.IsValid() || safe_end.ValueOrDie() > m_PartSize)
-      return false;
-
-    return m_pFileRead->ReadBlock(buffer, m_PartOffset + offset, size);
-  }
-
-  FX_FILESIZE GetSize() override { return m_PartSize; }
-
- private:
-  RetainPtr<IFX_SeekableReadStream> m_pFileRead;
-  FX_FILESIZE m_PartOffset;
-  FX_FILESIZE m_PartSize;
-};
-
 }  // namespace
 
 // static
@@ -608,6 +577,7 @@ std::unique_ptr<CPDF_Stream> CPDF_SyntaxParser::ReadStream(
   ToNextLine();
   const FX_FILESIZE streamStartPos = GetPos();
 
+  std::unique_ptr<uint8_t, FxFreeDeleter> pData;
   if (len > 0) {
     FX_SAFE_FILESIZE pos = GetPos();
     pos += len;
@@ -615,18 +585,12 @@ std::unique_ptr<CPDF_Stream> CPDF_SyntaxParser::ReadStream(
       len = -1;
   }
 
-  RetainPtr<IFX_SeekableReadStream> data;
   if (len > 0) {
-    // Check data availability first to allow the Validator to request data
+    pData.reset(FX_Alloc(uint8_t, len));
+    // We should try read data first to allow the Validator to request data
     // smoothly, without jumps.
-    if (!GetValidator()->CheckDataRangeAndRequestIfUnavailable(
-            m_HeaderOffset + GetPos(), len)) {
+    if (!ReadBlock(pData.get(), len))
       return nullptr;
-    }
-
-    data = pdfium::MakeRetain<ReadableSubStream>(
-        GetValidator(), m_HeaderOffset + GetPos(), len);
-    SetPos(GetPos() + len);
   }
 
   const ByteStringView kEndStreamStr("endstream");
@@ -647,7 +611,7 @@ std::unique_ptr<CPDF_Stream> CPDF_SyntaxParser::ReadStream(
     // specified length, it signals the end of stream.
     if (memcmp(m_WordBuffer, kEndStreamStr.raw_str(),
                kEndStreamStr.GetLength()) != 0) {
-      data.Reset();
+      pData.reset();
       len = -1;
       SetPos(streamStartPos);
     }
@@ -664,27 +628,14 @@ std::unique_ptr<CPDF_Stream> CPDF_SyntaxParser::ReadStream(
     ASSERT(len >= 0);
     if (len > 0) {
       SetPos(streamStartPos);
-      // Check data availability first to allow the Validator to request data
-      // smoothly, without jumps.
-      if (!GetValidator()->CheckDataRangeAndRequestIfUnavailable(
-              m_HeaderOffset + GetPos(), len)) {
+      pData.reset(FX_Alloc(uint8_t, len));
+      if (!ReadBlock(pData.get(), len))
         return nullptr;
-      }
-
-      data = pdfium::MakeRetain<ReadableSubStream>(
-          GetValidator(), m_HeaderOffset + GetPos(), len);
-      SetPos(GetPos() + len);
     }
   }
 
-  auto pStream = pdfium::MakeUnique<CPDF_Stream>();
-  if (data) {
-    pStream->InitStreamFromFile(data, std::move(pDict));
-  } else {
-    DCHECK(!len);
-    // Empty stream
-    pStream->InitStream(nullptr, 0, std::move(pDict));
-  }
+  auto pStream =
+      pdfium::MakeUnique<CPDF_Stream>(std::move(pData), len, std::move(pDict));
   const FX_FILESIZE end_stream_offset = GetPos();
   memset(m_WordBuffer, 0, kEndObjStr.GetLength() + 1);
   GetNextWordInternal(nullptr);
diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp
index e874640218..e4ac4ad2c7 100644
--- a/testing/embedder_test.cpp
+++ b/testing/embedder_test.cpp
@@ -351,9 +351,7 @@ FPDF_DOCUMENT EmbedderTest::OpenSavedDocument(const char* password) {
   memset(&saved_file_access_, 0, sizeof(saved_file_access_));
   saved_file_access_.m_FileLen = data_string_.size();
   saved_file_access_.m_GetBlock = GetBlockFromString;
-  // Copy data to prevent clearing it before saved document close.
-  saved_document_file_data_ = data_string_;
-  saved_file_access_.m_Param = &saved_document_file_data_;
+  saved_file_access_.m_Param = &data_string_;
 
   saved_fake_file_access_ =
       pdfium::MakeUnique<FakeFileAccess>(&saved_file_access_);
diff --git a/testing/embedder_test.h b/testing/embedder_test.h
index b0dada0341..e8f76c058f 100644
--- a/testing/embedder_test.h
+++ b/testing/embedder_test.h
@@ -260,7 +260,6 @@ class EmbedderTest : public ::testing::Test,
   int GetPageNumberForSavedPage(FPDF_PAGE page) const;
 
   std::string data_string_;
-  std::string saved_document_file_data_;
   std::ofstream filestream_;
 };