Fix retained argument TODOs, part 1.
Use retained arguments in CPDF_Bookmark, CPDF_IconFit, CPDF_FileSpec.
-- remove unused form of GetFileStream()
Change-Id: If54abf2c95a90a6ecc3c7cfd227330c548a69e06
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/98590
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfdoc/cpdf_action.cpp b/core/fpdfdoc/cpdf_action.cpp
index 42909b2..2517e42 100644
--- a/core/fpdfdoc/cpdf_action.cpp
+++ b/core/fpdfdoc/cpdf_action.cpp
@@ -71,7 +71,7 @@
RetainPtr<const CPDF_Object> pFile =
m_pDict->GetDirectObjectFor(pdfium::stream::kF);
if (pFile)
- return CPDF_FileSpec(pFile.Get()).GetFileName();
+ return CPDF_FileSpec(std::move(pFile)).GetFileName();
if (type != Type::kLaunch)
return WideString();
diff --git a/core/fpdfdoc/cpdf_apsettings.cpp b/core/fpdfdoc/cpdf_apsettings.cpp
index 9b9bd34..039708d 100644
--- a/core/fpdfdoc/cpdf_apsettings.cpp
+++ b/core/fpdfdoc/cpdf_apsettings.cpp
@@ -106,8 +106,7 @@
}
CPDF_IconFit CPDF_ApSettings::GetIconFit() const {
- // TODO(tsepez): pass retained object.
- return CPDF_IconFit(m_pDict ? m_pDict->GetDictFor("IF").Get() : nullptr);
+ return CPDF_IconFit(m_pDict ? m_pDict->GetDictFor("IF") : nullptr);
}
int CPDF_ApSettings::GetTextPosition() const {
diff --git a/core/fpdfdoc/cpdf_bookmark.cpp b/core/fpdfdoc/cpdf_bookmark.cpp
index 9de19c5..3c57f65 100644
--- a/core/fpdfdoc/cpdf_bookmark.cpp
+++ b/core/fpdfdoc/cpdf_bookmark.cpp
@@ -6,6 +6,8 @@
#include "core/fpdfdoc/cpdf_bookmark.h"
+#include <utility>
+
#include "core/fpdfapi/parser/cpdf_array.h"
#include "core/fpdfapi/parser/cpdf_dictionary.h"
#include "core/fpdfapi/parser/cpdf_string.h"
@@ -16,7 +18,8 @@
CPDF_Bookmark::CPDF_Bookmark(const CPDF_Bookmark& that) = default;
-CPDF_Bookmark::CPDF_Bookmark(const CPDF_Dictionary* pDict) : m_pDict(pDict) {}
+CPDF_Bookmark::CPDF_Bookmark(RetainPtr<const CPDF_Dictionary> pDict)
+ : m_pDict(std::move(pDict)) {}
CPDF_Bookmark::~CPDF_Bookmark() = default;
diff --git a/core/fpdfdoc/cpdf_bookmark.h b/core/fpdfdoc/cpdf_bookmark.h
index 1cdcf9b..f2c1719 100644
--- a/core/fpdfdoc/cpdf_bookmark.h
+++ b/core/fpdfdoc/cpdf_bookmark.h
@@ -19,7 +19,7 @@
public:
CPDF_Bookmark();
CPDF_Bookmark(const CPDF_Bookmark& that);
- explicit CPDF_Bookmark(const CPDF_Dictionary* pDict);
+ explicit CPDF_Bookmark(RetainPtr<const CPDF_Dictionary> pDict);
~CPDF_Bookmark();
const CPDF_Dictionary* GetDict() const { return m_pDict.Get(); }
diff --git a/core/fpdfdoc/cpdf_bookmarktree.cpp b/core/fpdfdoc/cpdf_bookmarktree.cpp
index 8f3ac78..0568154 100644
--- a/core/fpdfdoc/cpdf_bookmarktree.cpp
+++ b/core/fpdfdoc/cpdf_bookmarktree.cpp
@@ -6,6 +6,8 @@
#include "core/fpdfdoc/cpdf_bookmarktree.h"
+#include <utility>
+
#include "core/fpdfapi/parser/cpdf_dictionary.h"
#include "core/fpdfapi/parser/cpdf_document.h"
@@ -17,16 +19,15 @@
CPDF_Bookmark CPDF_BookmarkTree::GetFirstChild(
const CPDF_Bookmark& parent) const {
const CPDF_Dictionary* parent_dict = parent.GetDict();
- if (parent_dict) {
- // TODO(tsepez): pass retained argument.
- return CPDF_Bookmark(parent_dict->GetDictFor("First").Get());
- }
+ if (parent_dict)
+ return CPDF_Bookmark(parent_dict->GetDictFor("First"));
+
const CPDF_Dictionary* root = document_->GetRoot();
if (!root)
return CPDF_Bookmark();
RetainPtr<const CPDF_Dictionary> outlines = root->GetDictFor("Outlines");
- return outlines ? CPDF_Bookmark(outlines->GetDictFor("First").Get())
+ return outlines ? CPDF_Bookmark(outlines->GetDictFor("First"))
: CPDF_Bookmark();
}
@@ -37,5 +38,5 @@
return CPDF_Bookmark();
RetainPtr<const CPDF_Dictionary> next = dict->GetDictFor("Next");
- return next != dict ? CPDF_Bookmark(next.Get()) : CPDF_Bookmark();
+ return next != dict ? CPDF_Bookmark(std::move(next)) : CPDF_Bookmark();
}
diff --git a/core/fpdfdoc/cpdf_filespec.cpp b/core/fpdfdoc/cpdf_filespec.cpp
index 66de713..aa64ea7 100644
--- a/core/fpdfdoc/cpdf_filespec.cpp
+++ b/core/fpdfdoc/cpdf_filespec.cpp
@@ -7,6 +7,7 @@
#include "core/fpdfdoc/cpdf_filespec.h"
#include <iterator>
+#include <utility>
#include "build/build_config.h"
#include "constants/stream_dict_common.h"
@@ -56,7 +57,8 @@
} // namespace
-CPDF_FileSpec::CPDF_FileSpec(const CPDF_Object* pObj) : m_pObj(pObj) {
+CPDF_FileSpec::CPDF_FileSpec(RetainPtr<const CPDF_Object> pObj)
+ : m_pObj(std::move(pObj)) {
DCHECK(m_pObj);
}
@@ -125,7 +127,7 @@
return DecodeFileName(csFileName);
}
-const CPDF_Stream* CPDF_FileSpec::GetFileStream() const {
+RetainPtr<const CPDF_Stream> CPDF_FileSpec::GetFileStream() const {
const CPDF_Dictionary* pDict = m_pObj->AsDictionary();
if (!pDict)
return nullptr;
@@ -143,34 +145,25 @@
ByteString key = kKeys[i];
if (!pDict->GetUnicodeTextFor(key).IsEmpty()) {
RetainPtr<const CPDF_Stream> pStream = pFiles->GetStreamFor(key);
- if (pStream) {
- // TODO(tsepez): return retained object.
- return pStream.Get();
- }
+ if (pStream)
+ return pStream;
}
}
return nullptr;
}
-CPDF_Stream* CPDF_FileSpec::GetFileStream() {
- return const_cast<CPDF_Stream*>(
- static_cast<const CPDF_FileSpec*>(this)->GetFileStream());
-}
-
-// TODO(tsepez): return retained reference.
-const CPDF_Dictionary* CPDF_FileSpec::GetParamsDict() const {
+RetainPtr<const CPDF_Dictionary> CPDF_FileSpec::GetParamsDict() const {
const CPDF_Stream* pStream = GetFileStream();
if (!pStream)
return nullptr;
- // TODO(tsepez): return retained reference.
RetainPtr<const CPDF_Dictionary> pDict = pStream->GetDict();
- return pDict ? pDict->GetDictFor("Params").Get() : nullptr;
+ return pDict ? pDict->GetDictFor("Params") : nullptr;
}
-CPDF_Dictionary* CPDF_FileSpec::GetParamsDict() {
- return const_cast<CPDF_Dictionary*>(
- static_cast<const CPDF_FileSpec*>(this)->GetParamsDict());
+RetainPtr<CPDF_Dictionary> CPDF_FileSpec::GetMutableParamsDict() {
+ return pdfium::WrapRetain(
+ const_cast<CPDF_Dictionary*>(GetParamsDict().Get()));
}
WideString CPDF_FileSpec::EncodeFileName(const WideString& filepath) {
diff --git a/core/fpdfdoc/cpdf_filespec.h b/core/fpdfdoc/cpdf_filespec.h
index 7006f30..680253f 100644
--- a/core/fpdfdoc/cpdf_filespec.h
+++ b/core/fpdfdoc/cpdf_filespec.h
@@ -18,7 +18,7 @@
class CPDF_FileSpec {
public:
- explicit CPDF_FileSpec(const CPDF_Object* pObj);
+ explicit CPDF_FileSpec(RetainPtr<const CPDF_Object> pObj);
~CPDF_FileSpec();
// Convert a platform dependent file name into pdf format.
@@ -28,10 +28,9 @@
static WideString DecodeFileName(const WideString& filepath);
WideString GetFileName() const;
- const CPDF_Stream* GetFileStream() const;
- CPDF_Stream* GetFileStream();
- const CPDF_Dictionary* GetParamsDict() const;
- CPDF_Dictionary* GetParamsDict();
+ RetainPtr<const CPDF_Stream> GetFileStream() const;
+ RetainPtr<const CPDF_Dictionary> GetParamsDict() const;
+ RetainPtr<CPDF_Dictionary> GetMutableParamsDict();
private:
RetainPtr<const CPDF_Object> const m_pObj;
diff --git a/core/fpdfdoc/cpdf_filespec_unittest.cpp b/core/fpdfdoc/cpdf_filespec_unittest.cpp
index 689137a..a51f87e 100644
--- a/core/fpdfdoc/cpdf_filespec_unittest.cpp
+++ b/core/fpdfdoc/cpdf_filespec_unittest.cpp
@@ -74,7 +74,7 @@
#endif
};
auto str_obj = pdfium::MakeRetain<CPDF_String>(nullptr, test_data.input);
- CPDF_FileSpec file_spec(str_obj.Get());
+ CPDF_FileSpec file_spec(str_obj);
EXPECT_STREQ(test_data.expected, file_spec.GetFileName().c_str());
}
{
@@ -104,7 +104,7 @@
const char* const keywords[] = {"Unix", "Mac", "DOS", "F", "UF"};
static_assert(std::size(test_data) == std::size(keywords), "size mismatch");
auto dict_obj = pdfium::MakeRetain<CPDF_Dictionary>();
- CPDF_FileSpec file_spec(dict_obj.Get());
+ CPDF_FileSpec file_spec(dict_obj);
EXPECT_TRUE(file_spec.GetFileName().IsEmpty());
for (size_t i = 0; i < std::size(keywords); ++i) {
dict_obj->SetNewFor<CPDF_String>(keywords[i], test_data[i].input);
@@ -119,13 +119,13 @@
{
// Invalid object.
auto name_obj = pdfium::MakeRetain<CPDF_Name>(nullptr, "test.pdf");
- CPDF_FileSpec file_spec(name_obj.Get());
+ CPDF_FileSpec file_spec(name_obj);
EXPECT_TRUE(file_spec.GetFileName().IsEmpty());
}
{
// Invalid CPDF_Name objects in dictionary. See https://crbug.com/959183
auto dict_obj = pdfium::MakeRetain<CPDF_Dictionary>();
- CPDF_FileSpec file_spec(dict_obj.Get());
+ CPDF_FileSpec file_spec(dict_obj);
for (const char* key : {"Unix", "Mac", "DOS", "F", "UF"}) {
dict_obj->SetNewFor<CPDF_Name>(key, "http://evil.org");
EXPECT_TRUE(file_spec.GetFileName().IsEmpty());
@@ -139,27 +139,27 @@
{
// Invalid object.
auto name_obj = pdfium::MakeRetain<CPDF_Name>(nullptr, "test.pdf");
- CPDF_FileSpec file_spec(name_obj.Get());
+ CPDF_FileSpec file_spec(name_obj);
EXPECT_FALSE(file_spec.GetFileStream());
}
{
// Dictionary object missing its embedded files dictionary.
auto dict_obj = pdfium::MakeRetain<CPDF_Dictionary>();
- CPDF_FileSpec file_spec(dict_obj.Get());
+ CPDF_FileSpec file_spec(dict_obj);
EXPECT_FALSE(file_spec.GetFileStream());
}
{
// Dictionary object with an empty embedded files dictionary.
auto dict_obj = pdfium::MakeRetain<CPDF_Dictionary>();
dict_obj->SetNewFor<CPDF_Dictionary>("EF");
- CPDF_FileSpec file_spec(dict_obj.Get());
+ CPDF_FileSpec file_spec(dict_obj);
EXPECT_FALSE(file_spec.GetFileStream());
}
{
// Dictionary object with a non-empty embedded files dictionary.
auto dict_obj = pdfium::MakeRetain<CPDF_Dictionary>();
dict_obj->SetNewFor<CPDF_Dictionary>("EF");
- CPDF_FileSpec file_spec(dict_obj.Get());
+ CPDF_FileSpec file_spec(dict_obj);
const wchar_t file_name[] = L"test.pdf";
const char* const keys[] = {"Unix", "Mac", "DOS", "F", "UF"};
@@ -198,7 +198,7 @@
{
// Invalid object.
auto name_obj = pdfium::MakeRetain<CPDF_Name>(nullptr, "test.pdf");
- CPDF_FileSpec file_spec(name_obj.Get());
+ CPDF_FileSpec file_spec(name_obj);
EXPECT_FALSE(file_spec.GetParamsDict());
}
{
@@ -206,7 +206,7 @@
auto dict_obj = pdfium::MakeRetain<CPDF_Dictionary>();
dict_obj->SetNewFor<CPDF_Dictionary>("EF");
dict_obj->SetNewFor<CPDF_String>("UF", L"test.pdf");
- CPDF_FileSpec file_spec(dict_obj.Get());
+ CPDF_FileSpec file_spec(dict_obj);
EXPECT_FALSE(file_spec.GetParamsDict());
// Add a file stream to the embedded files dictionary.
diff --git a/core/fpdfdoc/cpdf_iconfit.cpp b/core/fpdfdoc/cpdf_iconfit.cpp
index d2154ef..30d52af 100644
--- a/core/fpdfdoc/cpdf_iconfit.cpp
+++ b/core/fpdfdoc/cpdf_iconfit.cpp
@@ -7,6 +7,7 @@
#include "core/fpdfdoc/cpdf_iconfit.h"
#include <algorithm>
+#include <utility>
#include "core/fpdfapi/parser/cpdf_array.h"
#include "core/fpdfapi/parser/cpdf_dictionary.h"
@@ -18,7 +19,8 @@
} // namespace
-CPDF_IconFit::CPDF_IconFit(const CPDF_Dictionary* pDict) : m_pDict(pDict) {}
+CPDF_IconFit::CPDF_IconFit(RetainPtr<const CPDF_Dictionary> pDict)
+ : m_pDict(std::move(pDict)) {}
CPDF_IconFit::CPDF_IconFit(const CPDF_IconFit& that) = default;
diff --git a/core/fpdfdoc/cpdf_iconfit.h b/core/fpdfdoc/cpdf_iconfit.h
index fb219a8..2c9e286 100644
--- a/core/fpdfdoc/cpdf_iconfit.h
+++ b/core/fpdfdoc/cpdf_iconfit.h
@@ -16,7 +16,7 @@
public:
enum class ScaleMethod { kAlways = 0, kBigger, kSmaller, kNever };
- explicit CPDF_IconFit(const CPDF_Dictionary* pDict);
+ explicit CPDF_IconFit(RetainPtr<const CPDF_Dictionary> pDict);
CPDF_IconFit(const CPDF_IconFit& that);
~CPDF_IconFit();
diff --git a/fpdfsdk/fpdf_attachment.cpp b/fpdfsdk/fpdf_attachment.cpp
index de02785..05ea1d2 100644
--- a/fpdfsdk/fpdf_attachment.cpp
+++ b/fpdfsdk/fpdf_attachment.cpp
@@ -125,8 +125,9 @@
if (!pFile)
return 0;
- return Utf16EncodeMaybeCopyAndReturnLength(CPDF_FileSpec(pFile).GetFileName(),
- buffer, buflen);
+ CPDF_FileSpec spec(pdfium::WrapRetain(pFile));
+ return Utf16EncodeMaybeCopyAndReturnLength(spec.GetFileName(), buffer,
+ buflen);
}
FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV
@@ -135,7 +136,8 @@
if (!pFile)
return 0;
- CPDF_Dictionary* pParamsDict = CPDF_FileSpec(pFile).GetParamsDict();
+ CPDF_FileSpec spec(pdfium::WrapRetain(pFile));
+ RetainPtr<const CPDF_Dictionary> pParamsDict = spec.GetParamsDict();
return pParamsDict ? pParamsDict->KeyExist(key) : 0;
}
@@ -144,7 +146,8 @@
if (!FPDFAttachment_HasKey(attachment, key))
return FPDF_OBJECT_UNKNOWN;
- CPDF_FileSpec spec(CPDFObjectFromFPDFAttachment(attachment));
+ CPDF_FileSpec spec(
+ pdfium::WrapRetain(CPDFObjectFromFPDFAttachment(attachment)));
RetainPtr<const CPDF_Object> pObj = spec.GetParamsDict()->GetObjectFor(key);
return pObj ? pObj->GetType() : FPDF_OBJECT_UNKNOWN;
}
@@ -157,7 +160,8 @@
if (!pFile)
return false;
- CPDF_Dictionary* pParamsDict = CPDF_FileSpec(pFile).GetParamsDict();
+ CPDF_FileSpec spec(pdfium::WrapRetain(pFile));
+ RetainPtr<CPDF_Dictionary> pParamsDict = spec.GetMutableParamsDict();
if (!pParamsDict)
return false;
@@ -180,7 +184,8 @@
if (!pFile)
return 0;
- CPDF_Dictionary* pParamsDict = CPDF_FileSpec(pFile).GetParamsDict();
+ CPDF_FileSpec spec(pdfium::WrapRetain(pFile));
+ RetainPtr<const CPDF_Dictionary> pParamsDict = spec.GetParamsDict();
if (!pParamsDict)
return 0;
@@ -260,11 +265,12 @@
if (!pFile)
return false;
- CPDF_Stream* pFileStream = CPDF_FileSpec(pFile).GetFileStream();
+ CPDF_FileSpec spec(pdfium::WrapRetain(pFile));
+ RetainPtr<const CPDF_Stream> pFileStream = spec.GetFileStream();
if (!pFileStream)
return false;
*out_buflen =
- DecodeStreamMaybeCopyAndReturnLength(pFileStream, buffer, buflen);
+ DecodeStreamMaybeCopyAndReturnLength(pFileStream.Get(), buffer, buflen);
return true;
}
diff --git a/fpdfsdk/fpdf_doc.cpp b/fpdfsdk/fpdf_doc.cpp
index 5469840..18218a1 100644
--- a/fpdfsdk/fpdf_doc.cpp
+++ b/fpdfsdk/fpdf_doc.cpp
@@ -80,7 +80,8 @@
if (!pDoc)
return nullptr;
CPDF_BookmarkTree tree(pDoc);
- CPDF_Bookmark cBookmark(CPDFDictionaryFromFPDFBookmark(bookmark));
+ CPDF_Bookmark cBookmark(
+ pdfium::WrapRetain(CPDFDictionaryFromFPDFBookmark(bookmark)));
return FPDFBookmarkFromCPDFDictionary(
tree.GetFirstChild(cBookmark).GetDict());
}
@@ -95,7 +96,8 @@
return nullptr;
CPDF_BookmarkTree tree(pDoc);
- CPDF_Bookmark cBookmark(CPDFDictionaryFromFPDFBookmark(bookmark));
+ CPDF_Bookmark cBookmark(
+ pdfium::WrapRetain(CPDFDictionaryFromFPDFBookmark(bookmark)));
return FPDFBookmarkFromCPDFDictionary(
tree.GetNextSibling(cBookmark).GetDict());
}
@@ -106,7 +108,8 @@
unsigned long buflen) {
if (!bookmark)
return 0;
- CPDF_Bookmark cBookmark(CPDFDictionaryFromFPDFBookmark(bookmark));
+ CPDF_Bookmark cBookmark(
+ pdfium::WrapRetain(CPDFDictionaryFromFPDFBookmark(bookmark)));
WideString title = cBookmark.GetTitle();
return Utf16EncodeMaybeCopyAndReturnLength(title, buffer, buflen);
}
@@ -114,7 +117,8 @@
FPDF_EXPORT int FPDF_CALLCONV FPDFBookmark_GetCount(FPDF_BOOKMARK bookmark) {
if (!bookmark)
return 0;
- CPDF_Bookmark cBookmark(CPDFDictionaryFromFPDFBookmark(bookmark));
+ CPDF_Bookmark cBookmark(
+ pdfium::WrapRetain(CPDFDictionaryFromFPDFBookmark(bookmark)));
return cBookmark.GetCount();
}
@@ -143,7 +147,8 @@
if (!bookmark)
return nullptr;
- CPDF_Bookmark cBookmark(CPDFDictionaryFromFPDFBookmark(bookmark));
+ CPDF_Bookmark cBookmark(
+ pdfium::WrapRetain(CPDFDictionaryFromFPDFBookmark(bookmark)));
CPDF_Dest dest = cBookmark.GetDest(pDoc);
if (dest.GetArray())
return FPDFDestFromCPDFArray(dest.GetArray());
@@ -160,7 +165,8 @@
if (!bookmark)
return nullptr;
- CPDF_Bookmark cBookmark(CPDFDictionaryFromFPDFBookmark(bookmark));
+ CPDF_Bookmark cBookmark(
+ pdfium::WrapRetain(CPDFDictionaryFromFPDFBookmark(bookmark)));
return FPDFActionFromCPDFDictionary(cBookmark.GetAction().GetDict());
}
