Make CPDF_Dictionary methods take CFX_ByteString arguments

Make CPDF_Dictionary methods take CFX_ByteString arguments

This will help avoid duplicate allocation of CFX_ByteStrings
when the caller already has one.  It may seem counter-intuitive 
that requiring the caller to pass an allocated CFX_ByteString 
rather than a static CFX_ByteStringC would improve the situation,
but due to the idiosyncrasies of std::map, the CPDF_Dictionary
methods must always do an allocation under the covers which
can't be avoided.

The changed callers in this CL are places where we would
previously demote to CFX_ByteStringC and then allocate a
a duplicate CFX_ByteString in the dictionary method.

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

[Tom Sepez, 2016-04-14 17:22:09]

Description was changed from

==========
Make CPDF_Dictionary methods take CFX_ByteString arguments

This will help avoid re-allocatin CFX_ByteStrings when the
caller already has one.  It may seem counter-intuitive that
requiring the caller to pass an allocated CFX_ByteString rather
than a static CFX_ByteStringC would improve the situation, but
due to the idiosyncracies of std::map, the CPDF_Dictionary
methods must always do an allocation under the covers which
can't be avoided.
==========

to

==========
Make CPDF_Dictionary methods take CFX_ByteString arguments

This will help avoid re-allocation CFX_ByteStrings when the
caller already has one.  It may seem counter-intuitive that
requiring the caller to pass an allocated CFX_ByteString rather
than a static CFX_ByteStringC would improve the situation, but
due to the idiosyncrasies of std::map, the CPDF_Dictionary
methods must always do an allocation under the covers which
can't be avoided.
==========

[Tom Sepez, 2016-04-14 17:22:10]

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

[Tom Sepez, 2016-04-14 17:23:33]

Description was changed from

==========
Make CPDF_Dictionary methods take CFX_ByteString arguments

This will help avoid re-allocation CFX_ByteStrings when the
caller already has one.  It may seem counter-intuitive that
requiring the caller to pass an allocated CFX_ByteString rather
than a static CFX_ByteStringC would improve the situation, but
due to the idiosyncrasies of std::map, the CPDF_Dictionary
methods must always do an allocation under the covers which
can't be avoided.
==========

to

==========
Make CPDF_Dictionary methods take CFX_ByteString arguments

This will help avoid re-allocation CFX_ByteStrings when the
caller already has one.  It may seem counter-intuitive that
requiring the caller to pass an allocated CFX_ByteString rather
than a static CFX_ByteStringC would improve the situation, but
due to the idiosyncrasies of std::map, the CPDF_Dictionary
methods must always do an allocation under the covers which
can't be avoided.

The changed callers in this CL are places where we would
previously demote to CFX_ByteStringC and then re-allocate
a new CFX_ByteString in the dictionary method.
==========

[Tom Sepez, 2016-04-14 17:23:50]

Dan, avoid a few malloc/copies.

[Tom Sepez, 2016-04-14 17:24:10]

Description was changed from

==========
Make CPDF_Dictionary methods take CFX_ByteString arguments

This will help avoid re-allocation CFX_ByteStrings when the
caller already has one.  It may seem counter-intuitive that
requiring the caller to pass an allocated CFX_ByteString rather
than a static CFX_ByteStringC would improve the situation, but
due to the idiosyncrasies of std::map, the CPDF_Dictionary
methods must always do an allocation under the covers which
can't be avoided.

The changed callers in this CL are places where we would
previously demote to CFX_ByteStringC and then re-allocate
a new CFX_ByteString in the dictionary method.
==========

to

==========
Make CPDF_Dictionary methods take CFX_ByteString arguments

This will help avoid re-allocation of CFX_ByteStrings when the
caller already has one.  It may seem counter-intuitive that
requiring the caller to pass an allocated CFX_ByteString rather
than a static CFX_ByteStringC would improve the situation, but
due to the idiosyncrasies of std::map, the CPDF_Dictionary
methods must always do an allocation under the covers which
can't be avoided.

The changed callers in this CL are places where we would
previously demote to CFX_ByteStringC and then re-allocate
a new CFX_ByteString in the dictionary method.
==========

[Tom Sepez, 2016-04-14 17:25:32]

Description was changed from

==========
Make CPDF_Dictionary methods take CFX_ByteString arguments

