Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(11)

Issue 2510223002: Make CPDF_Dictionary use unique pointers. (Closed)

Created:
4 years, 1 month ago by Tom Sepez
Modified:
4 years, 1 month ago
Reviewers:
Lei Zhang, dsinclair
CC:
pdfium-reviews_googlegroups.com
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Make CPDF_Dictionary use unique pointers. Some changes were required to match underlying ctors as invoked by the templated methods. Many release() calls go away, a few WrapUniques() are introduced to avoid going deeper into other code. Committed: https://pdfium.googlesource.com/pdfium/+/0e606b5ecd6e45f74391f110cc1fe0cce0e80c64

Patch Set 1 #

Patch Set 2 : fix win #

Patch Set 3 : Plug leaks #

Total comments: 6

Patch Set 4 : Fix IWYU #

Patch Set 5 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+670 lines, -680 lines) Patch
M core/fpdfapi/edit/cpdf_pagecontentgenerator.cpp View 6 chunks +14 lines, -11 lines 0 comments Download
M core/fpdfapi/edit/fpdf_edit_create.cpp View 1 2 3 4 8 chunks +12 lines, -8 lines 0 comments Download
M core/fpdfapi/font/cpdf_font.cpp View 1 2 3 2 chunks +5 lines, -4 lines 0 comments Download
M core/fpdfapi/font/cpdf_fontencoding.cpp View 1 2 3 3 chunks +5 lines, -3 lines 0 comments Download
M core/fpdfapi/page/cpdf_allstates.cpp View 1 2 3 2 chunks +3 lines, -1 line 0 comments Download
M core/fpdfapi/page/cpdf_colorspace.cpp View 1 chunk +1 line, -1 line 0 comments Download
M core/fpdfapi/page/cpdf_docpagedata.cpp View 1 2 3 3 chunks +8 lines, -5 lines 0 comments Download
M core/fpdfapi/page/cpdf_image.cpp View 1 2 3 10 chunks +35 lines, -37 lines 0 comments Download
M core/fpdfapi/page/cpdf_streamcontentparser.cpp View 5 chunks +9 lines, -10 lines 0 comments Download
M core/fpdfapi/page/cpdf_streamparser.cpp View 1 2 3 3 chunks +6 lines, -7 lines 0 comments Download
M core/fpdfapi/parser/cfdf_document.cpp View 1 2 3 2 chunks +4 lines, -2 lines 0 comments Download
M core/fpdfapi/parser/cpdf_data_avail.cpp View 1 chunk +2 lines, -4 lines 0 comments Download
M core/fpdfapi/parser/cpdf_dictionary.h View 1 2 3 5 chunks +24 lines, -17 lines 0 comments Download
M core/fpdfapi/parser/cpdf_dictionary.cpp View 6 chunks +23 lines, -79 lines 0 comments Download
M core/fpdfapi/parser/cpdf_document.cpp View 1 2 3 4 18 chunks +60 lines, -57 lines 0 comments Download
M core/fpdfapi/parser/cpdf_document_unittest.cpp View 1 2 3 5 chunks +19 lines, -11 lines 0 comments Download
M core/fpdfapi/parser/cpdf_object_unittest.cpp View 1 2 3 12 chunks +24 lines, -24 lines 0 comments Download
M core/fpdfapi/parser/cpdf_parser.cpp View 1 2 3 3 chunks +7 lines, -6 lines 0 comments Download
M core/fpdfapi/parser/cpdf_security_handler.cpp View 1 2 3 7 chunks +18 lines, -6 lines 0 comments Download
M core/fpdfapi/parser/cpdf_stream.cpp View 1 2 3 4 4 chunks +4 lines, -3 lines 0 comments Download
M core/fpdfapi/parser/cpdf_string.h View 1 2 3 2 chunks +3 lines, -1 line 0 comments Download
M core/fpdfapi/parser/cpdf_string.cpp View 1 2 3 2 chunks +7 lines, -2 lines 0 comments Download
M core/fpdfapi/parser/cpdf_syntax_parser.cpp View 1 2 3 5 chunks +6 lines, -5 lines 0 comments Download
M core/fpdfapi/parser/fpdf_parser_utility.cpp View 2 chunks +2 lines, -1 line 0 comments Download
M core/fpdfdoc/cpdf_annot.cpp View 2 chunks +2 lines, -1 line 0 comments Download
M core/fpdfdoc/cpdf_annotlist.cpp View 3 chunks +9 lines, -5 lines 0 comments Download
M core/fpdfdoc/cpdf_filespec.cpp View 3 chunks +6 lines, -3 lines 0 comments Download
M core/fpdfdoc/cpdf_filespec_unittest.cpp View 3 chunks +5 lines, -4 lines 0 comments Download
M core/fpdfdoc/cpdf_formcontrol.cpp View 4 chunks +5 lines, -4 lines 0 comments Download
M core/fpdfdoc/cpdf_formfield.cpp View 1 2 3 8 chunks +22 lines, -26 lines 0 comments Download
M core/fpdfdoc/cpdf_formfield_unittest.cpp View 2 chunks +9 lines, -8 lines 0 comments Download
M core/fpdfdoc/cpdf_interform.cpp View 1 2 3 13 chunks +43 lines, -49 lines 0 comments Download
M core/fpdfdoc/cpvt_fontmap.cpp View 2 chunks +3 lines, -2 lines 0 comments Download
M core/fpdfdoc/cpvt_generateap.cpp View 11 chunks +50 lines, -53 lines 0 comments Download
M core/fxge/dib/fx_dib_engine_unittest.cpp View 1 chunk +2 lines, -2 lines 0 comments Download
M fpdfsdk/cpdfsdk_annothandlermgr.cpp View 2 chunks +5 lines, -2 lines 0 comments Download
M fpdfsdk/cpdfsdk_baannot.cpp View 1 2 3 15 chunks +44 lines, -47 lines 0 comments Download
M fpdfsdk/cpdfsdk_widget.cpp View 3 chunks +11 lines, -10 lines 0 comments Download
M fpdfsdk/formfiller/cba_fontmap.cpp View 5 chunks +13 lines, -15 lines 0 comments Download
M fpdfsdk/fpdf_flatten.cpp View 1 2 3 7 chunks +27 lines, -30 lines 0 comments Download
M fpdfsdk/fpdf_transformpage.cpp View 4 chunks +6 lines, -5 lines 0 comments Download
M fpdfsdk/fpdfdoc_unittest.cpp View 4 chunks +64 lines, -64 lines 0 comments Download
M fpdfsdk/fpdfeditpage.cpp View 4 chunks +10 lines, -15 lines 0 comments Download
M fpdfsdk/fpdfppo.cpp View 1 2 3 7 chunks +17 lines, -19 lines 0 comments Download
M fpdfsdk/fpdfview.cpp View 1 chunk +1 line, -1 line 0 comments Download
M fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp View 1 2 3 2 chunks +3 lines, -1 line 0 comments Download
M fpdfsdk/javascript/Document.cpp View 3 chunks +4 lines, -2 lines 0 comments Download
M testing/libfuzzer/pdf_hint_table_fuzzer.cc View 3 chunks +7 lines, -6 lines 0 comments Download
M xfa/fxfa/app/xfa_fontmgr.cpp View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 30 (18 generated)
Tom Sepez
4 years, 1 month ago (2016-11-17 22:13:48 UTC) #8
Tom Sepez
Lei, Dan, please review. This is the one that gets us to 95% of the ...
4 years, 1 month ago (2016-11-17 22:15:53 UTC) #10
Lei Zhang
This CL is a bit big. I looked through maybe 1/3 of it, but I ...
4 years, 1 month ago (2016-11-18 04:14:20 UTC) #15
Tom Sepez
On 2016/11/18 04:14:20, Lei Zhang (Mostly OOO) wrote: > This CL is a bit big. ...
4 years, 1 month ago (2016-11-18 17:20:27 UTC) #16
Lei Zhang
https://codereview.chromium.org/2510223002/diff/40001/core/fpdfapi/parser/cpdf_document_unittest.cpp File core/fpdfapi/parser/cpdf_document_unittest.cpp (right): https://codereview.chromium.org/2510223002/diff/40001/core/fpdfapi/parser/cpdf_document_unittest.cpp#newcode42 core/fpdfapi/parser/cpdf_document_unittest.cpp:42: page->SetNewFor<CPDF_Number>("PageNumbering", static_cast<int>(number)); Maybe just pass in an int instead? ...
4 years, 1 month ago (2016-11-18 23:28:35 UTC) #17
Lei Zhang
lgtm, maybe simplify some of the SetNewFor<CPDF_String> calls later, assuming it's possible? https://codereview.chromium.org/2510223002/diff/40001/core/fpdfdoc/cpvt_generateap.cpp File core/fpdfdoc/cpvt_generateap.cpp ...
4 years, 1 month ago (2016-11-18 23:38:35 UTC) #18
Tom Sepez
Yes, the CPDF_String constructor arguments are for another day. I'm not sure what you're asking ...
4 years, 1 month ago (2016-11-18 23:51:22 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2510223002/60001
4 years, 1 month ago (2016-11-18 23:51:33 UTC) #22
Lei Zhang
https://codereview.chromium.org/2510223002/diff/40001/core/fpdfapi/parser/cpdf_string.h File core/fpdfapi/parser/cpdf_string.h (right): https://codereview.chromium.org/2510223002/diff/40001/core/fpdfapi/parser/cpdf_string.h#newcode22 core/fpdfapi/parser/cpdf_string.h:22: CPDF_String(CFX_WeakPtr<CFX_ByteStringPool> pPool, const CFX_WideString& str); On 2016/11/18 23:28:35, Lei ...
4 years, 1 month ago (2016-11-18 23:58:59 UTC) #23
commit-bot: I haz the power
Failed to apply patch for core/fpdfapi/parser/cpdf_document.cpp: While running git apply --index -p1; error: patch failed: ...
4 years, 1 month ago (2016-11-19 00:08:31 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2510223002/80001
4 years, 1 month ago (2016-11-19 00:11:01 UTC) #28
commit-bot: I haz the power
4 years, 1 month ago (2016-11-19 00:22:44 UTC) #30
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://pdfium.googlesource.com/pdfium/+/0e606b5ecd6e45f74391f110cc1fe0cce0e8...

Powered by Google App Engine
This is Rietveld 408576698