Removing CPDF_Parser::CloseParser.chromium/2839

Currently the only calls to CloseParser() happend in the destructor or the
start*Parse methods. The Start*Parse methods are currently only called on
freshly constructed parsers in fpdf_dataavail and fpdfview.

This CL removes the CloseParser() method and puts the contents in the
destructor. We then add an ASSERT that we don't re-enter the parser after it
has already completed the parse.

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

2 files changed, 54 insertions, 62 deletions

diff --git a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
index e2bab450be..9d26104caa 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
@@ -51,7 +51,8 @@ int32_t GetStreamFirst(CPDF_StreamAcc* pObjStream) {
 }  // namespace
 
 CPDF_Parser::CPDF_Parser()
-    : m_bOwnFileRead(true),
+    : m_bHasParsed(false),
+      m_bOwnFileRead(true),
       m_FileVersion(0),
       m_pTrailer(nullptr),
       m_pEncryptDict(nullptr),
@@ -63,7 +64,25 @@ CPDF_Parser::CPDF_Parser()
 }
 
 CPDF_Parser::~CPDF_Parser() {
-  CloseParser();
+  if (m_pTrailer)
+    m_pTrailer->Release();
+
+  ReleaseEncryptHandler();
+  SetEncryptDictionary(nullptr);
+
+  if (m_bOwnFileRead && m_pSyntax->m_pFileAccess) {
+    m_pSyntax->m_pFileAccess->Release();
+    m_pSyntax->m_pFileAccess = nullptr;
+  }
+
+  int32_t iLen = m_Trailers.GetSize();
+  for (int32_t i = 0; i < iLen; ++i) {
+    if (CPDF_Dictionary* trailer = m_Trailers.GetAt(i))
+      trailer->Release();
+  }
+
+  if (m_pLinearized)
+    m_pLinearized->Release();
 }
 
 uint32_t CPDF_Parser::GetLastObjNum() const {
@@ -124,43 +143,10 @@ void CPDF_Parser::ShrinkObjectMap(uint32_t objnum) {
     m_ObjectInfo[objnum - 1].pos = 0;
 }
 
-void CPDF_Parser::CloseParser() {
-  m_bVersionUpdated = false;
-  m_pDocument = nullptr;
-
-  if (m_pTrailer) {
-    m_pTrailer->Release();
-    m_pTrailer = nullptr;
-  }
-  ReleaseEncryptHandler();
-  SetEncryptDictionary(nullptr);
-
-  if (m_bOwnFileRead && m_pSyntax->m_pFileAccess) {
-    m_pSyntax->m_pFileAccess->Release();
-    m_pSyntax->m_pFileAccess = nullptr;
-  }
-
-  m_ObjectStreamMap.clear();
-  m_ObjCache.clear();
-  m_SortedOffset.clear();
-  m_ObjectInfo.clear();
-
-  int32_t iLen = m_Trailers.GetSize();
-  for (int32_t i = 0; i < iLen; ++i) {
-    if (CPDF_Dictionary* trailer = m_Trailers.GetAt(i))
-      trailer->Release();
-  }
-  m_Trailers.RemoveAll();
-
-  if (m_pLinearized) {
-    m_pLinearized->Release();
-    m_pLinearized = nullptr;
-  }
-}
-
 CPDF_Parser::Error CPDF_Parser::StartParse(IFX_FileRead* pFileAccess,
                                            CPDF_Document* pDocument) {
-  CloseParser();
+  ASSERT(!m_bHasParsed);
+  m_bHasParsed = true;
 
   m_bXRefStream = FALSE;
   m_LastXRefOffset = 0;
@@ -1550,7 +1536,8 @@ FX_BOOL CPDF_Parser::IsLinearizedFile(IFX_FileRead* pFileAccess,
 
 CPDF_Parser::Error CPDF_Parser::StartLinearizedParse(IFX_FileRead* pFileAccess,
                                                      CPDF_Document* pDocument) {
-  CloseParser();
+  ASSERT(!m_bHasParsed);
+
   m_bXRefStream = FALSE;
   m_LastXRefOffset = 0;
   m_bOwnFileRead = true;
@@ -1563,8 +1550,9 @@ CPDF_Parser::Error CPDF_Parser::StartLinearizedParse(IFX_FileRead* pFileAccess,
     m_pSyntax->m_pFileAccess = nullptr;
     return StartParse(pFileAccess, std::move(pDocument));
   }
-
+  m_bHasParsed = true;
   m_pDocument = pDocument;
+
   FX_FILESIZE dwFirstXRefOffset = m_pSyntax->SavePos();
 
   FX_BOOL bXRefRebuilt = FALSE;
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_parser.h b/core/fpdfapi/fpdf_parser/include/cpdf_parser.h
index d6a5d5703b..3d2408fad1 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_parser.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_parser.h
@@ -95,14 +95,37 @@ class CPDF_Parser {
     uint16_t gennum;
   };
 
-  void CloseParser();
+  std::unique_ptr<CPDF_SyntaxParser> m_pSyntax;
+  std::map<uint32_t, ObjectInfo> m_ObjectInfo;
+
+  bool LoadCrossRefV4(FX_FILESIZE pos, FX_FILESIZE streampos, FX_BOOL bSkip);
+  FX_BOOL RebuildCrossRef();
+
+ private:
+  friend class CPDF_DataAvail;
+
+  enum class ParserState {
+    kDefault,
+    kComment,
+    kWhitespace,
+    kString,
+    kHexString,
+    kEscapedString,
+    kXref,
+    kObjNum,
+    kPostObjNum,
+    kGenNum,
+    kPostGenNum,
+    kTrailer,
+    kBeginObj,
+    kEndObj
+  };
+
   CPDF_Object* ParseDirect(CPDF_Object* pObj);
   FX_BOOL LoadAllCrossRefV4(FX_FILESIZE pos);
   FX_BOOL LoadAllCrossRefV5(FX_FILESIZE pos);
-  bool LoadCrossRefV4(FX_FILESIZE pos, FX_FILESIZE streampos, FX_BOOL bSkip);
   FX_BOOL LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef);
   CPDF_Dictionary* LoadTrailerV4();
-  FX_BOOL RebuildCrossRef();
   Error SetEncryptHandler();
   void ReleaseEncryptHandler();
   FX_BOOL LoadLinearizedAllCrossRefV4(FX_FILESIZE pos, uint32_t dwObjCount);
@@ -118,7 +141,7 @@ class CPDF_Parser {
   bool VerifyCrossRefV4();
 
   CPDF_Document* m_pDocument;  // not owned
-  std::unique_ptr<CPDF_SyntaxParser> m_pSyntax;
+  bool m_bHasParsed;
   bool m_bOwnFileRead;
   int m_FileVersion;
   CPDF_Dictionary* m_pTrailer;
@@ -127,7 +150,6 @@ class CPDF_Parser {
   FX_BOOL m_bXRefStream;
   std::unique_ptr<CPDF_SecurityHandler> m_pSecurityHandler;
   CFX_ByteString m_Password;
-  std::map<uint32_t, ObjectInfo> m_ObjectInfo;
   std::set<FX_FILESIZE> m_SortedOffset;
   CFX_ArrayTemplate<CPDF_Dictionary*> m_Trailers;
   bool m_bVersionUpdated;
@@ -149,25 +171,7 @@ class CPDF_Parser {
   // All indirect object numbers that are being parsed.
   std::set<uint32_t> m_ParsingObjNums;
 
-  friend class CPDF_DataAvail;
 
- private:
-  enum class ParserState {
-    kDefault,
-    kComment,
-    kWhitespace,
-    kString,
-    kHexString,
-    kEscapedString,
-    kXref,
-    kObjNum,
-    kPostObjNum,
-    kGenNum,
-    kPostGenNum,
-    kTrailer,
-    kBeginObj,
-    kEndObj
-  };
 };
 
 #endif  // CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_PARSER_H_