From 822484b5f88d92dbcd567b1c2da38af1e720b56f Mon Sep 17 00:00:00 2001 From: tsepez Date: Mon, 11 Apr 2016 18:14:56 -0700 Subject: 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 --- core/fpdfapi/fpdf_page/fpdf_page_doc.cpp | 2 +- core/fpdfapi/fpdf_page/fpdf_page_parser.cpp | 7 +++-- core/fpdfapi/fpdf_parser/cpdf_array.cpp | 6 ---- core/fpdfapi/fpdf_parser/cpdf_dictionary.cpp | 13 --------- core/fpdfapi/fpdf_parser/cpdf_name.cpp | 4 --- core/fpdfapi/fpdf_parser/cpdf_object.cpp | 4 --- core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp | 32 ---------------------- core/fpdfapi/fpdf_parser/cpdf_reference.cpp | 5 ---- core/fpdfapi/fpdf_parser/cpdf_string.cpp | 4 --- core/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp | 4 +-- core/fpdfapi/fpdf_parser/include/cpdf_array.h | 1 - core/fpdfapi/fpdf_parser/include/cpdf_dictionary.h | 3 -- core/fpdfapi/fpdf_parser/include/cpdf_name.h | 1 - core/fpdfapi/fpdf_parser/include/cpdf_object.h | 3 -- core/fpdfapi/fpdf_parser/include/cpdf_reference.h | 1 - core/fpdfapi/fpdf_parser/include/cpdf_string.h | 1 - core/fpdfapi/fpdf_render/fpdf_render_image.cpp | 13 +++++---- core/fpdfdoc/cpvt_generateap.cpp | 2 +- core/fpdfdoc/doc_annot.cpp | 6 ++-- core/fpdfdoc/doc_basic.cpp | 3 +- 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; -- cgit v1.2.3