Remove Release() from CFX_RTFBreak

Remove Release() from CFX_RTFBreak

Use unique_ptrs in several places in xfa_textlayout.h

Committed: https://pdfium.googlesource.com/pdfium/+/7ee2e53f741cc17f35a4fece5980e60fded93861

[Tom Sepez, 2016-05-20 22:51:25]

Lei, for review.

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
File xfa/fxfa/app/xfa_textlayout.cpp (left):

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.cpp:42: delete m_pAllocator;
(!!!) given line 48

[Tom Sepez, 2016-05-20 22:51:39]

Description was changed from

==========
Remove Release() from CFX_RTFBreak

Use unique_ptrs in several places in xfa_textlayout.h
==========

to

==========
Remove Release() from CFX_RTFBreak

Use unique_ptrs in several places in xfa_textlayout.h
==========

[Tom Sepez, 2016-05-20 22:51:39]

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

[Tom Sepez, 2016-05-20 22:52:00]

Actually adding Lei.

[Lei Zhang, 2016-05-20 23:00:52]

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
File xfa/fxfa/app/xfa_textlayout.cpp (right):

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.cpp:1272: if (!m_pAllocator)
braces

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout.h
File xfa/fxfa/app/xfa_textlayout.h (right):

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.h:180: ~CXFA_LinkUserData() {}
needs override

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.h:221: ~CXFA_TextUserData() {
needs override

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.h:247: ~XFA_TextPiece() {
needs override

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.h:325: CXFA_TextLayout(CXFA_TextProvider*
pTextProvider);
explicit

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.h:326: virtual ~CXFA_TextLayout();
No need to be virtual.

[Tom Sepez, 2016-05-20 23:07:30]

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
File xfa/fxfa/app/xfa_textlayout.cpp (right):

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.cpp:1272: if (!m_pAllocator)
On 2016/05/20 23:00:52, Lei Zhang wrote:
> braces

Done.

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout.h
File xfa/fxfa/app/xfa_textlayout.h (right):

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.h:180: ~CXFA_LinkUserData() {}
On 2016/05/20 23:00:52, Lei Zhang wrote:
> needs override

Done.

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.h:221: ~CXFA_TextUserData() {
On 2016/05/20 23:00:52, Lei Zhang wrote:
> needs override

Done.

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.h:247: ~XFA_TextPiece() {
On 2016/05/20 23:00:52, Lei Zhang wrote:
> needs override

Done.

https://codereview.chromium.org/2003843002/diff/1/xfa/fxfa/app/xfa_textlayout...
xfa/fxfa/app/xfa_textlayout.h:326: virtual ~CXFA_TextLayout();
On 2016/05/20 23:00:52, Lei Zhang wrote:
> No need to be virtual.

Done.

[Lei Zhang, 2016-05-20 23:08:42]

lgtm

https://codereview.chromium.org/2003843002/diff/20001/xfa/fxfa/app/xfa_textla...
File xfa/fxfa/app/xfa_textlayout.h (right):

https://codereview.chromium.org/2003843002/diff/20001/xfa/fxfa/app/xfa_textla...
xfa/fxfa/app/xfa_textlayout.h:325: CXFA_TextLayout(CXFA_TextProvider*
pTextProvider);
explict? no?

[Tom Sepez, 2016-05-20 23:10:04]

https://codereview.chromium.org/2003843002/diff/20001/xfa/fxfa/app/xfa_textla...
File xfa/fxfa/app/xfa_textlayout.h (right):

https://codereview.chromium.org/2003843002/diff/20001/xfa/fxfa/app/xfa_textla...
xfa/fxfa/app/xfa_textlayout.h:325: CXFA_TextLayout(CXFA_TextProvider*
pTextProvider);
On 2016/05/20 23:08:41, Lei Zhang wrote:
> explict? no?

Done.

[Tom Sepez, 2016-05-20 23:10:09]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-05-20 23:10:09]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2003843002/#ps40001
(title: "Explicit.")

[commit-bot: I haz the power, 2016-05-20 23:10:13]

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

[commit-bot: I haz the power, 2016-05-21 00:43:23]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-21 00:43:23]

Try jobs failed on following builders:
  linux_xfa on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/linux_xfa/build...)

[Tom Sepez, 2016-05-23 17:14:41]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-05-23 17:14:42]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2003843002/#ps60001
(title: "Consistent use of override.")

[commit-bot: I haz the power, 2016-05-23 17:14:53]

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

[commit-bot: I haz the power, 2016-05-23 17:55:53]

Description was changed from

==========
Remove Release() from CFX_RTFBreak

Use unique_ptrs in several places in xfa_textlayout.h
==========

to

==========
Remove Release() from CFX_RTFBreak

Use unique_ptrs in several places in xfa_textlayout.h

Committed:
https://pdfium.googlesource.com/pdfium/+/7ee2e53f741cc17f35a4fece5980e60fded9...
==========

[commit-bot: I haz the power, 2016-05-23 17:55:53]

Committed patchset #4 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/7ee2e53f741cc17f35a4fece5980e60fded9...