Move xfa_basic_imp to cxfa_widetextread.

Move xfa_basic_imp to cxfa_widetextread.

This Cl splits out the CXFA_WideTextRead class into it's own file. The helper
methods have been moved into xfa_utils.cpp and their pre-declarations into
xfa_utils.h.

Committed: https://pdfium.googlesource.com/pdfium/+/9eb0db1d7f6273de7eb28b0e1490eaf893e3bfb5

[dsinclair, 2016-07-20 14:36:10]

Patchset #1 (id:1) has been deleted

[dsinclair, 2016-07-20 14:36:37]

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

[dsinclair, 2016-07-20 14:36:38]

PTAL.

[Lei Zhang, 2016-07-20 20:54:34]

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_me...
File xfa/fxfa/parser/cxfa_measurement.h (right):

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_me...
xfa/fxfa/parser/cxfa_measurement.h:16: explicit CXFA_Measurement(const
CFX_WideStringC& wsMeasure) {
Can we move the non-accessor impls out of the header?

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_sc...
File xfa/fxfa/parser/cxfa_scriptcontext.cpp (right):

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_sc...
xfa/fxfa/parser/cxfa_scriptcontext.cpp:81: while (iElementIndex != -1) {
Should we check we never go out of bound w.r.t. g_XFAScriptIndex? i.e. < 0 and
>= arraysize(g_XFAScriptIndex).

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_sc...
xfa/fxfa/parser/cxfa_scriptcontext.cpp:92: int32_t iMid = (iStart + iEnd) / 2;
std::lower_bound() later?

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_wi...
File xfa/fxfa/parser/cxfa_widetextread.cpp (right):

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_wi...
xfa/fxfa/parser/cxfa_widetextread.cpp:45: if (m_iPosition < 0)
std::min / max

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_wi...
xfa/fxfa/parser/cxfa_widetextread.cpp:109: void CXFA_WideTextRead::Lock() {}
Hmm. :)

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

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/xfa_uti...
xfa/fxfa/parser/xfa_utils_imp.cpp:236: const XFA_SCRIPTATTRIBUTEINFO*
XFA_GetScriptAttributeByName(
This looks a lot like GetMethodByName() in
xfa/fxfa/parser/cxfa_scriptcontext.cpp.

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/xfa_uti...
xfa/fxfa/parser/xfa_utils_imp.cpp:368: return eName == XFA_Element::Unknown
!= and reverse the two return values?

[dsinclair, 2016-07-21 13:34:24]

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_me...
File xfa/fxfa/parser/cxfa_measurement.h (right):

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_me...
xfa/fxfa/parser/cxfa_measurement.h:16: explicit CXFA_Measurement(const
CFX_WideStringC& wsMeasure) {
On 2016/07/20 20:54:33, Lei Zhang wrote:
> Can we move the non-accessor impls out of the header?

Done.

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_sc...
File xfa/fxfa/parser/cxfa_scriptcontext.cpp (right):

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_sc...
xfa/fxfa/parser/cxfa_scriptcontext.cpp:81: while (iElementIndex != -1) {
On 2016/07/20 20:54:33, Lei Zhang wrote:
> Should we check we never go out of bound w.r.t. g_XFAScriptIndex? i.e. < 0 and
> >= arraysize(g_XFAScriptIndex).

Done.

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_sc...
xfa/fxfa/parser/cxfa_scriptcontext.cpp:92: int32_t iMid = (iStart + iEnd) / 2;
On 2016/07/20 20:54:33, Lei Zhang wrote:
> std::lower_bound() later?

Acknowledged.

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_wi...
File xfa/fxfa/parser/cxfa_widetextread.cpp (right):

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_wi...
xfa/fxfa/parser/cxfa_widetextread.cpp:45: if (m_iPosition < 0)
On 2016/07/20 20:54:33, Lei Zhang wrote:
> std::min / max

Done.

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/cxfa_wi...
xfa/fxfa/parser/cxfa_widetextread.cpp:109: void CXFA_WideTextRead::Lock() {}
On 2016/07/20 20:54:33, Lei Zhang wrote:
> Hmm. :)


Removed. I'd assumed it was an override, but looks like it isn't.

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

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/xfa_uti...
xfa/fxfa/parser/xfa_utils_imp.cpp:236: const XFA_SCRIPTATTRIBUTEINFO*
XFA_GetScriptAttributeByName(
On 2016/07/20 20:54:33, Lei Zhang wrote:
> This looks a lot like GetMethodByName() in
> xfa/fxfa/parser/cxfa_scriptcontext.cpp.

Yes, and the ones below as well, it's a very similar function duplicated like 6
times that all need to get cleaned up at some point.

https://codereview.chromium.org/2165993002/diff/80001/xfa/fxfa/parser/xfa_uti...
xfa/fxfa/parser/xfa_utils_imp.cpp:368: return eName == XFA_Element::Unknown
On 2016/07/20 20:54:33, Lei Zhang wrote:
> != and reverse the two return values?

Done.

[Lei Zhang, 2016-07-21 19:00:57]

lgtm

[dsinclair, 2016-07-21 19:01:15]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-07-21 19:01:18]

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-07-21 19:01:42]

Description was changed from

==========
Move xfa_basic_imp to cxfa_widetextread.

This Cl splits out the CXFA_WideTextRead class into it's own file. The helper
methods have been moved into xfa_utils.cpp and their pre-declarations into
xfa_utils.h.
==========

to

==========
Move xfa_basic_imp to cxfa_widetextread.

This Cl splits out the CXFA_WideTextRead class into it's own file. The helper
methods have been moved into xfa_utils.cpp and their pre-declarations into
xfa_utils.h.

Committed:
https://pdfium.googlesource.com/pdfium/+/9eb0db1d7f6273de7eb28b0e1490eaf893e3...
==========

[commit-bot: I haz the power, 2016-07-21 19:01:43]

Committed patchset #6 (id:120001) as
https://pdfium.googlesource.com/pdfium/+/9eb0db1d7f6273de7eb28b0e1490eaf893e3...