Clean up XFA code which causes warnings

Clean up XFA code which causes warnings

This is part of efforts to bring XFA to chromium_code standard.
The warnings are from unreachable code, or using potentially
uninitialized variables, or using assignment within a condition.

This change list only contains easy to fix cases. More cleanups
will follow.

BUG=pdfium:29
Committed: https://pdfium.googlesource.com/pdfium/+/038aa531bcf1a76764d3cef46fd9b8e08b166dae

[Wei Li, 2016-05-20 00:21:51]

Description was changed from

==========
Clean up XFA code which causes warnings - part I

This mainly includes code which has unreachable code, or potentially
use uninitialized variables, or use assignment within a condition.

This change list mainly contains easy to fix cases.

BUG=pdfium:29
==========

to

==========
Clean up XFA code which causes warnings - part I

The warnings are from unreachable code, or using potentially
uninitialized variables, or using assignment within a condition.

This change list mainly contains easy to fix cases.

BUG=pdfium:29
==========

[Wei Li, 2016-05-20 00:51:06]

Patchset #1 (id:1) has been deleted

[Wei Li, 2016-05-20 01:01:03]

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

[Wei Li, 2016-05-20 01:01:04]

pls review, thanks

[Lei Zhang, 2016-05-20 03:48:02]

What build settings trigger these warnings?

