fgas/ code cleanup.

fgas/ code cleanup.

This CL shuffles code around in the fgas/ headers, removes unused functions
and adds anonymous namepaces for static methods and data.

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

[dsinclair, 2016-05-18 21:15:34]

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

[dsinclair, 2016-05-18 21:15:36]

PTAL.

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/app/xfa_ffdoc.cpp
File xfa/fxfa/app/xfa_ffdoc.cpp (right):

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/app/xfa_ffdoc.cpp#...
xfa/fxfa/app/xfa_ffdoc.cpp:32: struct FX_BASE64DATA {
This struct is duplicated, but given what it contains/does I thought that was
acceptable.

[Tom Sepez, 2016-05-18 22:01:22]

Still got red bots.

https://codereview.chromium.org/1992033002/diff/1/xfa/fde/css/fde_cssdatatabl...
File xfa/fde/css/fde_cssdatatable.cpp (right):

https://codereview.chromium.org/1992033002/diff/1/xfa/fde/css/fde_cssdatatabl...
xfa/fde/css/fde_cssdatatable.cpp:16: const uint8_t g_FXHex2DecMap[256] = {
seems like a lot of wasted space.  In particular we could lose the top 128 and
replace it with a check below.

https://codereview.chromium.org/1992033002/diff/1/xfa/fgas/crt/fgas_algorithm.h
File xfa/fgas/crt/fgas_algorithm.h (right):

https://codereview.chromium.org/1992033002/diff/1/xfa/fgas/crt/fgas_algorithm...
xfa/fgas/crt/fgas_algorithm.h:15: class CFX_DSPATemplate {
this needs to go someday.

https://codereview.chromium.org/1992033002/diff/1/xfa/fgas/crt/fgas_stream.cpp
File xfa/fgas/crt/fgas_stream.cpp (right):

https://codereview.chromium.org/1992033002/diff/1/xfa/fgas/crt/fgas_stream.cp...
xfa/fgas/crt/fgas_stream.cpp:277: int32_t filelength(FXSYS_FILE* file) {
Our naming would have functions starting with a capital, maybe FileLength()

https://codereview.chromium.org/1992033002/diff/1/xfa/fgas/crt/fgas_stream.cp...
xfa/fgas/crt/fgas_stream.cpp:290: FX_BOOL fsetsize(FXSYS_FILE* file, int32_t
size) {
maybe FileSetSize()

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/app/xfa_ffdoc.cpp
File xfa/fxfa/app/xfa_ffdoc.cpp (right):

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/app/xfa_ffdoc.cpp#...
xfa/fxfa/app/xfa_ffdoc.cpp:32: struct FX_BASE64DATA {
On 2016/05/18 21:15:35, dsinclair wrote:
> This struct is duplicated, but given what it contains/does I thought that was
> acceptable.

Acknowledged.

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/app/xfa_ffdoc.cpp#...
xfa/fxfa/app/xfa_ffdoc.cpp:55: 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF,
Again, we could probably save 128 bytes here.

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/app/xfa_ffdoc.cpp#...
xfa/fxfa/app/xfa_ffdoc.cpp:220: Base64DecodeW(wsPDFContent.c_str(),
wsPDFContent.GetLength(), NULL);
nit: nullptr

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/parser/xfa_layout_...
File xfa/fxfa/parser/xfa_layout_itemlayout.cpp (right):

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/parser/xfa_layout_...
xfa/fxfa/parser/xfa_layout_itemlayout.cpp:33: if (pStr == NULL) {
nit: !pStr, no braces

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/parser/xfa_layout_...
xfa/fxfa/parser/xfa_layout_itemlayout.cpp:36: if (iStrLen < 0) {
no {}

[dsinclair, 2016-05-19 14:24:44]

https://codereview.chromium.org/1992033002/diff/1/xfa/fde/css/fde_cssdatatabl...
File xfa/fde/css/fde_cssdatatable.cpp (right):

https://codereview.chromium.org/1992033002/diff/1/xfa/fde/css/fde_cssdatatabl...
xfa/fde/css/fde_cssdatatable.cpp:16: const uint8_t g_FXHex2DecMap[256] = {
On 2016/05/18 22:01:21, Tom Sepez wrote:
> seems like a lot of wasted space.  In particular we could lose the top 128 and
> replace it with a check below.

Removed completely, we already have a FXSYS_toHexDigit, so use that.

https://codereview.chromium.org/1992033002/diff/1/xfa/fgas/crt/fgas_algorithm.h
File xfa/fgas/crt/fgas_algorithm.h (right):

https://codereview.chromium.org/1992033002/diff/1/xfa/fgas/crt/fgas_algorithm...
xfa/fgas/crt/fgas_algorithm.h:15: class CFX_DSPATemplate {
On 2016/05/18 22:01:21, Tom Sepez wrote:
> this needs to go someday.

Acknowledged.

https://codereview.chromium.org/1992033002/diff/1/xfa/fgas/crt/fgas_stream.cpp
File xfa/fgas/crt/fgas_stream.cpp (right):

https://codereview.chromium.org/1992033002/diff/1/xfa/fgas/crt/fgas_stream.cp...
xfa/fgas/crt/fgas_stream.cpp:277: int32_t filelength(FXSYS_FILE* file) {
On 2016/05/18 22:01:21, Tom Sepez wrote:
> Our naming would have functions starting with a capital, maybe FileLength()

Done.

https://codereview.chromium.org/1992033002/diff/1/xfa/fgas/crt/fgas_stream.cp...
xfa/fgas/crt/fgas_stream.cpp:290: FX_BOOL fsetsize(FXSYS_FILE* file, int32_t
size) {
On 2016/05/18 22:01:21, Tom Sepez wrote:
> maybe FileSetSize()

Done.

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/app/xfa_ffdoc.cpp
File xfa/fxfa/app/xfa_ffdoc.cpp (right):

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/app/xfa_ffdoc.cpp#...
xfa/fxfa/app/xfa_ffdoc.cpp:55: 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF,
On 2016/05/18 22:01:21, Tom Sepez wrote:
> Again, we could probably save 128 bytes here.

Done.

Please check my numbers are right.

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/app/xfa_ffdoc.cpp#...
xfa/fxfa/app/xfa_ffdoc.cpp:220: Base64DecodeW(wsPDFContent.c_str(),
wsPDFContent.GetLength(), NULL);
On 2016/05/18 22:01:21, Tom Sepez wrote:
> nit: nullptr

Done.

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/parser/xfa_layout_...
File xfa/fxfa/parser/xfa_layout_itemlayout.cpp (right):

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/parser/xfa_layout_...
xfa/fxfa/parser/xfa_layout_itemlayout.cpp:33: if (pStr == NULL) {
On 2016/05/18 22:01:22, Tom Sepez wrote:
> nit: !pStr, no braces

Done.

https://codereview.chromium.org/1992033002/diff/1/xfa/fxfa/parser/xfa_layout_...
xfa/fxfa/parser/xfa_layout_itemlayout.cpp:36: if (iStrLen < 0) {
On 2016/05/18 22:01:21, Tom Sepez wrote:
> no {}

Done.

[Tom Sepez, 2016-05-19 21:23:42]

LGTM with comment.

https://codereview.chromium.org/1992033002/diff/60001/xfa/fxfa/app/xfa_ffdoc.cpp
File xfa/fxfa/app/xfa_ffdoc.cpp (right):

https://codereview.chromium.org/1992033002/diff/60001/xfa/fxfa/app/xfa_ffdoc....
xfa/fxfa/app/xfa_ffdoc.cpp:54: uint8_t base64DecoderValue(uint8_t val) {
The numbers look right, alternatively you might want to stick to multiples of
16, eg, trim 0..31 and 128..255 only.  Gets you most of the bang without weird
numbers.

[dsinclair, 2016-05-20 01:06:31]

https://codereview.chromium.org/1992033002/diff/60001/xfa/fxfa/app/xfa_ffdoc.cpp
File xfa/fxfa/app/xfa_ffdoc.cpp (right):

https://codereview.chromium.org/1992033002/diff/60001/xfa/fxfa/app/xfa_ffdoc....
xfa/fxfa/app/xfa_ffdoc.cpp:54: uint8_t base64DecoderValue(uint8_t val) {
On 2016/05/19 21:23:42, Tom Sepez wrote:
> The numbers look right, alternatively you might want to stick to multiples of
> 16, eg, trim 0..31 and 128..255 only.  Gets you most of the bang without weird
> numbers.

I think it's going to be strange either way. I think the variable names make it
clear what's going on.

[dsinclair, 2016-05-20 01:06:41]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-05-20 01:06:53]

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

[commit-bot: I haz the power, 2016-05-20 01:07:14]

Description was changed from

==========
fgas/ code cleanup.

This CL shuffles code around in the fgas/ headers, removes unused functions
and adds anonymous namepaces for static methods and data.
==========

to

==========
fgas/ code cleanup.

This CL shuffles code around in the fgas/ headers, removes unused functions
and adds anonymous namepaces for static methods and data.

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

[commit-bot: I haz the power, 2016-05-20 01:07:15]

Committed patchset #4 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/fc2cdf8657534467fd807e216d50650b0e95...