Remove CFX_PrivateData from CPDF_ModuleMgr

Remove CFX_PrivateData from CPDF_ModuleMgr

Its only used to store one object, so replace it with a unique_ptr
to a class with a virtual dtor.  Rename the prototypical class with
virtual dtor from CFX_DestructObject to CFX_Deletable. Rename the
fx_basic_module.cpp to cfx_modulemgr.cpp to match the one class
in it.

Committed: https://pdfium.googlesource.com/pdfium/+/ddffb57cf9763e2612e9f6f5730f334691adb692

[Tom Sepez, 2016-05-24 21:25:42]

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

[Tom Sepez, 2016-05-24 21:25:42]

Lei, for review.

[Lei Zhang, 2016-05-24 21:29:43]

Description was changed from

==========
Remove CFX_PrivateData from CPDF_ModuleMgr

Its only used to store one object, so replace it with a unqiue_ptr
to a class with a virtual dtor.  Rename the prototypical class with
virtual dtor from CFX_DestructObject to CFX_Deletable. Rename the
fx_basic_module.cpp to cfx_modulemgr.cpp to match the one class
in it.
==========

to

==========
Remove CFX_PrivateData from CPDF_ModuleMgr

Its only used to store one object, so replace it with a unique_ptr
to a class with a virtual dtor.  Rename the prototypical class with
virtual dtor from CFX_DestructObject to CFX_Deletable. Rename the
fx_basic_module.cpp to cfx_modulemgr.cpp to match the one class
in it.
==========

[Lei Zhang, 2016-05-24 21:34:58]

https://codereview.chromium.org/2013483003/diff/1/core/fpdfapi/include/cpdf_m...
File core/fpdfapi/include/cpdf_modulemgr.h (right):

https://codereview.chromium.org/2013483003/diff/1/core/fpdfapi/include/cpdf_m...
core/fpdfapi/include/cpdf_modulemgr.h:36: // Takes ownership of |pAdapter|.
There's only 1 caller - change the param to a unique_ptr?

https://codereview.chromium.org/2013483003/diff/1/core/fxcodec/codec/fx_codec...
File core/fxcodec/codec/fx_codec_jbig.cpp (right):

https://codereview.chromium.org/2013483003/diff/1/core/fxcodec/codec/fx_codec...
core/fxcodec/codec/fx_codec_jbig.cpp:53: CFX_Deletable* pDestructObject =
pDeletableObject

https://codereview.chromium.org/2013483003/diff/1/pdfium.gyp
File pdfium.gyp (right):

https://codereview.chromium.org/2013483003/diff/1/pdfium.gyp#newcode267
pdfium.gyp:267: 'core/fpdfapi/cpdf_modulemgr.cpp',
- double quotes

- can we sort this by ascii order once and for all?

[Tom Sepez, 2016-05-24 21:56:56]

https://codereview.chromium.org/2013483003/diff/1/core/fpdfapi/include/cpdf_m...
File core/fpdfapi/include/cpdf_modulemgr.h (right):

https://codereview.chromium.org/2013483003/diff/1/core/fpdfapi/include/cpdf_m...
core/fpdfapi/include/cpdf_modulemgr.h:36: // Takes ownership of |pAdapter|.
On 2016/05/24 21:34:58, Lei Zhang wrote:
> There's only 1 caller - change the param to a unique_ptr?

Done.

https://codereview.chromium.org/2013483003/diff/1/core/fxcodec/codec/fx_codec...
File core/fxcodec/codec/fx_codec_jbig.cpp (right):

https://codereview.chromium.org/2013483003/diff/1/core/fxcodec/codec/fx_codec...
core/fxcodec/codec/fx_codec_jbig.cpp:53: CFX_Deletable* pDestructObject =
On 2016/05/24 21:34:58, Lei Zhang wrote:
> pDeletableObject

Done.

https://codereview.chromium.org/2013483003/diff/1/pdfium.gyp
File pdfium.gyp (right):

https://codereview.chromium.org/2013483003/diff/1/pdfium.gyp#newcode267
pdfium.gyp:267: 'core/fpdfapi/cpdf_modulemgr.cpp',
On 2016/05/24 21:34:58, Lei Zhang wrote:
> - double quotes
> 
> - can we sort this by ascii order once and for all?
Ah, case sensitivity is what shuffled this up.

[Lei Zhang, 2016-05-24 22:07:50]