https://codereview.chromium.org/1998873002/diff/20001/xfa/fde/cfde_txtedtengi...
File xfa/fde/cfde_txtedtengine.cpp (left):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fde/cfde_txtedtengi...
xfa/fde/cfde_txtedtengine.cpp:1611: while (nCountRange > 0) {
What's wrong here? Should you initialize |nSelStart| instead?

https://codereview.chromium.org/1998873002/diff/20001/xfa/fde/cfde_txtedtengi...
File xfa/fde/cfde_txtedtengine.cpp (right):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fde/cfde_txtedtengi...
xfa/fde/cfde_txtedtengine.cpp:1604: int32_t nSelCount;
Merge with line 1606.

https://codereview.chromium.org/1998873002/diff/20001/xfa/fwl/core/fwl_widget...
File xfa/fwl/core/fwl_widgetmgrimp.cpp (right):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fwl/core/fwl_widget...
xfa/fwl/core/fwl_widgetmgrimp.cpp:475: while (pNext) {
I think this is missing the initial call to GetSiblingRadioButton() ?

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
File xfa/fxfa/app/xfa_ffwidget.cpp (right):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
xfa/fxfa/app/xfa_ffwidget.cpp:271: static void XFA_GetMatrix(CFX_Matrix& m,
BTW, this only has 1 caller and |at| is always XFA_ATTRIBUTEENUM_TopLeft.

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
xfa/fxfa/app/xfa_ffwidget.cpp:278: FX_FLOAT fAnchorX = 0, fAnchorY = 0;
One per line please.

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
xfa/fxfa/app/xfa_ffwidget.cpp:1162: FX_FLOAT sx = 0.0f;
Can you declare variables closer to where they are used?

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
xfa/fxfa/app/xfa_ffwidget.cpp:1198: { halfBefore = edgeBefore.GetThickness() /
2; }
There's some extra braces here.

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
File xfa/fxfa/app/xfa_ffwidgetacc.cpp (right):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
xfa/fxfa/app/xfa_ffwidgetacc.cpp:716: while ((pWidget = GetNextWidget(pWidget))
!= nullptr) {
Why do we need the explicit check against nullptr?

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/parser/xfa_uti...
File xfa/fxfa/parser/xfa_utils_imp.cpp (right):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/parser/xfa_uti...
xfa/fxfa/parser/xfa_utils_imp.cpp:258: return TRUE;
delete this line and fall through?

[Wei Li, 2016-05-20 06:30:42]

Description was changed from

==========
Clean up XFA code which causes warnings - part I

The warnings are from unreachable code, or using potentially
uninitialized variables, or using assignment within a condition.

This change list mainly contains easy to fix cases.

BUG=pdfium:29
==========

to

==========
Clean up XFA code which causes warnings - part I

This is part of efforts to bring XFA to chromium_code standard.
The warnings are from unreachable code, or using potentially
uninitialized variables, or using assignment within a condition.

This change list mainly contains easy to fix cases.

BUG=pdfium:29
==========

[Wei Li, 2016-05-20 16:28:22]

Patchset #2 (id:40001) has been deleted

[Wei Li, 2016-05-20 16:33:15]

I am trying to enable 'chromium_code' on XFA as well. Added that into the desc
as well. Here is the config change (eventually):
https://codereview.chromium.org/1996463002/diff/100001/xfa.gyp

https://codereview.chromium.org/1998873002/diff/20001/xfa/fde/cfde_txtedtengi...
File xfa/fde/cfde_txtedtengine.cpp (left):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fde/cfde_txtedtengi...
xfa/fde/cfde_txtedtengine.cpp:1611: while (nCountRange > 0) {
On 2016/05/20 03:48:01, Lei Zhang wrote:
> What's wrong here? Should you initialize |nSelStart| instead?

Yes, the warning is about |nSelStart| may not be initialized. Changing to
do-while makes the immediate assignment explicit. But initializing it would be
ok too.

https://codereview.chromium.org/1998873002/diff/20001/xfa/fde/cfde_txtedtengi...
File xfa/fde/cfde_txtedtengine.cpp (right):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fde/cfde_txtedtengi...
xfa/fde/cfde_txtedtengine.cpp:1604: int32_t nSelCount;
On 2016/05/20 03:48:01, Lei Zhang wrote:
> Merge with line 1606.

Done.

https://codereview.chromium.org/1998873002/diff/20001/xfa/fwl/core/fwl_widget...
File xfa/fwl/core/fwl_widgetmgrimp.cpp (right):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fwl/core/fwl_widget...
xfa/fwl/core/fwl_widgetmgrimp.cpp:475: while (pNext) {
On 2016/05/20 03:48:01, Lei Zhang wrote:
> I think this is missing the initial call to GetSiblingRadioButton() ?

I merge the condition in line 474 into the loop, So the first loop will just
check for pRadioButton before calling on the sibling button.

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
File xfa/fxfa/app/xfa_ffwidget.cpp (right):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
xfa/fxfa/app/xfa_ffwidget.cpp:271: static void XFA_GetMatrix(CFX_Matrix& m,
On 2016/05/20 03:48:02, Lei Zhang wrote:
> BTW, this only has 1 caller and |at| is always XFA_ATTRIBUTEENUM_TopLeft.

Yes, I noticed that. I suspect there are some unfinished code as 
we probably want support on rotation on other points as well.
So keep this utility function for now.

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
xfa/fxfa/app/xfa_ffwidget.cpp:278: FX_FLOAT fAnchorX = 0, fAnchorY = 0;
On 2016/05/20 03:48:02, Lei Zhang wrote:
> One per line please.

Done.

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
xfa/fxfa/app/xfa_ffwidget.cpp:1162: FX_FLOAT sx = 0.0f;
On 2016/05/20 03:48:02, Lei Zhang wrote:
> Can you declare variables closer to where they are used?

Done.

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
xfa/fxfa/app/xfa_ffwidget.cpp:1198: { halfBefore = edgeBefore.GetThickness() /
2; }
On 2016/05/20 03:48:02, Lei Zhang wrote:
> There's some extra braces here.

Done.

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
File xfa/fxfa/app/xfa_ffwidgetacc.cpp (right):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/app/xfa_ffwidg...
xfa/fxfa/app/xfa_ffwidgetacc.cpp:716: while ((pWidget = GetNextWidget(pWidget))
!= nullptr) {
On 2016/05/20 03:48:02, Lei Zhang wrote:
> Why do we need the explicit check against nullptr?

This is due to msvs warning c4706 "assignment within conditional expression".
The warning prevents people accidentally write "if (a=b)" instead of "if(a==b)".

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/parser/xfa_uti...
File xfa/fxfa/parser/xfa_utils_imp.cpp (right):

https://codereview.chromium.org/1998873002/diff/20001/xfa/fxfa/parser/xfa_uti...
xfa/fxfa/parser/xfa_utils_imp.cpp:258: return TRUE;
On 2016/05/20 03:48:02, Lei Zhang wrote:
> delete this line and fall through?

Done.

[Wei Li, 2016-05-20 19:27:13]

Description was changed from

==========
Clean up XFA code which causes warnings - part I

This is part of efforts to bring XFA to chromium_code standard.
The warnings are from unreachable code, or using potentially
uninitialized variables, or using assignment within a condition.

This change list mainly contains easy to fix cases.

BUG=pdfium:29
==========

to

==========
Clean up XFA code which causes warnings

This is part of efforts to bring XFA to chromium_code standard.
The warnings are from unreachable code, or using potentially
uninitialized variables, or using assignment within a condition.

This change list only contains easy to fix cases.

BUG=pdfium:29
==========

[Wei Li, 2016-05-20 19:27:54]

Description was changed from

==========
Clean up XFA code which causes warnings

This is part of efforts to bring XFA to chromium_code standard.
The warnings are from unreachable code, or using potentially
uninitialized variables, or using assignment within a condition.

This change list only contains easy to fix cases.

BUG=pdfium:29
==========

to

==========
Clean up XFA code which causes warnings

This is part of efforts to bring XFA to chromium_code standard.
The warnings are from unreachable code, or using potentially
uninitialized variables, or using assignment within a condition.

This change list only contains easy to fix cases. More cleanups
will follow.

BUG=pdfium:29
==========

[Lei Zhang, 2016-05-20 21:59:22]

lgtm

[Wei Li, 2016-05-20 22:04:10]

The CQ bit was checked by weili@chromium.org

[commit-bot: I haz the power, 2016-05-20 22:04:14]

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

[commit-bot: I haz the power, 2016-05-20 22:38:33]

Description was changed from

==========
Clean up XFA code which causes warnings

This is part of efforts to bring XFA to chromium_code standard.
The warnings are from unreachable code, or using potentially
uninitialized variables, or using assignment within a condition.

This change list only contains easy to fix cases. More cleanups
will follow.

BUG=pdfium:29
==========

to

==========
Clean up XFA code which causes warnings

This is part of efforts to bring XFA to chromium_code standard.
The warnings are from unreachable code, or using potentially
uninitialized variables, or using assignment within a condition.

This change list only contains easy to fix cases. More cleanups
will follow.

BUG=pdfium:29

Committed:
https://pdfium.googlesource.com/pdfium/+/038aa531bcf1a76764d3cef46fd9b8e08b16...
==========

[commit-bot: I haz the power, 2016-05-20 22:38:33]

Committed patchset #2 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/038aa531bcf1a76764d3cef46fd9b8e08b16...