Split fpdfsdk/fsdk_baseform.h into individual classes.

Split fpdfsdk/fsdk_baseform.h into individual classes.

This CL moves classes in fsdk_baseform.h to their own files.
Classes include CPDFSDK_Widget, CBA_AnnotIterator, CPDFSDK_XFAWidget,
PDFSDK_FieldAction, and CPDFSDK_Interform.

Committed: https://pdfium.googlesource.com/pdfium/+/611adb8c91c1d40c95097bb04b4f9eba03b599f9

[jaepark, 2016-08-16 17:37:22]

The CQ bit was checked by jaepark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 17:37: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-08-16 18:00:37]

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

[commit-bot: I haz the power, 2016-08-16 18:00:38]

Dry run: This issue passed the CQ dry run.

[jaepark, 2016-08-16 18:03:06]

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

[dsinclair, 2016-08-16 18:22:19]

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cba_annotiterat...
File fpdfsdk/cba_annotiterator.cpp (right):

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cba_annotiterat...
fpdfsdk/cba_annotiterator.cpp:59: // static
nit: move the static methods to the top above the constructor.

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cpdfsdk_interfo...
File fpdfsdk/cpdfsdk_interform.cpp (right):

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:58: m_iHighlightAlpha = 0;
Put this in the initializer list?

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:228: if (CPDF_FormField* pField =
Can these if's be turned into early returns so we have less indenting?

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:249: if (bRet) {
Combine the if() blocks.

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:280: if (pFormField->CountSelectedItems() > 0) {
combine if's

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:519: void
CPDFSDK_InterForm::SynchronizeField(CPDF_FormField* pFormField,
Group this with the block above.

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:557: if (bUrlEncoded) {
Combine the if's

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:659: if (pField->GetFieldType() ==
FIELDTYPE_LISTBOX) {
Invert and early return

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:668: if (nType == FIELDTYPE_CHECKBOX || nType ==
FIELDTYPE_RADIOBUTTON) {
Invert and early return.

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cpdfsdk_xfawidg...
File fpdfsdk/cpdfsdk_xfawidget.cpp (right):

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/cpdfsdk_xfawidg...
fpdfsdk/cpdfsdk_xfawidget.cpp:9: #ifdef PDF_ENABLE_XFA
Can we do this in the build files instead of wrapping the entire thing with
PDF_ENABLE_XFA?

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/include/cpdfsdk...
File fpdfsdk/include/cpdfsdk_interform.h (right):

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/include/cpdfsdk...
fpdfsdk/include/cpdfsdk_interform.h:95: #ifdef PDF_ENABLE_XFA
Move this up with the block above

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/include/cpdfsdk...
fpdfsdk/include/cpdfsdk_interform.h:142: static const int kNumFieldTypes = 7;
Move this up with the block above.

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/include/cpdfsdk...
File fpdfsdk/include/cpdfsdk_xfawidget.h (right):

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/include/cpdfsdk...
fpdfsdk/include/cpdfsdk_xfawidget.h:10: #ifdef PDF_ENABLE_XFA
Ditto build file instead of whole file ifdef

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/include/pdfsdk_...
File fpdfsdk/include/pdfsdk_fieldaction.h (right):

https://codereview.chromium.org/2252723002/diff/20001/fpdfsdk/include/pdfsdk_...
fpdfsdk/include/pdfsdk_fieldaction.h:13: typedef enum _PDFSDK_XFAAActionType {
Remove the _ from the start of the name as I believe it's technically reserved
for the compiler.

[jaepark, 2016-08-16 19:05:37]

Patchset #2 (id:20001) has been deleted

[jaepark, 2016-08-16 19:05:57]

Patchset #2 (id:40001) has been deleted

[jaepark, 2016-08-16 19:06:18]

The CQ bit was checked by jaepark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 19:06:26]

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

[jaepark, 2016-08-16 19:06:44]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-08-16 19:14:16]

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

[commit-bot: I haz the power, 2016-08-16 19:14:18]

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

[jaepark, 2016-08-16 20:07:36]

The CQ bit was checked by jaepark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 20:07:56]

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-16 20:20:58]

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

[commit-bot: I haz the power, 2016-08-16 20:20:59]

Dry run: This issue passed the CQ dry run.

[jaepark, 2016-08-16 20:21:32]

I've accidentally removed the patch with the comment, so I commented on where
modified.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cba_annotiterat...
File fpdfsdk/cba_annotiterator.cpp (right):

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cba_annotiterat...
fpdfsdk/cba_annotiterator.cpp:24: 
Moved static methods to the top above the constructor.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cpdfsdk_interfo...
File fpdfsdk/cpdfsdk_interform.cpp (right):

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:55: m_iHighlightAlpha(0) {
Put m_iHighlightAlpha in the initializer list.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:209: 
Moved to group PDF_ENABLE_XFA methods.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:235: 
Added early returns for less indenting.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:290: 
Combined if's.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:466: 
Combined if blocks.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:556: 
Combined if's.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:659: 
Early return

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:669: 
Early return

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cpdfsdk_xfawidg...
File fpdfsdk/cpdfsdk_xfawidget.cpp (right):

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/cpdfsdk_xfawidg...
fpdfsdk/cpdfsdk_xfawidget.cpp:6: 
Removed #ifdef PDF_ENABLE_XFA for the entire file.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/include/cpdfsdk...
File fpdfsdk/include/cpdfsdk_interform.h (right):

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/include/cpdfsdk...
fpdfsdk/include/cpdfsdk_interform.h:63: void SynchronizeField(CPDF_FormField*
pFormField, FX_BOOL bSynchronizeElse);
Moved to group PDF_ENABLE_XFA methods.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/include/cpdfsdk...
fpdfsdk/include/cpdfsdk_interform.h:132: static const int kNumFieldTypes = 6;
Moved to group PDF_ENABLE_XFA variables.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/include/cpdfsdk...
File fpdfsdk/include/cpdfsdk_xfawidget.h (right):

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/include/cpdfsdk...
fpdfsdk/include/cpdfsdk_xfawidget.h:9: 
Removed #ifdef PDF_ENABLE_XFA for the entire file.

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/include/pdfsdk_...
File fpdfsdk/include/pdfsdk_fieldaction.h (right):

https://codereview.chromium.org/2252723002/diff/60001/fpdfsdk/include/pdfsdk_...
fpdfsdk/include/pdfsdk_fieldaction.h:13: typedef enum {
Removed _PDFSDK_XFAAActionType from the start of the name.

[dsinclair, 2016-08-16 20:32:30]

https://codereview.chromium.org/2252723002/diff/80001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2252723002/diff/80001/BUILD.gn#newcode981
BUILD.gn:981: "fpdfsdk/cpdfsdk_xfawidget.cpp",
I think we want to keep these in the pdfium static library, you just need to add
a section with something like

if (pdf_enable_xfa) {
  sources += [
    ...
  ]
}

above.

https://codereview.chromium.org/2252723002/diff/80001/fpdfsdk/cpdfsdk_interfo...
File fpdfsdk/cpdfsdk_interform.cpp (right):

https://codereview.chromium.org/2252723002/diff/80001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:246: if (!(nType == FIELDTYPE_COMBOBOX || nType ==
FIELDTYPE_TEXTFIELD))
if (nType != FIELDTYPE_COMBOBOX && nType != FIELDTYPE_TEXTFIELD)

https://codereview.chromium.org/2252723002/diff/80001/pdfium.gyp
File pdfium.gyp (right):

https://codereview.chromium.org/2252723002/diff/80001/pdfium.gyp#newcode1140
pdfium.gyp:1140: 'fpdfsdk/cpdfsdk_xfawidget.cpp',
same as BUILD.gn, keep this in the pdfium shared library.

[jaepark, 2016-08-16 20:43:23]

The CQ bit was checked by jaepark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 20:43:30]

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-16 20:51:00]

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

[commit-bot: I haz the power, 2016-08-16 20:51:01]

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

[jaepark, 2016-08-16 20:55:46]

The CQ bit was checked by jaepark@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 20:55:51]

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-16 21:26:01]

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

