[XFA] Force destruction order of font managers.

[XFA] Force destruction order of font managers.

The GEFont points to the font manager which creates it and tries to unregister
itself. Currently the GEFont can be created by the default mapper and then
stored in a different mapper. If the default mapper is destroyed first, when
the second mapper cleans up the font there will be a call to unregister on
the default mapper causing a use-after-free.

The long term fix is to fixup the GEFont so it points to the correct mapper
to unregister from. This CL forces the destruction order in CXFA_FFApp to
cleanup the non-default mapper first.

BUG=chromium:637546
Committed: https://pdfium.googlesource.com/pdfium/+/837735660808d52580703183ae24a3c7c7b05c7d

[dsinclair, 2016-08-18 15:49:30]

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

[dsinclair, 2016-08-18 15:49:31]

PTAL.

This isn't the issue reported, but it happens with the same input file so added
the BUG= line.

[dsinclair, 2016-08-18 15:50:45]

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

[commit-bot: I haz the power, 2016-08-18 15:50:47]

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-08-18 15:58:57]

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

[commit-bot: I haz the power, 2016-08-18 15:58:57]

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

[dsinclair, 2016-08-18 16:05:13]

On 2016/08/18 15:58:57, commit-bot: I haz the power wrote:
> Dry run: Try jobs failed on following builders:
>   mac_xfa_rel_gyp on master.tryserver.client.pdfium (JOB_FAILED,
>
https://build.chromium.org/p/tryserver.client.pdfium/builders/mac_xfa_rel_gyp...)

Test failures, ignore this for now ..... Will ping when fixed.

[dsinclair, 2016-08-18 17:54:24]

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

[commit-bot: I haz the power, 2016-08-18 17:54:33]

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-08-18 18:10:40]

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

[commit-bot: I haz the power, 2016-08-18 18:10:41]

Dry run: This issue passed the CQ dry run.

[dsinclair, 2016-08-18 18:13:52]

K, bots green. Turns out the theme needs to be deleted before the font manager
because the theme accesses the font on deletion. Another thing to untangle ....

[Wei Li, 2016-08-18 22:10:58]

https://codereview.chromium.org/2259823004/diff/20001/xfa/fxfa/app/xfa_ffapp.cpp
File xfa/fxfa/app/xfa_ffapp.cpp (right):

https://codereview.chromium.org/2259823004/diff/20001/xfa/fxfa/app/xfa_ffapp....
xfa/fxfa/app/xfa_ffapp.cpp:105: m_pFDEFontMgr.reset();
Can you just exchange the order of |m_pFDEFontMgr| and |m_pFontMgr| in the
header?

[dsinclair, 2016-08-22 15:04:04]

I can't trigger this anymore, going to close this CL.

[dsinclair, 2016-08-23 13:53:47]

Description was changed from

==========
Force destruction order of font managers.

The GEFont points to the font manager which creates it and tries to unregister
itself. Currently the GEFont can be created by the default mapper and then
stored in a different mapper. If the default mapper is destroyed first, when
the second mapper cleans up the font there will be a call to unregister on
the default mapper causing a use-after-free.

The long term fix is to fixup the GEFont so it points to the correct mapper
to unregister from. This CL forces the destruction order in CXFA_FFApp to
cleanup the non-default mapper first.

BUG=chromium:637546
==========

to

==========
Force destruction order of font managers.

The GEFont points to the font manager which creates it and tries to unregister
itself. Currently the GEFont can be created by the default mapper and then
stored in a different mapper. If the default mapper is destroyed first, when
the second mapper cleans up the font there will be a call to unregister on
the default mapper causing a use-after-free.

The long term fix is to fixup the GEFont so it points to the correct mapper
to unregister from. This CL forces the destruction order in CXFA_FFApp to
cleanup the non-default mapper first.

BUG=chromium:637546
==========

[dsinclair, 2016-08-23 13:59:43]

Re-opened, this requires XFA which I'd disabled without realizing.

https://codereview.chromium.org/2259823004/diff/20001/xfa/fxfa/app/xfa_ffapp.cpp
File xfa/fxfa/app/xfa_ffapp.cpp (right):

https://codereview.chromium.org/2259823004/diff/20001/xfa/fxfa/app/xfa_ffapp....
xfa/fxfa/app/xfa_ffapp.cpp:105: m_pFDEFontMgr.reset();
On 2016/08/18 22:10:58, Wei Li wrote:
> Can you just exchange the order of |m_pFDEFontMgr| and |m_pFontMgr| in the
> header? 

Done.

[dsinclair, 2016-08-23 14:00:16]

Description was changed from

==========
Force destruction order of font managers.

The GEFont points to the font manager which creates it and tries to unregister
itself. Currently the GEFont can be created by the default mapper and then
stored in a different mapper. If the default mapper is destroyed first, when
the second mapper cleans up the font there will be a call to unregister on
the default mapper causing a use-after-free.

The long term fix is to fixup the GEFont so it points to the correct mapper
to unregister from. This CL forces the destruction order in CXFA_FFApp to
cleanup the non-default mapper first.

BUG=chromium:637546
==========

to

==========
[XFA] Force destruction order of font managers.

The GEFont points to the font manager which creates it and tries to unregister
itself. Currently the GEFont can be created by the default mapper and then
stored in a different mapper. If the default mapper is destroyed first, when
the second mapper cleans up the font there will be a call to unregister on
the default mapper causing a use-after-free.

The long term fix is to fixup the GEFont so it points to the correct mapper
to unregister from. This CL forces the destruction order in CXFA_FFApp to
cleanup the non-default mapper first.

BUG=chromium:637546
==========

[Wei Li, 2016-08-23 16:33:47]

lgtm

[dsinclair, 2016-08-23 18:22:06]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-08-23 18:22:17]

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-08-23 18:39:28]

Description was changed from

==========
[XFA] Force destruction order of font managers.

The GEFont points to the font manager which creates it and tries to unregister
itself. Currently the GEFont can be created by the default mapper and then
stored in a different mapper. If the default mapper is destroyed first, when
the second mapper cleans up the font there will be a call to unregister on
the default mapper causing a use-after-free.

The long term fix is to fixup the GEFont so it points to the correct mapper
to unregister from. This CL forces the destruction order in CXFA_FFApp to
cleanup the non-default mapper first.

BUG=chromium:637546
==========

to

==========
[XFA] Force destruction order of font managers.

The GEFont points to the font manager which creates it and tries to unregister
itself. Currently the GEFont can be created by the default mapper and then
stored in a different mapper. If the default mapper is destroyed first, when
the second mapper cleans up the font there will be a call to unregister on
the default mapper causing a use-after-free.

The long term fix is to fixup the GEFont so it points to the correct mapper
to unregister from. This CL forces the destruction order in CXFA_FFApp to
cleanup the non-default mapper first.

BUG=chromium:637546

Committed:
https://pdfium.googlesource.com/pdfium/+/837735660808d52580703183ae24a3c7c7b0...
==========

[commit-bot: I haz the power, 2016-08-23 18:39:29]

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