Pass CFX_*StringCs to FX_HashCode_GETA and _GETW hash functions.

Pass CFX_*StringCs to FX_HashCode_GETA and _GETW hash functions.

Too many calls were of the form fn(x.c_str(), x.GetLength())
which is an anti-pattern given the StringC classes which tie
these together.

There are a few places where explicit CFX_*StringCs are
constructed, but this can be avoided by changing the args
to these functions in the same manner.

Removed String_ from name of functions since it added
little value. Also removed default argument.

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

[Tom Sepez, 2016-04-22 22:44:20]

Description was changed from

==========
Pass CFX_*StringCs to FX_HashCode_GETA and _GETW hash functions.

Too many calls were of the form fn(x.c_str(), x.GetLength())
which is an anti-pattern give the StringC classes which tie
these together.

There are a few places where explicit CFX_*StringCs are
constructed, but this can be avoided by changing the args
to these functions in the same manner.

Removed String_ from name of functions since it added
little value. Also removed default argument.
==========

to

==========
Pass CFX_*StringCs to FX_HashCode_GETA and _GETW hash functions.

Too many calls were of the form fn(x.c_str(), x.GetLength())
which is an anti-pattern given the StringC classes which tie
these together.

There are a few places where explicit CFX_*StringCs are
constructed, but this can be avoided by changing the args
to these functions in the same manner.

Removed String_ from name of functions since it added
little value. Also removed default argument.
==========

[Tom Sepez, 2016-04-22 22:44:21]

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

[Tom Sepez, 2016-04-22 22:44:32]

Lei, please review.

[Lei Zhang, 2016-04-23 01:33:29]

https://codereview.chromium.org/1919563002/diff/140001/xfa/fde/css/fde_csssty...
File xfa/fde/css/fde_cssstylesheet.h (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fde/css/fde_csssty...
xfa/fde/css/fde_cssstylesheet.h:18: FX_BOOL bIgnoreCase)
Also bool, 4 callers in xfa/fde/css/fde_cssstylesheet.cpp.

https://codereview.chromium.org/1919563002/diff/140001/xfa/fgas/crt/fgas_code...
File xfa/fgas/crt/fgas_codepage.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fgas/crt/fgas_code...
xfa/fgas/crt/fgas_codepage.cpp:312: uint32_t uHash =
FX_HashCode_GetA(CFX_ByteStringC(pStr, iLength), true);
The only FX_GetCodePageFromStringA() caller is on line 342 below. Maybe just
combine the two?

https://codereview.chromium.org/1919563002/diff/140001/xfa/fgas/font/fgas_std...
File xfa/fgas/font/fgas_stdfontmgr.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fgas/font/fgas_std...
xfa/fgas/font/fgas_stdfontmgr.cpp:234: CFX_ByteStringC((uint8_t*)buffer, 3 *
sizeof(void*)), false);
sizeof(buffer) ?

https://codereview.chromium.org/1919563002/diff/140001/xfa/fgas/font/fgas_std...
xfa/fgas/font/fgas_stdfontmgr.cpp:849: CFX_ByteStringC((const FX_CHAR*)Hash, 2 *
sizeof(void*)), false);
Ditto, more below.

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/app/xfa_textl...
File xfa/fxfa/app/xfa_textlayout.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/app/xfa_textl...
xfa/fxfa/app/xfa_textlayout.cpp:1643: uint32_t dwAlgin = pTabstops->dwAlign;
s/dwAlgin/dwAlign/

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/fm2js/xfa_fm2...
File xfa/fxfa/fm2js/xfa_fm2jscontext.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/fm2js/xfa_fm2...
xfa/fxfa/fm2js/xfa_fm2jscontext.cpp:4244: int32_t iEnd =
(sizeof(reservesForDecode) / sizeof(reservesForDecode[0])) - 1;
FX_ArraySize() ?

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/parser/xfa_do...
File xfa/fxfa/parser/xfa_document_datamerger_imp.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/parser/xfa_do...
xfa/fxfa/parser/xfa_document_datamerger_imp.cpp:423: for (CXFA_Node
*pCurDataScope = pDataScope, *pLastDataScope = NULL;
My eyes!

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/parser/xfa_ob...
File xfa/fxfa/parser/xfa_object_imp.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/parser/xfa_ob...
xfa/fxfa/parser/xfa_object_imp.cpp:4951: m_dwNameHash = FX_HashCode_GetW(wsName,
false);
Not sure if it matters that in the else case, this gets set to 0 now.

[Tom Sepez, 2016-04-25 16:50:21]

https://codereview.chromium.org/1919563002/diff/140001/xfa/fde/css/fde_csssty...
File xfa/fde/css/fde_cssstylesheet.h (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fde/css/fde_csssty...
xfa/fde/css/fde_cssstylesheet.h:18: FX_BOOL bIgnoreCase)
On 2016/04/23 01:33:29, Lei Zhang wrote:
> Also bool, 4 callers in xfa/fde/css/fde_cssstylesheet.cpp.

Done.

https://codereview.chromium.org/1919563002/diff/140001/xfa/fgas/crt/fgas_code...
File xfa/fgas/crt/fgas_codepage.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fgas/crt/fgas_code...
xfa/fgas/crt/fgas_codepage.cpp:312: uint32_t uHash =
FX_HashCode_GetA(CFX_ByteStringC(pStr, iLength), true);
On 2016/04/23 01:33:29, Lei Zhang wrote:
> The only FX_GetCodePageFromStringA() caller is on line 342 below. Maybe just
> combine the two?

I think its better as part of a follow-up to kill the handwritten bsearchs.

https://codereview.chromium.org/1919563002/diff/140001/xfa/fgas/font/fgas_std...
File xfa/fgas/font/fgas_stdfontmgr.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fgas/font/fgas_std...
xfa/fgas/font/fgas_stdfontmgr.cpp:234: CFX_ByteStringC((uint8_t*)buffer, 3 *
sizeof(void*)), false);
On 2016/04/23 01:33:29, Lei Zhang wrote:
> sizeof(buffer) ?

