Revert "Re-land "Assert that only 0-numbered objects are Released()""
This reverts commit b69a98cf50537130f88ce3a799117f2ca8353ac5.
Reason for revert: crashes on mac.
BUG=654387
TBR=thestig@chromium.org
Review-Url: https://codereview.chromium.org/2410483002
diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp
index 0d0c02f..9a2dba0 100644
--- a/core/fpdfapi/parser/cpdf_array.cpp
+++ b/core/fpdfapi/parser/cpdf_array.cpp
@@ -23,7 +23,7 @@
// in case of cyclic references.
m_ObjNum = kInvalidObjNum;
for (auto& it : m_Objects) {
- if (it && it->GetObjNum() != kInvalidObjNum)
+ if (it)
it->Release();
}
}
diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp
index 31f520a..7ef5a53 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/parser/cpdf_dictionary.cpp
@@ -30,7 +30,7 @@
// in case of cyclic references.
m_ObjNum = kInvalidObjNum;
for (const auto& it : m_Map) {
- if (it.second && it.second->GetObjNum() != kInvalidObjNum)
+ if (it.second)
it.second->Release();
}
}
diff --git a/core/fpdfapi/parser/cpdf_object.cpp b/core/fpdfapi/parser/cpdf_object.cpp
index 5c40733..3bf0a95 100644
--- a/core/fpdfapi/parser/cpdf_object.cpp
+++ b/core/fpdfapi/parser/cpdf_object.cpp
@@ -38,7 +38,9 @@
}
void CPDF_Object::Release() {
- CHECK(!m_ObjNum);
+ if (m_ObjNum)
+ return;
+
delete this;
}
diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp
index 657f536..b2177af 100644
--- a/core/fpdfapi/parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp
@@ -25,7 +25,6 @@
using ScopedArray = std::unique_ptr<CPDF_Array, ReleaseDeleter<CPDF_Array>>;
using ScopedDict =
std::unique_ptr<CPDF_Dictionary, ReleaseDeleter<CPDF_Dictionary>>;
-using ScopedStream = std::unique_ptr<CPDF_Stream, ReleaseDeleter<CPDF_Stream>>;
void TestArrayAccessors(const CPDF_Array* arr,
size_t index,
@@ -96,10 +95,8 @@
// Indirect references to indirect objects.
m_ObjHolder.reset(new CPDF_IndirectObjectHolder());
- m_IndirectObjs = {boolean_true_obj->Clone(), number_int_obj->Clone(),
- str_spec_obj->Clone(), name_obj->Clone(),
- m_ArrayObj->Clone(), m_DictObj->Clone(),
- stream_obj->Clone()};
+ m_IndirectObjs = {boolean_true_obj, number_int_obj, str_spec_obj, name_obj,
+ m_ArrayObj, m_DictObj, stream_obj};
for (size_t i = 0; i < m_IndirectObjs.size(); ++i) {
m_ObjHolder->AddIndirectObject(m_IndirectObjs[i]);
m_RefObjs.emplace_back(new CPDF_Reference(
@@ -107,7 +104,7 @@
}
}
- bool Equal(const CPDF_Object* obj1, const CPDF_Object* obj2) {
+ bool Equal(CPDF_Object* obj1, CPDF_Object* obj2) {
if (obj1 == obj2)
return true;
if (!obj1 || !obj2 || obj1->GetType() != obj2->GetType())
@@ -256,7 +253,7 @@
const CPDF_Dictionary* const indirect_obj_results[] = {
nullptr, nullptr, nullptr, nullptr, nullptr, m_DictObj, m_StreamDictObj};
for (size_t i = 0; i < m_RefObjs.size(); ++i)
- EXPECT_TRUE(Equal(indirect_obj_results[i], m_RefObjs[i]->GetDict()));
+ EXPECT_EQ(indirect_obj_results[i], m_RefObjs[i]->GetDict());
}
TEST_F(PDFObjectsTest, GetArray) {
@@ -790,9 +787,10 @@
TEST(PDFObjectTest, CloneCheckLoop) {
{
- // Create a dictionary/array pair with a reference loop.
- CPDF_Dictionary* dict_obj = new CPDF_Dictionary();
+ // Create an object with a reference loop.
ScopedArray arr_obj(new CPDF_Array);
+ // Dictionary object.
+ CPDF_Dictionary* dict_obj = new CPDF_Dictionary();
dict_obj->SetFor("arr", arr_obj.get());
arr_obj->InsertAt(0, dict_obj);
@@ -808,22 +806,6 @@
EXPECT_EQ(nullptr, cloned_dict->AsDictionary()->GetObjectFor("arr"));
}
{
- // Create a dictionary/stream pair with a reference loop.
- CPDF_Dictionary* dict_obj = new CPDF_Dictionary();
- ScopedStream stream_obj(new CPDF_Stream(nullptr, 0, dict_obj));
- dict_obj->SetFor("stream", stream_obj.get());
-
- // Clone this object to see whether stack overflow will be triggered.
- ScopedStream cloned_stream(stream_obj->Clone()->AsStream());
- // Cloned object should be the same as the original.
- ASSERT_TRUE(cloned_stream);
- CPDF_Object* cloned_dict = cloned_stream->GetDict();
- ASSERT_TRUE(cloned_dict);
- ASSERT_TRUE(cloned_dict->IsDictionary());
- // Recursively referenced object is not cloned.
- EXPECT_EQ(nullptr, cloned_dict->AsDictionary()->GetObjectFor("stream"));
- }
- {
CPDF_IndirectObjectHolder objects_holder;
// Create an object with a reference loop.
CPDF_Dictionary* dict_obj = new CPDF_Dictionary();
diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp
index f4cde0c..1753910 100644
--- a/core/fpdfapi/parser/cpdf_parser.cpp
+++ b/core/fpdfapi/parser/cpdf_parser.cpp
@@ -956,34 +956,36 @@
}
FX_BOOL CPDF_Parser::LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef) {
- std::unique_ptr<CPDF_Object> pObject(
- ParseIndirectObjectAt(m_pDocument, *pos, 0));
+ CPDF_Object* pObject = ParseIndirectObjectAt(m_pDocument, *pos, 0);
if (!pObject)
return FALSE;
- CPDF_Object* pUnownedObject = pObject.get();
-
if (m_pDocument) {
CPDF_Dictionary* pRootDict = m_pDocument->GetRoot();
- if (pRootDict && pRootDict->GetObjNum() == pObject->m_ObjNum)
+ if (pRootDict && pRootDict->GetObjNum() == pObject->m_ObjNum) {
+ // If |pObject| has an objnum assigned then this will leak as Release()
+ // will early exit.
+ if (pObject->IsStream())
+ pObject->Release();
return FALSE;
- // Takes ownership of object (std::move someday).
- uint32_t objnum = pObject->m_ObjNum;
- if (!m_pDocument->ReplaceIndirectObjectIfHigherGeneration(
- objnum, pObject.release())) {
+ }
+ if (!m_pDocument->ReplaceIndirectObjectIfHigherGeneration(pObject->m_ObjNum,
+ pObject)) {
return FALSE;
}
}
- CPDF_Stream* pStream = pUnownedObject->AsStream();
+ CPDF_Stream* pStream = pObject->AsStream();
if (!pStream)
return FALSE;
CPDF_Dictionary* pDict = pStream->GetDict();
*pos = pDict->GetIntegerFor("Prev");
int32_t size = pDict->GetIntegerFor("Size");
- if (size < 0)
+ if (size < 0) {
+ pStream->Release();
return FALSE;
+ }
CPDF_Dictionary* pNewTrailer = ToDictionary(pDict->Clone());
if (bMainXRef) {
@@ -1015,8 +1017,10 @@
arrIndex.push_back(std::make_pair(0, size));
pArray = pDict->GetArrayFor("W");
- if (!pArray)
+ if (!pArray) {
+ pStream->Release();
return FALSE;
+ }
CFX_ArrayTemplate<uint32_t> WidthArray;
FX_SAFE_UINT32 dwAccWidth = 0;
@@ -1025,8 +1029,10 @@
dwAccWidth += WidthArray[i];
}
- if (!dwAccWidth.IsValid() || WidthArray.GetSize() < 3)
+ if (!dwAccWidth.IsValid() || WidthArray.GetSize() < 3) {
+ pStream->Release();
return FALSE;
+ }
uint32_t totalWidth = dwAccWidth.ValueOrDie();
CPDF_StreamAcc acc;
@@ -1086,14 +1092,17 @@
if (type == 1) {
m_SortedOffset.insert(offset);
} else {
- if (offset < 0 || !IsValidObjectNumber(offset))
+ if (offset < 0 || !IsValidObjectNumber(offset)) {
+ pStream->Release();
return FALSE;
+ }
m_ObjectInfo[offset].type = 255;
}
}
}
segindex += count;
}
+ pStream->Release();
return TRUE;
}
diff --git a/core/fpdfapi/parser/cpdf_stream.cpp b/core/fpdfapi/parser/cpdf_stream.cpp
index 935c80c..c221ede 100644
--- a/core/fpdfapi/parser/cpdf_stream.cpp
+++ b/core/fpdfapi/parser/cpdf_stream.cpp
@@ -17,11 +17,7 @@
CPDF_Stream::CPDF_Stream(uint8_t* pData, uint32_t size, CPDF_Dictionary* pDict)
: m_pDict(pDict), m_dwSize(size), m_pDataBuf(pData) {}
-CPDF_Stream::~CPDF_Stream() {
- m_ObjNum = kInvalidObjNum;
- if (m_pDict && m_pDict->GetObjNum() == kInvalidObjNum)
- m_pDict.release(); // lowercase release, release ownership.
-}
+CPDF_Stream::~CPDF_Stream() {}
CPDF_Object::Type CPDF_Stream::GetType() const {
return STREAM;
