Merge the CFWL_*Imp classes into the IFWL_* classes.

Merge the CFWL_*Imp classes into the IFWL_* classes.

This Cl merges the implementation classes up into the IFWL classes as the
intermediate classes were just acting as proxies.

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

[dsinclair, 2016-10-19 14:25:18]

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

[commit-bot: I haz the power, 2016-10-19 14:25:23]

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-10-19 14:26:52]

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

[commit-bot: I haz the power, 2016-10-19 14:26:53]

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

[dsinclair, 2016-10-19 14:29:25]

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

[dsinclair, 2016-10-19 14:29:27]

PTAL (red bots are patch failure due to patch chain)

I did not rename any of the CFWL_*DelegateImp classes as I'm hoping to merge the
IFWL_* classes up into the CFWL_* classes. The delegates will get fixed after
that is done.

I wouldn't bother going over this for nits as there are a lot of them. I'll do a
separate pass to cleanup the code later.

[dsinclair, 2016-10-19 15:00:24]

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

[commit-bot: I haz the power, 2016-10-19 15:00:29]

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-10-19 15:02:34]

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

[commit-bot: I haz the power, 2016-10-19 15:02:35]

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

[dsinclair, 2016-10-19 17:12:34]

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

[commit-bot: I haz the power, 2016-10-19 17:12:42]

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-10-19 17:26:31]

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

[commit-bot: I haz the power, 2016-10-19 17:26:32]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-10-19 19:12:33]

Any change this can be split into smaller pieces so it's easier to review?

Can we try tweaking the --similarity threshold so (all?) files that are 'A' show
up as 'A+' instead? [Insert High Expectations Father meme here]

https://codereview.chromium.org/2432423002/diff/30001/xfa/fde/ifde_txtedtengi...
File xfa/fde/ifde_txtedtengine.h (right):

https://codereview.chromium.org/2432423002/diff/30001/xfa/fde/ifde_txtedtengi...
xfa/fde/ifde_txtedtengine.h:14: class IFWL_Edit;
alphabetical order

[dsinclair, 2016-10-19 19:18:52]

https://codereview.chromium.org/2432423002/diff/30001/xfa/fde/ifde_txtedtengi...
File xfa/fde/ifde_txtedtengine.h (right):

https://codereview.chromium.org/2432423002/diff/30001/xfa/fde/ifde_txtedtengi...
xfa/fde/ifde_txtedtengine.h:14: class IFWL_Edit;
On 2016/10/19 19:12:33, Lei Zhang wrote:
> alphabetical order

Done.

[dsinclair, 2016-10-19 19:20:10]

On 2016/10/19 19:12:33, Lei Zhang wrote:
> Any change this can be split into smaller pieces so it's easier to review?

I don't think so, you have to merge the whole hierarchy in order for it to work,
so everything ends up changing.


> Can we try tweaking the --similarity threshold so (all?) files that are 'A'
show
> up as 'A+' instead? [Insert High Expectations Father meme here]
> 

Most of the files are A+, the ones that aren't are because they are extracted
from larger files. I don't think setting similarity is going to help.

[Lei Zhang, 2016-10-19 23:32:13]

On 2016/10/19 19:20:10, dsinclair wrote:
> On 2016/10/19 19:12:33, Lei Zhang wrote:
> > Any change this can be split into smaller pieces so it's easier to review?
> 
> I don't think so, you have to merge the whole hierarchy in order for it to
work,
> so everything ends up changing.

Aww, ok.

> > Can we try tweaking the --similarity threshold so (all?) files that are 'A'
> show
> > up as 'A+' instead? [Insert High Expectations Father meme here]
> > 
> 
> Most of the files are A+, the ones that aren't are because they are extracted
> from larger files. I don't think setting similarity is going to help.

I think it might. Can you give it a try? If it doesn't look good, you can always
delete the patch set.

For the 'A' files:
xfa/fwl/basewidget/fwl_formproxyimp.cpp -> xfa/fwl/core/ifwl_formproxy.cpp
xfa/fwl/basewidget/ifwl_listbox.h + xfa/fwl/basewidget/fwl_listboximp.h ->
xfa/fwl/core/ifwl_listbox.h

Some of the other ones are extracted from another file, but if Rietveld can
recognize it as a copy of file with some code deleted, that's still easier to
review than what looks like a completely new file.

[Lei Zhang, 2016-10-19 23:58:25]

lgtm

https://codereview.chromium.org/2432423002/diff/40001/xfa/fwl/core/ifwl_form.h
File xfa/fwl/core/ifwl_form.h (right):

https://codereview.chromium.org/2432423002/diff/40001/xfa/fwl/core/ifwl_form....
xfa/fwl/core/ifwl_form.h:63: typedef struct RestoreResizeInfo {
(for later) Drop the typedef?

https://codereview.chromium.org/2432423002/diff/40001/xfa/fwl/core/ifwl_form....
xfa/fwl/core/ifwl_form.h:185: friend class CFWL_FormImpDelegate;
move up

[dsinclair, 2016-10-20 12:51:04]

https://codereview.chromium.org/2432423002/diff/40001/xfa/fwl/core/ifwl_form.h
File xfa/fwl/core/ifwl_form.h (right):

https://codereview.chromium.org/2432423002/diff/40001/xfa/fwl/core/ifwl_form....
xfa/fwl/core/ifwl_form.h:63: typedef struct RestoreResizeInfo {
On 2016/10/19 23:58:25, Lei Zhang wrote:
> (for later) Drop the typedef?

Done.

https://codereview.chromium.org/2432423002/diff/40001/xfa/fwl/core/ifwl_form....
xfa/fwl/core/ifwl_form.h:185: friend class CFWL_FormImpDelegate;
On 2016/10/19 23:58:25, Lei Zhang wrote:
> move up

Done.

[dsinclair, 2016-10-20 12:52:42]

Patchset #7 (id:60001) has been deleted

[dsinclair, 2016-10-20 12:54:09]

Patchset #7 (id:70001) has been deleted

[dsinclair, 2016-10-20 12:55:03]

On 2016/10/19 23:32:13, Lei Zhang wrote:
> I think it might. Can you give it a try? If it doesn't look good, you can
always
> delete the patch set.

I tried it at a couple settings, it started matching some of the A files, but it
was matching them to the wrong original class so I removed the patchsets.

[dsinclair, 2016-10-20 12:55:15]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-10-20 12:55:16]

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/2432423002/#ps50001
(title: "Review feedback")

[commit-bot: I haz the power, 2016-10-20 12:55:26]

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

[dsinclair, 2016-10-20 13:18:51]

The CQ bit was unchecked by dsinclair@chromium.org

[dsinclair, 2016-10-20 13:18:53]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-10-20 13:19:03]

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-10-20 13:44:07]

Description was changed from

==========
Merge the CFWL_*Imp classes into the IFWL_* classes.

This Cl merges the implementation classes up into the IFWL classes as the
intermediate classes were just acting as proxies.
==========

to

==========
Merge the CFWL_*Imp classes into the IFWL_* classes.

This Cl merges the implementation classes up into the IFWL classes as the
intermediate classes were just acting as proxies.

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

[commit-bot: I haz the power, 2016-10-20 13:44:09]

Committed patchset #6 (id:50001) as
https://pdfium.googlesource.com/pdfium/+/fb5055947efd78e0e7f8a2bfd5b754c63dbb...