Done.

https://codereview.chromium.org/1919563002/diff/140001/xfa/fgas/font/fgas_std...
xfa/fgas/font/fgas_stdfontmgr.cpp:849: CFX_ByteStringC((const FX_CHAR*)Hash, 2 *
sizeof(void*)), false);
On 2016/04/23 01:33:29, Lei Zhang wrote:
> Ditto, more below.

Done.

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/app/xfa_textl...
File xfa/fxfa/app/xfa_textlayout.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/app/xfa_textl...
xfa/fxfa/app/xfa_textlayout.cpp:1643: uint32_t dwAlgin = pTabstops->dwAlign;
On 2016/04/23 01:33:29, Lei Zhang wrote:
> s/dwAlgin/dwAlign/

Done, global grep for Algin.

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/fm2js/xfa_fm2...
File xfa/fxfa/fm2js/xfa_fm2jscontext.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/fm2js/xfa_fm2...
xfa/fxfa/fm2js/xfa_fm2jscontext.cpp:4244: int32_t iEnd =
(sizeof(reservesForDecode) / sizeof(reservesForDecode[0])) - 1;
On 2016/04/23 01:33:29, Lei Zhang wrote:
> FX_ArraySize() ?
yup.

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/parser/xfa_do...
File xfa/fxfa/parser/xfa_document_datamerger_imp.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/parser/xfa_do...
xfa/fxfa/parser/xfa_document_datamerger_imp.cpp:423: for (CXFA_Node
*pCurDataScope = pDataScope, *pLastDataScope = NULL;
On 2016/04/23 01:33:29, Lei Zhang wrote:
> My eyes!
Made slightly better with both for-loops having one clause per line, and
re-ordering an ||.

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/parser/xfa_ob...
File xfa/fxfa/parser/xfa_object_imp.cpp (right):

https://codereview.chromium.org/1919563002/diff/140001/xfa/fxfa/parser/xfa_ob...
xfa/fxfa/parser/xfa_object_imp.cpp:4951: m_dwNameHash = FX_HashCode_GetW(wsName,
false);
On 2016/04/23 01:33:29, Lei Zhang wrote:
> Not sure if it matters that in the else case, this gets set to 0 now.

restored.

[Lei Zhang, 2016-04-25 17:53:15]

lgtm

[Tom Sepez, 2016-04-25 18:23:22]

The CQ bit was checked by tsepez@chromium.org

[commit-bot: I haz the power, 2016-04-25 18:23:28]

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

[commit-bot: I haz the power, 2016-04-25 18:23:50]

Description was changed from

==========
Pass CFX_*StringCs to FX_HashCode_GETA and _GETW hash functions.

Too many calls were of the form fn(x.c_str(), x.GetLength())
which is an anti-pattern given the StringC classes which tie
these together.

There are a few places where explicit CFX_*StringCs are
constructed, but this can be avoided by changing the args
to these functions in the same manner.

Removed String_ from name of functions since it added
little value. Also removed default argument.
==========

to

==========
Pass CFX_*StringCs to FX_HashCode_GETA and _GETW hash functions.

Too many calls were of the form fn(x.c_str(), x.GetLength())
which is an anti-pattern given the StringC classes which tie
these together.

There are a few places where explicit CFX_*StringCs are
constructed, but this can be avoided by changing the args
to these functions in the same manner.

Removed String_ from name of functions since it added
little value. Also removed default argument.

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

[commit-bot: I haz the power, 2016-04-25 18:23:50]

Committed patchset #9 (id:160001) as
https://pdfium.googlesource.com/pdfium/+/b6853cfe4fd1ee089dfdd0cb09bbc4063532...