Make CPDF_Object subclass constructors intern strings

Make CPDF_Object subclass constructors intern strings

Make CDPF_Arrays intern the object they create.
Allow passing nullptr as a CFX_WeakPtr shortcut as well.

Committed: https://pdfium.googlesource.com/pdfium/+/5913a6ca71c85401e3f5317758d44a9fc4a667b2

[Tom Sepez, 2016-11-16 22:31:02]

Description was changed from

==========
Make CPDF_Name constructor intern strings
==========

to

==========
Make CPDF_Name constructor intern strings.

Allow passing nullptr as a shortcut as well.
==========

[Tom Sepez, 2016-11-16 22:34:51]

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

[commit-bot: I haz the power, 2016-11-16 22:34:55]

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-16 22:35:46]

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

[Tom Sepez, 2016-11-16 22:35:47]

Dan, for review.  This starts to move us away from a bunch of MaybeIntern()
calls and letting the object intern themselves.

https://codereview.chromium.org/2509123002/diff/1/core/fpdfapi/parser/cpdf_ar...
File core/fpdfapi/parser/cpdf_array.h (right):

https://codereview.chromium.org/2509123002/diff/1/core/fpdfapi/parser/cpdf_ar...
core/fpdfapi/parser/cpdf_array.h:59: typename std::enable_if<!std::is_same<T,
CPDF_Name>::value, T*>::type AddNew(
note: the return value type is nonsense if T is CPDF_Name, so it is silently
skipped for it. Otherwise the return type becomes T* as usual.

https://codereview.chromium.org/2509123002/diff/1/core/fpdfapi/parser/cpdf_ar...
core/fpdfapi/parser/cpdf_array.h:64: typename std::enable_if<std::is_same<T,
CPDF_Name>::value, T*>::type AddNew(
Exact opposite.  Inject a nullptr argument for the pool, subsequently arrays
will track actual pools in a follow-up CL.

[Tom Sepez, 2016-11-16 22:36:36]

https://codereview.chromium.org/2509123002/diff/1/core/fpdfapi/parser/cpdf_in...
File core/fpdfapi/parser/cpdf_indirect_object_holder.h (right):

https://codereview.chromium.org/2509123002/diff/1/core/fpdfapi/parser/cpdf_in...
core/fpdfapi/parser/cpdf_indirect_object_holder.h:47:
AddIndirectObject(pdfium::MakeUnique<T>(m_pByteStringPool, args...)));
Here, the indirect object holder injects its own bytestringpool into the ctor
call, transparently to the callers if the type is expected to take it.

[commit-bot: I haz the power, 2016-11-16 23:03:45]

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

[commit-bot: I haz the power, 2016-11-16 23:03:46]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-17 00:38:56]

Description was changed from

==========
Make CPDF_Name constructor intern strings.

Allow passing nullptr as a shortcut as well.
==========

to

==========
Make CPDF_Object subclass constructors intern strings

Make CDPF_Arrays intern the object they create.
Allow passing nullptr as a CFX_WeakPtr shortcut as well.
==========

[Tom Sepez, 2016-11-17 00:38:56]

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

[Tom Sepez, 2016-11-17 00:39:27]

I think "mostly OOO" is going to have to sign off on this one, as there may be
better C++ tricks to use.

[Tom Sepez, 2016-11-17 00:40:41]

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

[commit-bot: I haz the power, 2016-11-17 00:40:45]

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-17 00:48:55]

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

[commit-bot: I haz the power, 2016-11-17 00:48:56]

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

[Lei Zhang, 2016-11-17 00:52:13]

Android bots are red.

[Lei Zhang, 2016-11-17 01:05:06]

lgtm

https://codereview.chromium.org/2509123002/diff/20001/core/fpdfapi/parser/cpd...
File core/fpdfapi/parser/cpdf_string.cpp (right):

https://codereview.chromium.org/2509123002/diff/20001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_string.cpp:36: return pRet;
Old Android toolchain probably want a std::move() here.

https://codereview.chromium.org/2509123002/diff/20001/core/fpdfapi/parser/cpd...
File core/fpdfapi/parser/cpdf_syntax_parser.cpp (right):

https://codereview.chromium.org/2509123002/diff/20001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_syntax_parser.cpp:922: CFX_ByteString
CPDF_SyntaxParser::MaybeIntern(const CFX_ByteString& str) {
Still in use?

https://codereview.chromium.org/2509123002/diff/20001/fpdfsdk/fpdfeditpage.cpp
File fpdfsdk/fpdfeditpage.cpp (right):

https://codereview.chromium.org/2509123002/diff/20001/fpdfsdk/fpdfeditpage.cp...
fpdfsdk/fpdfeditpage.cpp:86: if
(FSDK_IsSandBoxPolicyEnabled(FPDF_POLICY_MACHINETIME_ACCESS))
braces

[Tom Sepez, 2016-11-17 01:13:42]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-11-17 01:13:43]

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/2509123002/#ps40001
(title: "Nits")

[Tom Sepez, 2016-11-17 01:13:48]

https://codereview.chromium.org/2509123002/diff/20001/core/fpdfapi/parser/cpd...
File core/fpdfapi/parser/cpdf_string.cpp (right):

https://codereview.chromium.org/2509123002/diff/20001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_string.cpp:36: return pRet;
On 2016/11/17 01:05:06, Lei Zhang (Mostly OOO) wrote:
> Old Android toolchain probably want a std::move() here.

Done.

https://codereview.chromium.org/2509123002/diff/20001/core/fpdfapi/parser/cpd...
File core/fpdfapi/parser/cpdf_syntax_parser.cpp (right):

https://codereview.chromium.org/2509123002/diff/20001/core/fpdfapi/parser/cpd...
core/fpdfapi/parser/cpdf_syntax_parser.cpp:922: CFX_ByteString
CPDF_SyntaxParser::MaybeIntern(const CFX_ByteString& str) {
On 2016/11/17 01:05:06, Lei Zhang (Mostly OOO) wrote:
> Still in use?

Removed.

https://codereview.chromium.org/2509123002/diff/20001/fpdfsdk/fpdfeditpage.cpp
File fpdfsdk/fpdfeditpage.cpp (right):

https://codereview.chromium.org/2509123002/diff/20001/fpdfsdk/fpdfeditpage.cp...
fpdfsdk/fpdfeditpage.cpp:86: if
(FSDK_IsSandBoxPolicyEnabled(FPDF_POLICY_MACHINETIME_ACCESS))
On 2016/11/17 01:05:06, Lei Zhang (Mostly OOO) wrote:
> braces

Done.

[commit-bot: I haz the power, 2016-11-17 01:13:50]

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-17 01:31:21]

Description was changed from

==========
Make CPDF_Object subclass constructors intern strings

Make CDPF_Arrays intern the object they create.
Allow passing nullptr as a CFX_WeakPtr shortcut as well.
==========

to

==========
Make CPDF_Object subclass constructors intern strings

Make CDPF_Arrays intern the object they create.
Allow passing nullptr as a CFX_WeakPtr shortcut as well.

Committed:
https://pdfium.googlesource.com/pdfium/+/5913a6ca71c85401e3f5317758d44a9fc4a6...
==========

[commit-bot: I haz the power, 2016-11-17 01:31:22]

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