[commit-bot: I haz the power, 2016-08-16 21:26:02]

Dry run: This issue passed the CQ dry run.

[jaepark, 2016-08-16 21:26:28]

https://codereview.chromium.org/2252723002/diff/80001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2252723002/diff/80001/BUILD.gn#newcode981
BUILD.gn:981: "fpdfsdk/cpdfsdk_xfawidget.cpp",
On 2016/08/16 20:32:30, dsinclair wrote:
> I think we want to keep these in the pdfium static library, you just need to
add
> a section with something like
> 
> if (pdf_enable_xfa) {
>   sources += [
>     ...
>   ]
> }
> 
> above.

Done.

https://codereview.chromium.org/2252723002/diff/80001/fpdfsdk/cpdfsdk_interfo...
File fpdfsdk/cpdfsdk_interform.cpp (right):

https://codereview.chromium.org/2252723002/diff/80001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:246: if (!(nType == FIELDTYPE_COMBOBOX || nType ==
FIELDTYPE_TEXTFIELD))
On 2016/08/16 20:32:30, dsinclair wrote:
> if (nType != FIELDTYPE_COMBOBOX && nType != FIELDTYPE_TEXTFIELD)

Done.

https://codereview.chromium.org/2252723002/diff/80001/pdfium.gyp
File pdfium.gyp (right):

https://codereview.chromium.org/2252723002/diff/80001/pdfium.gyp#newcode1140
pdfium.gyp:1140: 'fpdfsdk/cpdfsdk_xfawidget.cpp',
On 2016/08/16 20:32:30, dsinclair wrote:
> same as BUILD.gn, keep this in the pdfium shared library.

Done.

[dsinclair, 2016-08-17 18:34:11]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-08-17 18:34:12]

lgtm

[commit-bot: I haz the power, 2016-08-17 18:34:19]

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-17 18:34:40]

Description was changed from

==========
Split fpdfsdk/fsdk_baseform.h into individual classes.

This CL moves classes in fsdk_baseform.h to their own files.
Classes include CPDFSDK_Widget, CBA_AnnotIterator, CPDFSDK_XFAWidget,
PDFSDK_FieldAction, and CPDFSDK_Interform.
==========

to

==========
Split fpdfsdk/fsdk_baseform.h into individual classes.

This CL moves classes in fsdk_baseform.h to their own files.
Classes include CPDFSDK_Widget, CBA_AnnotIterator, CPDFSDK_XFAWidget,
PDFSDK_FieldAction, and CPDFSDK_Interform.

Committed:
https://pdfium.googlesource.com/pdfium/+/611adb8c91c1d40c95097bb04b4f9eba03b5...
==========

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

Committed patchset #4 (id:120001) as
https://pdfium.googlesource.com/pdfium/+/611adb8c91c1d40c95097bb04b4f9eba03b5...