Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2489653003:
Fix abort above FPDFPage_Flatten (Closed)
DescriptionFix abort above FPDFPage_Flatten
Main issue: FPDFPage_Flatten trying to re-add an indirect object.
BUG=662698
Committed: https://pdfium.googlesource.com/pdfium/+/d0ecd899d632461bd6dc53e32b42bbd3a5d030ad
Patch Set 1 : Add failing test case first. #Patch Set 2 : Fix issue #
Total comments: 2
Patch Set 3 : Fix it in code, not comments #Patch Set 4 : Leak in test #Patch Set 5 : rebased #
Messages
Total messages: 27 (18 generated)
Description was changed from ========== Fix abort above FPDFPage_Flatten BUG=662698 ========== to ========== Fix abort above FPDFPage_Flatten Two issues: FPDFPage_Flatten trying to re-add an indirect object. Destruction order wrong in cpdfsdk_formfillenvironment. BUG=662698 ==========
tsepez@chromium.org changed reviewers: + thestig@chromium.org
The CQ bit was checked by tsepez@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Lei, ready for review. It seems we never called FPDF_Flatten in any of the pdfium-side tests.
https://codereview.chromium.org/2489653003/diff/20001/fpdfsdk/fpdf_flatten.cpp File fpdfsdk/fpdf_flatten.cpp (left): https://codereview.chromium.org/2489653003/diff/20001/fpdfsdk/fpdf_flatten.cp... fpdfsdk/fpdf_flatten.cpp:400: CFX_Matrix matrix = pAPDic->GetMatrixFor("Matrix"); Note: not used til below, pAPDic not modified in-between. https://codereview.chromium.org/2489653003/diff/20001/fpdfsdk/fpdf_flatten.cp... fpdfsdk/fpdf_flatten.cpp:413: if (pObj) { Note: already checked at 396.
LSAN says this is leaky.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_asan_lsan on master.tryserver.client.pdfium (JOB_FAILED, https://build.chromium.org/p/tryserver.client.pdfium/builders/linux_asan_lsan...)
The CQ bit was checked by tsepez@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Fix abort above FPDFPage_Flatten Two issues: FPDFPage_Flatten trying to re-add an indirect object. Destruction order wrong in cpdfsdk_formfillenvironment. BUG=662698 ========== to ========== Fix abort above FPDFPage_Flatten Main issue: FPDFPage_Flatten trying to re-add an indirect object. Secondary issues kicked loose by test case: Destruction order wrong in cpdfsdk_formfillenvironment. Also, fix m_PageMap naming to match conventions for members without a type prefix code. BUG=662698 ==========
Description was changed from ========== Fix abort above FPDFPage_Flatten Main issue: FPDFPage_Flatten trying to re-add an indirect object. Secondary issues kicked loose by test case: Destruction order wrong in cpdfsdk_formfillenvironment. Also, fix m_PageMap naming to match conventions for members without a type prefix code. BUG=662698 ========== to ========== Fix abort above FPDFPage_Flatten Main issue: FPDFPage_Flatten trying to re-add an indirect object. Secondary issues kicked loose by test case: Destruction order wrong in cpdfsdk_formfillenvironment. LSAN hit another leak. Also, fix m_PageMap naming to match conventions for members without a type prefix code. BUG=662698 ==========
The CQ bit was checked by tsepez@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Fix abort above FPDFPage_Flatten Main issue: FPDFPage_Flatten trying to re-add an indirect object. Secondary issues kicked loose by test case: Destruction order wrong in cpdfsdk_formfillenvironment. LSAN hit another leak. Also, fix m_PageMap naming to match conventions for members without a type prefix code. BUG=662698 ========== to ========== Fix abort above FPDFPage_Flatten Main issue: FPDFPage_Flatten trying to re-add an indirect object. Secondary issues kicked loose by test case: Destruction order wrong in cpdfsdk_formfillenvironment as of 6c659ab2. Also, fix m_PageMap naming to match conventions for members without a type prefix code. BUG=662698 ==========
On 2016/11/08 19:52:36, Lei Zhang (slow) wrote: > LSAN says this is leaky. Fixed leaks in new test (I hope). But, do you want me to split this into two separate patches one for each issue?
On 2016/11/08 20:11:35, Tom Sepez wrote: > On 2016/11/08 19:52:36, Lei Zhang (slow) wrote: > > LSAN says this is leaky. > Fixed leaks in new test (I hope). But, do you want me to split this > into two separate patches one for each issue? Sounds good.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I done did it.
Description was changed from ========== Fix abort above FPDFPage_Flatten Main issue: FPDFPage_Flatten trying to re-add an indirect object. Secondary issues kicked loose by test case: Destruction order wrong in cpdfsdk_formfillenvironment as of 6c659ab2. Also, fix m_PageMap naming to match conventions for members without a type prefix code. BUG=662698 ========== to ========== Fix abort above FPDFPage_Flatten Main issue: FPDFPage_Flatten trying to re-add an indirect object. BUG=662698 ==========
lgtm
The CQ bit was checked by tsepez@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Fix abort above FPDFPage_Flatten Main issue: FPDFPage_Flatten trying to re-add an indirect object. BUG=662698 ========== to ========== Fix abort above FPDFPage_Flatten Main issue: FPDFPage_Flatten trying to re-add an indirect object. BUG=662698 Committed: https://pdfium.googlesource.com/pdfium/+/d0ecd899d632461bd6dc53e32b42bbd3a5d0... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://pdfium.googlesource.com/pdfium/+/d0ecd899d632461bd6dc53e32b42bbd3a5d0... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
