Fix leak in CPDF_StreamContentParser::AddTextObject().

Fix leak in CPDF_StreamContentParser::AddTextObject().

... by using STL containers in more places.
Remove dead / duplicate code as well.

BUG=603904
Committed: https://pdfium.googlesource.com/pdfium/+/80f25a5a8135933a405349ffc798d13273b3d690

[Lei Zhang, 2016-05-19 17:28:44]

thestig@chromium.org changed reviewers:
+ tsepez@chromium.org

[Lei Zhang, 2016-05-19 17:28:47]

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_clippath.cpp (left):

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_clippath.cpp:83: void CPDF_ClipPath::DeletePath(int
index) {
Dead code.

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_clippathdata.cpp (left):

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_clippathdata.cpp:64: void
CPDF_ClipPathData::SetCount(int path_count, int text_count) {
Dead code.

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/fpdf_page_parser.cpp (left):

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:1288:
m_ClipTextList.Add(pText->Clone());
CPDF_StreamContentParser::Handle_EndText() above sometimes forget to delete
this.

[Tom Sepez, 2016-05-19 18:32:03]

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_clippath.cpp (right):

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_clippath.cpp:63: pData->m_PathList.back().SetNull();
Doesn't the dtor invoked by pop_back on the next line cover this?

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_clippath.cpp:68: pData->m_PathList.push_back(path);
you don't suppose we could have a single vector<std::pair> rather than two
parallel vectors?

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/fpdf_page_parser.cpp (left):

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:1288:
m_ClipTextList.Add(pText->Clone());
On 2016/05/19 17:28:46, Lei Zhang wrote:
> CPDF_StreamContentParser::Handle_EndText() above sometimes forget to delete
> this.

I'm being dense and not seeing it.  Looks like either it gets delete at 822, or
it gets passed to appendTexts, which either appends it taking ownership or
deletes it itself.

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/include/cpdf_clippath.h (right):

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/include/cpdf_clippath.h:20: uint32_t GetPathCount() const
{ return m_pObject->m_PathList.size(); }
pdfium::CollectionSize<uint32_t>() if we worry about overflow from maybe 64-bit
size_t.

[Lei Zhang, 2016-05-19 20:44:26]

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_clippath.cpp (right):

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_clippath.cpp:63: pData->m_PathList.back().SetNull();
On 2016/05/19 18:32:03, Tom Sepez wrote:
> Doesn't the dtor invoked by pop_back on the next line cover this?

Done.

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_clippath.cpp:68: pData->m_PathList.push_back(path);
On 2016/05/19 18:32:03, Tom Sepez wrote:
> you don't suppose we could have a single vector<std::pair> rather than two
> parallel vectors?

Done.

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/fpdf_page_parser.cpp (left):

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/fpdf_page_parser.cpp:1288:
m_ClipTextList.Add(pText->Clone());
On 2016/05/19 18:32:03, Tom Sepez wrote:
> On 2016/05/19 17:28:46, Lei Zhang wrote:
> > CPDF_StreamContentParser::Handle_EndText() above sometimes forget to delete
> > this.
> 
> I'm being dense and not seeing it.  Looks like either it gets delete at 822,
or
> it gets passed to appendTexts, which either appends it taking ownership or
> deletes it itself.

Oh, I read it wrong. Looking again, it's due to Handle_EndText() not being
called at all.

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/include/cpdf_clippath.h (right):

https://codereview.chromium.org/1998583002/diff/40001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/include/cpdf_clippath.h:20: uint32_t GetPathCount() const
{ return m_pObject->m_PathList.size(); }
On 2016/05/19 18:32:03, Tom Sepez wrote:
> pdfium::CollectionSize<uint32_t>() if we worry about overflow from maybe
64-bit
> size_t.

Done.

[Lei Zhang, 2016-05-19 20:45:12]

Description was changed from

==========
Fix leak in CPDF_StreamContentParser::AddTextObject().

BUG=603904
==========

to

==========
Fix leak in CPDF_StreamContentParser::AddTextObject().

... by using STL containers in more places.
Remove dead / duplicate code as well.

BUG=603904
==========

[Tom Sepez, 2016-05-19 21:28:03]

lgtm

[Lei Zhang, 2016-05-19 21:35:36]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-05-19 21:35:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1998583002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1998583002/100001

[commit-bot: I haz the power, 2016-05-19 21:36:03]

Description was changed from

==========
Fix leak in CPDF_StreamContentParser::AddTextObject().

... by using STL containers in more places.
Remove dead / duplicate code as well.

BUG=603904
==========

to

==========
Fix leak in CPDF_StreamContentParser::AddTextObject().

... by using STL containers in more places.
Remove dead / duplicate code as well.

BUG=603904

Committed:
https://pdfium.googlesource.com/pdfium/+/80f25a5a8135933a405349ffc798d13273b3...
==========

[commit-bot: I haz the power, 2016-05-19 21:36:04]

Committed patchset #6 (id:100001) as
https://pdfium.googlesource.com/pdfium/+/80f25a5a8135933a405349ffc798d13273b3...