Remove CPDF_Object::GetConstString and overrides

GetConstString() has sharp edges in that when applied to
a CPDF_Number, it must return null whereas GetString() returns
a the stringified number, because of the inability to
control the lifetime of the underlying allocated string.

Deleting this method showed several places where we
actually wanted a *String, not a *StringC, so we were
re-allocating a string we already had.

Review URL: https://codereview.chromium.org/1879683002

20 files changed, 21 insertions, 94 deletions

diff --git a/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp b/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp
index 2dfccb26d7..6cd66185c6 100644
--- a/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp
+++ b/core/fpdfapi/fpdf_page/fpdf_page_doc.cpp
@@ -326,7 +326,7 @@ CPDF_ColorSpace* CPDF_DocPageData::GetColorSpace(
     return nullptr;
 
   if (pCSObj->IsName()) {
-    CFX_ByteString name = pCSObj->GetConstString();
+    CFX_ByteString name = pCSObj->GetString();
     CPDF_ColorSpace* pCS = CPDF_ColorSpace::ColorspaceFromName(name);
     if (!pCS && pResources) {
       CPDF_Dictionary* pList = pResources->GetDictBy("ColorSpace");
diff --git a/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp b/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
index ba2901df24..654c9350a9 100644
--- a/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
+++ b/core/fpdfapi/fpdf_page/fpdf_page_parser.cpp
@@ -736,9 +736,10 @@ void CPDF_StreamContentParser::Handle_ExecuteXObject() {
     return;
   }
 
-  CFX_ByteStringC type = pXObject->GetDict()
-                             ? pXObject->GetDict()->GetConstStringBy("Subtype")
-                             : CFX_ByteStringC();
+  CFX_ByteString type;
+  if (pXObject->GetDict())
+    type = pXObject->GetDict()->GetStringBy("Subtype");
+
   if (type == "Image") {
     if (m_Options.m_bTextOnly) {
       return;
diff --git a/core/fpdfapi/fpdf_parser/cpdf_array.cpp b/core/fpdfapi/fpdf_parser/cpdf_array.cpp
index e2279c8e1d..d588f339a7 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_array.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_array.cpp
@@ -92,12 +92,6 @@ CFX_ByteString CPDF_Array::GetStringAt(size_t i) const {
   return m_Objects.at(i)->GetString();
 }
 
-CFX_ByteStringC CPDF_Array::GetConstStringAt(size_t i) const {
-  if (i >= m_Objects.size())
-    return CFX_ByteStringC();
-  return m_Objects.at(i)->GetConstString();
-}
-
 int CPDF_Array::GetIntegerAt(size_t i) const {
   if (i >= m_Objects.size())
     return 0;
diff --git a/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp b/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp
index 5f86a7bbc2..f03b14d8e1 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp
@@ -68,12 +68,6 @@ CFX_ByteString CPDF_Dictionary::GetStringBy(const CFX_ByteStringC& key) const {
   return p ? p->GetString() : CFX_ByteString();
 }
 
-CFX_ByteStringC CPDF_Dictionary::GetConstStringBy(
-    const CFX_ByteStringC& key) const {
-  CPDF_Object* p = GetObjectBy(key);
-  return p ? p->GetConstString() : CFX_ByteStringC();
-}
-
 CFX_WideString CPDF_Dictionary::GetUnicodeTextBy(
     const CFX_ByteStringC& key) const {
   CPDF_Object* p = GetObjectBy(key);
@@ -88,13 +82,6 @@ CFX_ByteString CPDF_Dictionary::GetStringBy(const CFX_ByteStringC& key,
   return p ? p->GetString() : CFX_ByteString(def);
 }
 
-CFX_ByteStringC CPDF_Dictionary::GetConstStringBy(
-    const CFX_ByteStringC& key,
-    const CFX_ByteStringC& def) const {
-  CPDF_Object* p = GetObjectBy(key);
-  return p ? p->GetConstString() : CFX_ByteStringC(def);
-}
-
 int CPDF_Dictionary::GetIntegerBy(const CFX_ByteStringC& key) const {
   CPDF_Object* p = GetObjectBy(key);
   return p ? p->GetInteger() : 0;
diff --git a/core/fpdfapi/fpdf_parser/cpdf_name.cpp b/core/fpdfapi/fpdf_parser/cpdf_name.cpp
index d641374547..2999772aec 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_name.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_name.cpp
@@ -28,10 +28,6 @@ CFX_ByteString CPDF_Name::GetString() const {
   return m_Name;
 }
 
-CFX_ByteStringC CPDF_Name::GetConstString() const {
-  return m_Name.AsStringC();
-}
-
 void CPDF_Name::SetString(const CFX_ByteString& str) {
   m_Name = str;
 }
diff --git a/core/fpdfapi/fpdf_parser/cpdf_object.cpp b/core/fpdfapi/fpdf_parser/cpdf_object.cpp
index 9a190a7308..86b9449cf8 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_object.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_object.cpp
@@ -33,10 +33,6 @@ CFX_ByteString CPDF_Object::GetString() const {
   return CFX_ByteString();
 }
 
-CFX_ByteStringC CPDF_Object::GetConstString() const {
-  return CFX_ByteStringC();
-}
-
 CFX_WideString CPDF_Object::GetUnicodeText() const {
   return CFX_WideString();
 }
diff --git a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
index 362991f723..3d0947d643 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp
@@ -34,7 +34,6 @@ void TestArrayAccessors(const CPDF_Array* arr,
                         CPDF_Dictionary* dict_val,
                         CPDF_Stream* stream_val) {
   EXPECT_STREQ(str_val, arr->GetStringAt(index).c_str());
-  EXPECT_STREQ(const_str_val, arr->GetConstStringAt(index).c_str());
   EXPECT_EQ(int_val, arr->GetIntegerAt(index));
   EXPECT_EQ(float_val, arr->GetNumberAt(index));
   EXPECT_EQ(float_val, arr->GetFloatAt(index));
@@ -199,33 +198,6 @@ TEST_F(PDFObjectsTest, GetString) {
   }
 }
 
-TEST_F(PDFObjectsTest, GetConstString) {
-  const char* direct_obj_results[] = {
-      nullptr, nullptr, nullptr, nullptr, "A simple test", "\t\n",
-      "space", nullptr, nullptr, nullptr, nullptr};
-  // Check for direct objects.
-  for (size_t i = 0; i < m_DirectObjs.size(); ++i) {
-    if (!direct_obj_results[i]) {
-      EXPECT_EQ(direct_obj_results[i],
-                m_DirectObjs[i]->GetConstString().c_str());
-    } else {
-      EXPECT_STREQ(direct_obj_results[i],
-                   m_DirectObjs[i]->GetConstString().c_str());
-    }
-  }
-  // Check indirect references.
-  const char* indirect_obj_results[] = {nullptr, nullptr, "\t\n", "space",
-                                        nullptr, nullptr, nullptr};
-  for (size_t i = 0; i < m_RefObjs.size(); ++i) {
-    if (!indirect_obj_results[i]) {
-      EXPECT_EQ(nullptr, m_RefObjs[i]->GetConstString().c_str());
-    } else {
-      EXPECT_STREQ(indirect_obj_results[i],
-                   m_RefObjs[i]->GetConstString().c_str());
-    }
-  }
-}
-
 TEST_F(PDFObjectsTest, GetUnicodeText) {
   const wchar_t* direct_obj_results[] = {
       L"",     L"",      L"", L"", L"A simple test",
@@ -668,16 +640,12 @@ TEST(PDFArrayTest, GetTypeAt) {
     const char* const expected_str[] = {
         "true",          "false", "0",    "-1234", "2345", "0.05", "",
         "It is a test!", "NAME",  "test", "",      "",     "",     ""};
-    const char* const expected_cstr[] = {
-        nullptr,         nullptr, nullptr, nullptr, nullptr, nullptr, nullptr,
-        "It is a test!", "NAME",  "test",  nullptr, nullptr, nullptr, nullptr};
     const int expected_int[] = {1, 0, 0, -1234, 2345, 0, 0,
                                 0, 0, 0, 0,     0,    0, 0};
     const float expected_float[] = {0, 0, 0, -1234, 2345, 0.05f, 0,
                                     0, 0, 0, 0,     0,    0,     0};
     for (size_t i = 0; i < arr->GetCount(); ++i) {
       EXPECT_STREQ(expected_str[i], arr->GetStringAt(i).c_str());
-      EXPECT_STREQ(expected_cstr[i], arr->GetConstStringAt(i).c_str());
       EXPECT_EQ(expected_int[i], arr->GetIntegerAt(i));
       EXPECT_EQ(expected_float[i], arr->GetNumberAt(i));
       EXPECT_EQ(expected_float[i], arr->GetFloatAt(i));
diff --git a/core/fpdfapi/fpdf_parser/cpdf_reference.cpp b/core/fpdfapi/fpdf_parser/cpdf_reference.cpp
index 9053d08b36..007423e5fb 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_reference.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_reference.cpp
@@ -22,11 +22,6 @@ CFX_ByteString CPDF_Reference::GetString() const {
   return obj ? obj->GetString() : CFX_ByteString();
 }
 
-CFX_ByteStringC CPDF_Reference::GetConstString() const {
-  CPDF_Object* obj = SafeGetDirect();
-  return obj ? obj->GetConstString() : CFX_ByteStringC();
-}
-
 FX_FLOAT CPDF_Reference::GetNumber() const {
   CPDF_Object* obj = SafeGetDirect();
   return obj ? obj->GetNumber() : 0;
diff --git a/core/fpdfapi/fpdf_parser/cpdf_string.cpp b/core/fpdfapi/fpdf_parser/cpdf_string.cpp
index 52f6a5a836..4b92596242 100644
--- a/core/fpdfapi/fpdf_parser/cpdf_string.cpp
+++ b/core/fpdfapi/fpdf_parser/cpdf_string.cpp
@@ -31,10 +31,6 @@ CFX_ByteString CPDF_String::GetString() const {
   return m_String;
 }
 
-CFX_ByteStringC CPDF_String::GetConstString() const {
-  return m_String.AsStringC();
-}
-
 void CPDF_String::SetString(const CFX_ByteString& str) {
   m_String = str;
 }
diff --git a/core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp b/core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp
index 11d9e524ee..f81f41c89f 100644
--- a/core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp
+++ b/core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp
@@ -350,11 +350,11 @@ FX_BOOL PDF_DataDecode(const uint8_t* src_buf,
       pParams = nullptr;
 
     for (size_t i = 0; i < pDecoders->GetCount(); i++) {
-      DecoderList.push_back(pDecoders->GetConstStringAt(i));
+      DecoderList.push_back(pDecoders->GetStringAt(i));
       ParamList.Add(pParams ? pParamsArray->GetDictAt(i) : nullptr);
     }
   } else {
-    DecoderList.push_back(pDecoder->GetConstString());
+    DecoderList.push_back(pDecoder->GetString());
     ParamList.Add(pParams ? pParams->GetDict() : nullptr);
   }
   uint8_t* last_buf = (uint8_t*)src_buf;
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_array.h b/core/fpdfapi/fpdf_parser/include/cpdf_array.h
index 506a6bc338..ac3bf41b45 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_array.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_array.h
@@ -30,7 +30,6 @@ class CPDF_Array : public CPDF_Object {
   CPDF_Object* GetObjectAt(size_t index) const;
   CPDF_Object* GetDirectObjectAt(size_t index) const;
   CFX_ByteString GetStringAt(size_t index) const;
-  CFX_ByteStringC GetConstStringAt(size_t index) const;
   int GetIntegerAt(size_t index) const;
   FX_FLOAT GetNumberAt(size_t index) const;
   CPDF_Dictionary* GetDictAt(size_t index) const;
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h b/core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h
index 82abc35c02..c39370e6ec 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h
@@ -34,11 +34,8 @@ class CPDF_Dictionary : public CPDF_Object {
   CPDF_Object* GetObjectBy(const CFX_ByteStringC& key) const;
   CPDF_Object* GetDirectObjectBy(const CFX_ByteStringC& key) const;
   CFX_ByteString GetStringBy(const CFX_ByteStringC& key) const;
-  CFX_ByteStringC GetConstStringBy(const CFX_ByteStringC& key) const;
   CFX_ByteString GetStringBy(const CFX_ByteStringC& key,
                              const CFX_ByteStringC& default_str) const;
-  CFX_ByteStringC GetConstStringBy(const CFX_ByteStringC& key,
-                                   const CFX_ByteStringC& default_str) const;
   CFX_WideString GetUnicodeTextBy(const CFX_ByteStringC& key) const;
   int GetIntegerBy(const CFX_ByteStringC& key) const;
   int GetIntegerBy(const CFX_ByteStringC& key, int default_int) const;
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_name.h b/core/fpdfapi/fpdf_parser/include/cpdf_name.h
index 62fd5c14a8..bebccb5841 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_name.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_name.h
@@ -19,7 +19,6 @@ class CPDF_Name : public CPDF_Object {
   Type GetType() const override;
   CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override;
   CFX_ByteString GetString() const override;
-  CFX_ByteStringC GetConstString() const override;
   CFX_WideString GetUnicodeText() const override;
   void SetString(const CFX_ByteString& str) override;
   bool IsName() const override;
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_object.h b/core/fpdfapi/fpdf_parser/include/cpdf_object.h
index 6cdaaf58fd..c0c442157f 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_object.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_object.h
@@ -46,9 +46,6 @@ class CPDF_Object {
   void Release();
 
   virtual CFX_ByteString GetString() const;
-
-  // Note: |this| must outlive the use of |GetConstString|'s result.
-  virtual CFX_ByteStringC GetConstString() const;
   virtual CFX_WideString GetUnicodeText() const;
   virtual FX_FLOAT GetNumber() const;
   virtual int GetInteger() const;
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_reference.h b/core/fpdfapi/fpdf_parser/include/cpdf_reference.h
index b7d826902b..49b698eacb 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_reference.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_reference.h
@@ -20,7 +20,6 @@ class CPDF_Reference : public CPDF_Object {
   CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override;
   CPDF_Object* GetDirect() const override;
   CFX_ByteString GetString() const override;
-  CFX_ByteStringC GetConstString() const override;
   FX_FLOAT GetNumber() const override;
   int GetInteger() const override;
   CPDF_Dictionary* GetDict() const override;
diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_string.h b/core/fpdfapi/fpdf_parser/include/cpdf_string.h
index 77e41ecd6a..c17cc182f7 100644
--- a/core/fpdfapi/fpdf_parser/include/cpdf_string.h
+++ b/core/fpdfapi/fpdf_parser/include/cpdf_string.h
@@ -21,7 +21,6 @@ class CPDF_String : public CPDF_Object {
   Type GetType() const override;
   CPDF_Object* Clone(FX_BOOL bDirect = FALSE) const override;
   CFX_ByteString GetString() const override;
-  CFX_ByteStringC GetConstString() const override;
   CFX_WideString GetUnicodeText() const override;
   void SetString(const CFX_ByteString& str) override;
   bool IsString() const override;
diff --git a/core/fpdfapi/fpdf_render/fpdf_render_image.cpp b/core/fpdfapi/fpdf_render/fpdf_render_image.cpp
index 6aee8e75b6..55e97a9d81 100644
--- a/core/fpdfapi/fpdf_render/fpdf_render_image.cpp
+++ b/core/fpdfapi/fpdf_render/fpdf_render_image.cpp
@@ -412,13 +412,13 @@ FX_BOOL CPDF_ImageRenderer::StartRenderDIBSource() {
             "Filter");
     if (pFilters) {
       if (pFilters->IsName()) {
-        CFX_ByteStringC bsDecodeType = pFilters->GetConstString();
+        CFX_ByteString bsDecodeType = pFilters->GetString();
         if (bsDecodeType == "DCTDecode" || bsDecodeType == "JPXDecode") {
           m_Flags |= FXRENDER_IMAGE_LOSSY;
         }
       } else if (CPDF_Array* pArray = pFilters->AsArray()) {
         for (size_t i = 0; i < pArray->GetCount(); i++) {
-          CFX_ByteStringC bsDecodeType = pArray->GetConstStringAt(i);
+          CFX_ByteString bsDecodeType = pArray->GetStringAt(i);
           if (bsDecodeType == "DCTDecode" || bsDecodeType == "JPXDecode") {
             m_Flags |= FXRENDER_IMAGE_LOSSY;
             break;
@@ -880,10 +880,6 @@ CFX_DIBitmap* CPDF_RenderStatus::LoadSMask(CPDF_Dictionary* pSMaskDict,
   if (!pSMaskDict) {
     return NULL;
   }
-  int width = pClipRect->right - pClipRect->left;
-  int height = pClipRect->bottom - pClipRect->top;
-  FX_BOOL bLuminosity = FALSE;
-  bLuminosity = pSMaskDict->GetConstStringBy("S") != "Alpha";
   CPDF_Stream* pGroup = pSMaskDict->GetStreamBy("G");
   if (!pGroup) {
     return NULL;
@@ -895,10 +891,15 @@ CFX_DIBitmap* CPDF_RenderStatus::LoadSMask(CPDF_Dictionary* pSMaskDict,
 
   CFX_Matrix matrix = *pMatrix;
   matrix.TranslateI(-pClipRect->left, -pClipRect->top);
+
   CPDF_Form form(m_pContext->GetDocument(), m_pContext->GetPageResources(),
                  pGroup);
   form.ParseContent(NULL, NULL, NULL, NULL);
+
   CFX_FxgeDevice bitmap_device;
+  FX_BOOL bLuminosity = pSMaskDict->GetStringBy("S") != "Alpha";
+  int width = pClipRect->right - pClipRect->left;
+  int height = pClipRect->bottom - pClipRect->top;
 #if _FXM_PLATFORM_ == _FXM_PLATFORM_APPLE_
   if (!bitmap_device.Create(width, height,
                             bLuminosity ? FXDIB_Rgb32 : FXDIB_8bppMask)) {
diff --git a/core/fpdfdoc/cpvt_generateap.cpp b/core/fpdfdoc/cpvt_generateap.cpp
index a837c35801..7b4ad2ff4f 100644
--- a/core/fpdfdoc/cpvt_generateap.cpp
+++ b/core/fpdfdoc/cpvt_generateap.cpp
@@ -451,7 +451,7 @@ FX_BOOL GenerateWidgetAP(CPDF_Document* pDoc,
 }  // namespace
 
 FX_BOOL FPDF_GenerateAP(CPDF_Document* pDoc, CPDF_Dictionary* pAnnotDict) {
-  if (!pAnnotDict || pAnnotDict->GetConstStringBy("Subtype") != "Widget") {
+  if (!pAnnotDict || pAnnotDict->GetStringBy("Subtype") != "Widget") {
     return FALSE;
   }
   CFX_ByteString field_type = FPDF_GetFieldAttr(pAnnotDict, "FT")->GetString();
diff --git a/core/fpdfdoc/doc_annot.cpp b/core/fpdfdoc/doc_annot.cpp
index 8650fbd23d..3b90b0b4b5 100644
--- a/core/fpdfdoc/doc_annot.cpp
+++ b/core/fpdfdoc/doc_annot.cpp
@@ -43,7 +43,7 @@ CPDF_AnnotList::CPDF_AnnotList(CPDF_Page* pPage)
       pDict = pAnnots->GetDictAt(i);
     }
     m_AnnotList.push_back(new CPDF_Annot(pDict, this));
-    if (bRegenerateAP && pDict->GetConstStringBy("Subtype") == "Widget" &&
+    if (bRegenerateAP && pDict->GetStringBy("Subtype") == "Widget" &&
         CPDF_InterForm::UpdatingAPEnabled()) {
       FPDF_GenerateAP(m_pDocument, pDict);
     }
@@ -127,10 +127,12 @@ void CPDF_AnnotList::DisplayAnnots(CPDF_Page* pPage,
 CPDF_Annot::CPDF_Annot(CPDF_Dictionary* pDict, CPDF_AnnotList* pList)
     : m_pAnnotDict(pDict),
       m_pList(pList),
-      m_sSubtype(m_pAnnotDict->GetConstStringBy("Subtype")) {}
+      m_sSubtype(m_pAnnotDict->GetStringBy("Subtype")) {}
+
 CPDF_Annot::~CPDF_Annot() {
   ClearCachedAP();
 }
+
 void CPDF_Annot::ClearCachedAP() {
   for (const auto& pair : m_APMap) {
     delete pair.second;
diff --git a/core/fpdfdoc/doc_basic.cpp b/core/fpdfdoc/doc_basic.cpp
index 1cfca7e8ee..e0b71df392 100644
--- a/core/fpdfdoc/doc_basic.cpp
+++ b/core/fpdfdoc/doc_basic.cpp
@@ -316,7 +316,8 @@ bool CPDF_FileSpec::GetFileName(CFX_WideString* csFileName) const {
   if (CPDF_Dictionary* pDict = m_pObj->AsDictionary()) {
     *csFileName = pDict->GetUnicodeTextBy("UF");
     if (csFileName->IsEmpty()) {
-      *csFileName = CFX_WideString::FromLocal(pDict->GetConstStringBy("F"));
+      *csFileName =
+          CFX_WideString::FromLocal(pDict->GetStringBy("F").AsStringC());
     }
     if (pDict->GetStringBy("FS") == "URL")
       return true;