https://codereview.chromium.org/2013483003/diff/20001/pdfium.gyp
File pdfium.gyp (left):

https://codereview.chromium.org/2013483003/diff/20001/pdfium.gyp#oldcode385
pdfium.gyp:385: "core/fpdfapi/fpdf_page/cpdf_pagemodule.cpp",
Doh.

https://codereview.chromium.org/2013483003/diff/20001/pdfium.gyp
File pdfium.gyp (right):

https://codereview.chromium.org/2013483003/diff/20001/pdfium.gyp#newcode312
pdfium.gyp:312: 'core/fpdfapi/fpdf_cmaps/Japan1/UniJIS-UCS2-H_4.cpp',
Not ASCII order. 'W' is 97 and '_' is 95. Basically my gripe is this list
doesn't match BUILD.gn, ordering-wise.

[Lei Zhang, 2016-05-24 22:08:13]

Otherwise lgtm

[Tom Sepez, 2016-05-24 23:08:58]

https://codereview.chromium.org/2013483003/diff/20001/pdfium.gyp
File pdfium.gyp (right):

https://codereview.chromium.org/2013483003/diff/20001/pdfium.gyp#newcode312
pdfium.gyp:312: 'core/fpdfapi/fpdf_cmaps/Japan1/UniJIS-UCS2-H_4.cpp',
On 2016/05/24 22:07:50, Lei Zhang wrote:
> Not ASCII order. 'W' is 97 and '_' is 95. Basically my gripe is this list
> doesn't match BUILD.gn, ordering-wise.

You mean 'W' is 87, '_', is 95, 'w' is 119. So its that that case-insensitive
thing.  Do we pretend to upcase everything and sort, or do we pretend to
downcase everything and sort?

I'll manually put these two back where they were.  Otherwise, the other
re-arrangements look like they fix actual issues.

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

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-05-24 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/2013483003/#ps40001
(title: "Re-order two lines in .gyp file.")

[commit-bot: I haz the power, 2016-05-24 23:10:16]

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

[Lei Zhang, 2016-05-24 23:16:03]

On 2016/05/24 23:08:58, Tom Sepez wrote:
> https://codereview.chromium.org/2013483003/diff/20001/pdfium.gyp
> File pdfium.gyp (right):
> 
> https://codereview.chromium.org/2013483003/diff/20001/pdfium.gyp#newcode312
> pdfium.gyp:312: 'core/fpdfapi/fpdf_cmaps/Japan1/UniJIS-UCS2-H_4.cpp',
> On 2016/05/24 22:07:50, Lei Zhang wrote:
> > Not ASCII order. 'W' is 97 and '_' is 95. Basically my gripe is this list
> > doesn't match BUILD.gn, ordering-wise.
> 
> You mean 'W' is 87, '_', is 95, 'w' is 119. So its that that case-insensitive
> thing.  Do we pretend to upcase everything and sort, or do we pretend to
> downcase everything and sort?

I'll sort the list again in a separate CL. Basically different locales sort
differently, so at least in Chromium land the convension as I understand it is
to use ASCII sort to avoid a back and forth tug of war between developers on
machines with different locales.

$ echo -e 'w\nW\n_' | LANG=en_US.UTF-8 sort
_
w
W

vs

$ echo -e 'w\nW\n_' | LANG=C sort
W
_
w

[commit-bot: I haz the power, 2016-05-24 23:20:35]

Description was changed from

==========
Remove CFX_PrivateData from CPDF_ModuleMgr

Its only used to store one object, so replace it with a unique_ptr
to a class with a virtual dtor.  Rename the prototypical class with
virtual dtor from CFX_DestructObject to CFX_Deletable. Rename the
fx_basic_module.cpp to cfx_modulemgr.cpp to match the one class
in it.
==========

to

==========
Remove CFX_PrivateData from CPDF_ModuleMgr

Its only used to store one object, so replace it with a unique_ptr
to a class with a virtual dtor.  Rename the prototypical class with
virtual dtor from CFX_DestructObject to CFX_Deletable. Rename the
fx_basic_module.cpp to cfx_modulemgr.cpp to match the one class
in it.

Committed:
https://pdfium.googlesource.com/pdfium/+/ddffb57cf9763e2612e9f6f5730f334691ad...
==========

[commit-bot: I haz the power, 2016-05-24 23:20:36]

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