Use CFX_MaybeOwned<> in fpdf_edit_create.cpp

Use CFX_MaybeOwned<> in fpdf_edit_create.cpp

Fix missing second template parameter in cfx_maybe_owned.h

Committed: https://pdfium.googlesource.com/pdfium/+/405ac0f09e1622d7ff3cf60314d290851ac9f7fd

[Tom Sepez, 2016-11-23 23:04:32]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-23 23:04:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tom Sepez, 2016-11-23 23:05:16]

Description was changed from

==========
Use CFX_MaybeOwned<> in fpdf_edit_create.cpp
==========

to

==========
Use CFX_MaybeOwned<> in fpdf_edit_create.cpp

Fix missing second template parameter in cfx_maybe_owned.h
==========

[Tom Sepez, 2016-11-23 23:05:16]

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

[commit-bot: I haz the power, 2016-11-23 23:14:32]

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

[commit-bot: I haz the power, 2016-11-23 23:14:33]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-11-28 18:11:43]

thestig@chromium.org changed reviewers:
+ npm@chromium.org, weili@chromium.org

[Lei Zhang, 2016-11-28 18:14:16]

Not sure if this is ready, but it was in my queue and I punted.

[Tom Sepez, 2016-11-28 18:16:13]

Now it is.

[Tom Sepez, 2016-11-28 18:17:12]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-28 18:17:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-28 18:46:27]

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

[commit-bot: I haz the power, 2016-11-28 18:46:28]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-28 19:10:43]

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

[Tom Sepez, 2016-11-28 19:11:14]

If you're gonna punt, you should probably move to CC:

[Tom Sepez, 2016-11-28 19:54:10]

On 2016/11/28 19:11:14, Tom Sepez wrote:
> If you're gonna punt, you should probably move to CC:

wei, npm, ready for review.

[npm, 2016-11-28 20:31:02]

lgtm

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
File core/fpdfapi/edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
core/fpdfapi/edit/fpdf_edit_create.cpp:428: m_pData = (uint8_t*)m_Acc.GetData();
const_cast?

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
core/fpdfapi/edit/fpdf_edit_create.cpp:821: if
(pFile->AppendBlock(encoder.m_pData.Get(), encoder.m_dwSize) < 0) {
Nit: no {}

[Wei Li, 2016-11-28 20:59:56]

lgtm

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
File core/fpdfapi/edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
core/fpdfapi/edit/fpdf_edit_create.cpp:449: m_pData =
const_cast<uint8_t*>(pBuffer);
Nit: is cast needed? pBuffer is of type "const uint8_t*"

[npm, 2016-11-28 21:01:29]

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
File core/fpdfapi/edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
core/fpdfapi/edit/fpdf_edit_create.cpp:449: m_pData =
const_cast<uint8_t*>(pBuffer);
On 2016/11/28 20:59:56, Wei Li wrote:
> Nit: is cast needed? pBuffer is of type "const uint8_t*"

I think yes because m_pData is not const

[Wei Li, 2016-11-28 21:21:12]

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
File core/fpdfapi/edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
core/fpdfapi/edit/fpdf_edit_create.cpp:449: m_pData =
const_cast<uint8_t*>(pBuffer);
On 2016/11/28 21:01:29, npm wrote:
> On 2016/11/28 20:59:56, Wei Li wrote:
> > Nit: is cast needed? pBuffer is of type "const uint8_t*"
> 
> I think yes because m_pData is not const

Is const type better for m_pData?

[Tom Sepez, 2016-11-28 21:38:08]

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
File core/fpdfapi/edit/fpdf_edit_create.cpp (right):

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
core/fpdfapi/edit/fpdf_edit_create.cpp:428: m_pData = (uint8_t*)m_Acc.GetData();
On 2016/11/28 20:31:02, npm wrote:
> const_cast?

Done.

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
core/fpdfapi/edit/fpdf_edit_create.cpp:449: m_pData =
const_cast<uint8_t*>(pBuffer);
On 2016/11/28 21:21:12, Wei Li wrote:
> On 2016/11/28 21:01:29, npm wrote:
> > On 2016/11/28 20:59:56, Wei Li wrote:
> > > Nit: is cast needed? pBuffer is of type "const uint8_t*"
> > 
> > I think yes because m_pData is not const
> 
> Is const type better for m_pData?
We may free it later, so it's technically not const.

https://codereview.chromium.org/2522313002/diff/40001/core/fpdfapi/edit/fpdf_...
core/fpdfapi/edit/fpdf_edit_create.cpp:821: if
(pFile->AppendBlock(encoder.m_pData.Get(), encoder.m_dwSize) < 0) {
On 2016/11/28 20:31:02, npm wrote:
> Nit: no {}

Done.

[Tom Sepez, 2016-11-28 21:44:15]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-11-28 21:44:16]

The patchset sent to the CQ was uploaded after l-g-t-m from npm@chromium.org,
weili@chromium.org
Link to the patchset: https://codereview.chromium.org/2522313002/#ps60001
(title: "nits")

[commit-bot: I haz the power, 2016-11-28 21:44:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-28 21:59:14]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1480369455875550,
"parent_rev": "3c0f366daafed42495b22ecbdda8f21f3f939d9b", "commit_rev":
"405ac0f09e1622d7ff3cf60314d290851ac9f7fd"}

[commit-bot: I haz the power, 2016-11-28 21:59:17]

Description was changed from

==========
Use CFX_MaybeOwned<> in fpdf_edit_create.cpp

Fix missing second template parameter in cfx_maybe_owned.h
==========

to

==========
Use CFX_MaybeOwned<> in fpdf_edit_create.cpp

Fix missing second template parameter in cfx_maybe_owned.h

Committed:
https://pdfium.googlesource.com/pdfium/+/405ac0f09e1622d7ff3cf60314d290851ac9...
==========

[commit-bot: I haz the power, 2016-11-28 21:59:18]

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