This will help avoid re-allocation of CFX_ByteStrings when the
caller already has one.  It may seem counter-intuitive that
requiring the caller to pass an allocated CFX_ByteString rather
than a static CFX_ByteStringC would improve the situation, but
due to the idiosyncrasies of std::map, the CPDF_Dictionary
methods must always do an allocation under the covers which
can't be avoided.

The changed callers in this CL are places where we would
previously demote to CFX_ByteStringC and then re-allocate
a new CFX_ByteString in the dictionary method.
==========

to

==========
Make CPDF_Dictionary methods take CFX_ByteString arguments

This will help avoid duplicate allocation of CFX_ByteStrings
when the caller already has one.  It may seem counter-intuitive 
that requiring the caller to pass an allocated CFX_ByteString 
rather than a static CFX_ByteStringC would improve the situation,
but due to the idiosyncrasies of std::map, the CPDF_Dictionary
methods must always do an allocation under the covers which
can't be avoided.

The changed callers in this CL are places where we would
previously demote to CFX_ByteStringC and then allocate a
a duplicate CFX_ByteString in the dictionary method.
==========

[Tom Sepez, 2016-04-14 17:26:57]

On 2016/04/14 17:23:50, Tom Sepez wrote:
> Dan, avoid a few malloc/copies.
Hang on. Found a few more places.

[Tom Sepez, 2016-04-14 17:43:24]

Ok, please review.

[dsinclair, 2016-04-14 17:48:19]

lgtm w/ question.

https://codereview.chromium.org/1889863002/diff/20001/core/fpdfapi/fpdf_parse...
File core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/1889863002/diff/20001/core/fpdfapi/fpdf_parse...
core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:559:
vals[i]->SetAt(key.c_str(), new CPDF_Number(value));
I'm assuming we'll auto convert the const char* into a CFX_ByteString() ?

[Tom Sepez, 2016-04-14 17:51:49]

On 2016/04/14 17:48:19, dsinclair wrote:
> lgtm w/ question.
> 
>
https://codereview.chromium.org/1889863002/diff/20001/core/fpdfapi/fpdf_parse...
> File core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):
> 
>
https://codereview.chromium.org/1889863002/diff/20001/core/fpdfapi/fpdf_parse...
> core/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:559:
> vals[i]->SetAt(key.c_str(), new CPDF_Number(value));
> I'm assuming we'll auto convert the const char* into a CFX_ByteString() ?

Yes, the up-coversions from "C types" (esp. literals) are implicit, along the
lines of what std::string allows.

[Tom Sepez, 2016-04-14 17:52:03]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-04-14 17:52:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
dsinclair@chromium.org
Link to the patchset: https://codereview.chromium.org/1889863002/#ps40001
(title: "One last caller.")

[commit-bot: I haz the power, 2016-04-14 17:52:08]

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

[commit-bot: I haz the power, 2016-04-14 18:05:00]

Description was changed from

==========
Make CPDF_Dictionary methods take CFX_ByteString arguments

This will help avoid duplicate allocation of CFX_ByteStrings
when the caller already has one.  It may seem counter-intuitive 
that requiring the caller to pass an allocated CFX_ByteString 
rather than a static CFX_ByteStringC would improve the situation,
but due to the idiosyncrasies of std::map, the CPDF_Dictionary
methods must always do an allocation under the covers which
can't be avoided.

The changed callers in this CL are places where we would
previously demote to CFX_ByteStringC and then allocate a
a duplicate CFX_ByteString in the dictionary method.
==========

to

==========
Make CPDF_Dictionary methods take CFX_ByteString arguments

This will help avoid duplicate allocation of CFX_ByteStrings
when the caller already has one.  It may seem counter-intuitive 
that requiring the caller to pass an allocated CFX_ByteString 
rather than a static CFX_ByteStringC would improve the situation,
but due to the idiosyncrasies of std::map, the CPDF_Dictionary
methods must always do an allocation under the covers which
can't be avoided.

The changed callers in this CL are places where we would
previously demote to CFX_ByteStringC and then allocate a
a duplicate CFX_ByteString in the dictionary method.

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

[commit-bot: I haz the power, 2016-04-14 18:05:01]

Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/7b1ccf9697692844e764d730079a0f0b98fd...