Change the clear order of pDocPage and pDocRender

Change the clear order of pDocPage and pDocRender

pTransfer function is released in pDocRender cleanup but is still being accessed in
~CPDF_GeneralStateData in pDocPage cleanup.

BUG=419320
R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/db01bffb1960534b19dd87b4540d4c50eca0f6bf

[Bo Xu, 2014-10-21 20:52:30]

bo_xu@foxitsoftware.com changed reviewers:
+ jun_fang@foxitsoftware.com, tsepez@chromium.org

[Bo Xu, 2014-10-21 20:52:31]

Hi Tom, please review this one. Thanks.

[Tom Sepez, 2014-10-21 21:05:29]

https://codereview.chromium.org/662063003/diff/1/core/src/fpdfapi/fpdf_parser...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_document.cpp (right):

https://codereview.chromium.org/662063003/diff/1/core/src/fpdfapi/fpdf_parser...
core/src/fpdfapi/fpdf_parser/fpdf_parser_document.cpp:102: if (m_pDocRender) {
Are you sure this doesn't introduce new UAFs from render objects that are now
outliving the page objects?  Is there a design principle somewhere that states
one must outlast the other? Without knowing the details, it feels like a design
error if render objects aren't shorter lived than the pages they represent.

[Bo Xu, 2014-10-21 21:19:42]

https://codereview.chromium.org/662063003/diff/1/core/src/fpdfapi/fpdf_parser...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_document.cpp (right):

https://codereview.chromium.org/662063003/diff/1/core/src/fpdfapi/fpdf_parser...
core/src/fpdfapi/fpdf_parser/fpdf_parser_document.cpp:102: if (m_pDocRender) {
On 2014/10/21 21:05:29, Tom Sepez wrote:
> Are you sure this doesn't introduce new UAFs from render objects that are now
> outliving the page objects?  Is there a design principle somewhere that states
> one must outlast the other? Without knowing the details, it feels like a
design
> error if render objects aren't shorter lived than the pages they represent.

Hi Tom, I have the same question and checked the devs in Chinese team before. So
DestroyDocData(m_pDocRender)will calls into CPDF_DocRenderData::Clear(FX_BOOL
bRelease) and it will release m_Type3FaceMap, m_TransferFuncMap and
m_pFontCache. This is very similar to CPDF_DocPageData::Clear(FX_BOOL
bForceRelease) where all those page objects are released.

So what I got from them are m_pDocPage and m_pDocRender are kinda independent.
m_pDocRender is more light-weighted (only 3 resources) and are not dependent on
m_pDocPage. I checked the code and that seems to be the case.

[Tom Sepez, 2014-10-21 21:21:13]

LGTM.

[Bo Xu, 2014-10-21 21:50:18]

Committed patchset #1 (id:1) manually as
db01bffb1960534b19dd87b4540d4c50eca0f6bf (presubmit successful).