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

Issue 1810823002: Use CFX_RetainPtr to ref count CFX_ByteString (Closed)

Created:
4 years, 9 months ago by Tom Sepez
Modified:
4 years, 8 months ago
Reviewers:
Lei Zhang, dsinclair
CC:
pdfium-reviews_googlegroups.com
Base URL:
https://pdfium.googlesource.com/pdfium.git@retain_ptr
Target Ref:
refs/heads/master
Project:
pdfium
Visibility:
Public.

Description

Use CFX_RetainPtr to ref count CFX_ByteString This moves the reference counts from an ad-hoc mechanism to being automatically tracked. Also: Make StringData::Create() always return non-null. Add better ctors for StringData itself. Consolidate copies into StringData methods. Simplify the concat() code. Rename/reorder some parameter names to be simpler. Committed: https://pdfium.googlesource.com/pdfium/+/ac88953dfa7c1a68c69989d61d7bc03c0595064b

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : Rebase #

Total comments: 25

Patch Set 4 : Add asserts, comments, min(max()). #

Unified diffs Side-by-side diffs Delta from patch set Stats (+391 lines, -470 lines) Patch
M core/fxcrt/fx_basic_bstring.cpp View 1 2 3 13 chunks +340 lines, -372 lines 0 comments Download
M core/fxcrt/fx_basic_bstring_unittest.cpp View 1 1 chunk +6 lines, -6 lines 0 comments Download
M core/fxcrt/include/fx_string.h View 1 2 3 16 chunks +45 lines, -92 lines 0 comments Download

Messages

Total messages: 15 (7 generated)
Tom Sepez
Lei, for review. Also Dan.
4 years, 8 months ago (2016-03-29 23:10:52 UTC) #5
Lei Zhang
https://codereview.chromium.org/1810823002/diff/40001/core/fxcrt/fx_basic_bstring.cpp File core/fxcrt/fx_basic_bstring.cpp (right): https://codereview.chromium.org/1810823002/diff/40001/core/fxcrt/fx_basic_bstring.cpp#newcode94 core/fxcrt/fx_basic_bstring.cpp:94: FXSYS_assert(allocLen >= 0); The next assert implies this one. ...
4 years, 8 months ago (2016-03-30 00:15:33 UTC) #7
dsinclair
https://codereview.chromium.org/1810823002/diff/40001/core/fxcrt/fx_basic_bstring.cpp File core/fxcrt/fx_basic_bstring.cpp (right): https://codereview.chromium.org/1810823002/diff/40001/core/fxcrt/fx_basic_bstring.cpp#newcode100 core/fxcrt/fx_basic_bstring.cpp:100: FXSYS_memcpy(m_String, other.m_String, other.m_nDataLength + 1); m_nDataLength appears to be ...
4 years, 8 months ago (2016-03-30 13:27:20 UTC) #8
Tom Sepez
https://codereview.chromium.org/1810823002/diff/40001/core/fxcrt/fx_basic_bstring.cpp File core/fxcrt/fx_basic_bstring.cpp (right): https://codereview.chromium.org/1810823002/diff/40001/core/fxcrt/fx_basic_bstring.cpp#newcode94 core/fxcrt/fx_basic_bstring.cpp:94: FXSYS_assert(allocLen >= 0); On 2016/03/30 00:15:32, Lei Zhang wrote: ...
4 years, 8 months ago (2016-03-30 18:49:19 UTC) #9
dsinclair
lgtm
4 years, 8 months ago (2016-03-30 18:55:06 UTC) #10
Lei Zhang
lgtm
4 years, 8 months ago (2016-03-30 22:05:25 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1810823002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1810823002/60001
4 years, 8 months ago (2016-03-30 22:13:54 UTC) #13
commit-bot: I haz the power
4 years, 8 months ago (2016-03-30 22:14:12 UTC) #15
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/ac88953dfa7c1a68c69989d61d7bc03c0595...

Powered by Google App Engine
This is Rietveld 408576698