Fix a leak with FPDFPageObj_NewImgeObj().

core/fpdfapi/fpdf_page/cpdf_imageobject.cpp

Index: core/fpdfapi/fpdf_page/cpdf_imageobject.cpp
diff --git a/core/fpdfapi/fpdf_page/cpdf_imageobject.cpp b/core/fpdfapi/fpdf_page/cpdf_imageobject.cpp
index e2defa80544be13b50a30babfe498ed7edae4a4e..929f162725c8e068bf2d0b3606d85827cc847389 100644
--- a/core/fpdfapi/fpdf_page/cpdf_imageobject.cpp
+++ b/core/fpdfapi/fpdf_page/cpdf_imageobject.cpp
@@ -13,15 +13,19 @@
 CPDF_ImageObject::CPDF_ImageObject() : m_pImage(nullptr) {}
 
 CPDF_ImageObject::~CPDF_ImageObject() {
-  if (!m_pImage) {
+  if (!m_pImage)
+    return;
+
+  if (m_pImage->CanRelease()) {
+    m_pImage->Release();
     return;
   }
-  if (m_pImage->IsInline() ||
-      (m_pImage->GetStream() && m_pImage->GetStream()->GetObjNum() == 0)) {
-    delete m_pImage;
-  } else {
-    m_pImage->GetDocument()->GetPageData()->ReleaseImage(m_pImage->GetStream());
-  }
+
+  auto* pPageData = m_pImage->GetDocument()->GetPageData();
+  if (pPageData->ReleaseImage(m_pImage->GetStream()))
+    return;
+
+  delete m_pImage;

[Lei Zhang, 2016/08/01 22:08:03]

Not sure what else we can do here.

[Wei Li, 2016/08/04 18:00:39]

I suspect this might not be the right thing to do. If ReleaseImage() failed or
m_pImage can not be released, this means the image is still referenced by some
other object. This is one of the tricky situation that release order matters. 

[Lei Zhang, 2016/08/04 21:58:25]

Does you comment apply to patch set 2 as well?

[Wei Li, 2016/08/04 23:36:39]

Yes, I think both patches share similar logic. Could you be more explicit about
which situation this 'delete' tries to cover? Is it "m_pImage->CanRelease()"
failed and "pPageData has no such image"? I expect the image is either stored
inside this ImageObject (CanRelease() should be true) or in the pageData's map.
Should it be deleted from its owner side?

[Lei Zhang, 2016/08/05 18:55:22]

So CPDF_ImageObject::m_pImage gets assigned from 6 places:
1) CPDF_PageContentGenerator::ProcessImage: Owned by CPDF_DocPageData
2) CPDF_ImageObject::Clone: Either owned by CPDF_DocPageData, or owned by
CPDF_ImageObject
3) CPDF_StreamContentParser::AddImage: Owned by CPDF_DocPageData
4) CPDF_StreamContentParser::AddImage, again: Owned by CPDF_DocPageData
5) CPDF_StreamContentParser::AddImage, thrice: Owned by CPDF_ImageObject
6) FPDFPageObj_NewImgeObj: Owned by CPDF_ImageObject

Another theory is that we have Release() because
CPDF_DocPageData::ReleaseImage() returns a void.

If we change CPDF_DocPageData::ReleaseImage() to return a bool, CPDF_ImageObject
can get rid of its Release() and just delete after checking with ReleaseImage().

 }
 
 CPDF_ImageObject* CPDF_ImageObject::Clone() const {