Pass element hash and name into CXFA_Object constructor.

Pass element hash and name into CXFA_Object constructor.

Providing the element hash and name in the constructor allows us to remove the
calls to XFA_GetElementByID in the get methods.

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

[dsinclair, 2016-06-28 14:46:19]

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

[dsinclair, 2016-06-28 14:46:20]

PTAL.

[Lei Zhang, 2016-06-28 15:12:03]

lgtm

https://codereview.chromium.org/2101403002/diff/1/xfa/fxfa/parser/xfa_object.h
File xfa/fxfa/parser/xfa_object.h (right):

https://codereview.chromium.org/2101403002/diff/1/xfa/fxfa/parser/xfa_object....
xfa/fxfa/parser/xfa_object.h:89: const CFX_WideStringC GetClassName() const;
Is the const useful?

https://codereview.chromium.org/2101403002/diff/1/xfa/fxfa/parser/xfa_script_...
File xfa/fxfa/parser/xfa_script_signaturepseudomodel.cpp (right):

https://codereview.chromium.org/2101403002/diff/1/xfa/fxfa/parser/xfa_script_...
xfa/fxfa/parser/xfa_script_signaturepseudomodel.cpp:26: 0x2457a033,
xfa_object.h probably needs a comment to explain where these hash values come
from.

[dsinclair, 2016-06-28 15:42:10]

PTAL. Changed how the hashes are handled.

https://codereview.chromium.org/2101403002/diff/1/xfa/fxfa/parser/xfa_object.h
File xfa/fxfa/parser/xfa_object.h (right):

https://codereview.chromium.org/2101403002/diff/1/xfa/fxfa/parser/xfa_object....
xfa/fxfa/parser/xfa_object.h:89: const CFX_WideStringC GetClassName() const;
On 2016/06/28 15:12:03, Lei Zhang wrote:
> Is the const useful?


Probably not, removed.

https://codereview.chromium.org/2101403002/diff/1/xfa/fxfa/parser/xfa_script_...
File xfa/fxfa/parser/xfa_script_signaturepseudomodel.cpp (right):

https://codereview.chromium.org/2101403002/diff/1/xfa/fxfa/parser/xfa_script_...
xfa/fxfa/parser/xfa_script_signaturepseudomodel.cpp:26: 0x2457a033,
On 2016/06/28 15:12:03, Lei Zhang wrote:
> xfa_object.h probably needs a comment to explain where these hash values come
> from.


I removed the numbers and just calculate them in CXFA_Object.

[dsinclair, 2016-07-06 20:06:37]

ping.

[Wei Li, 2016-07-06 22:19:28]

I have a general question: Does this mean we store the name for each object?
If so, would there be a lot of duplications? How about increased storage?

[dsinclair, 2016-07-07 01:44:31]

On 2016/07/06 22:19:28, Wei Li wrote:
> I have a general question: Does this mean we store the name for each object?
> If so, would there be a lot of duplications? How about increased storage?

Yes, it will store the name per object and they'll be duplicated (I believe the
WidestringC does a copy and Widestring doesn't?) I guess the amount of memory
would depend on the size of the XFA document, looking at the example files we
have, the XFA content isn't huge so I don't think it would be too bad. I don't
know if our XFA test files are in any way representative of what a file looks
like in the wild though.

In the long run, I'm trying to figure out if these can be turned into real
objects instead of a single object with a bunch of random data shoved into it.
I'm not sure if that will be achievable or not (I keep figuring out more bits of
XFA and ... it's complicated) but that's where I'd like to get too.

[Wei Li, 2016-07-07 21:15:46]

I don't have much experience about XFA files to estimate the cost either.
But I am concerned that we had a type indexed table, now we are joining it
with each object which means hashes and names are all duplicated.

One alternative to getting rid of hard-coded hash values is to generate them
during initialization, like you are doing in constructors.

[dsinclair, 2016-07-14 15:36:56]

On 2016/07/07 21:15:46, Wei Li wrote:
> I don't have much experience about XFA files to estimate the cost either.
> But I am concerned that we had a type indexed table, now we are joining it
> with each object which means hashes and names are all duplicated.
> 
> One alternative to getting rid of hard-coded hash values is to generate them
> during initialization, like you are doing in constructors.

What about converting to WideString instead of WideStringC? I believe WideString
is copy-on-write, correct? If so, that would save the memory we'd just have the
pointers in each file.

[Wei Li, 2016-07-15 21:47:53]

On 2016/07/14 15:36:56, dsinclair wrote:
> On 2016/07/07 21:15:46, Wei Li wrote:
> > I don't have much experience about XFA files to estimate the cost either.
> > But I am concerned that we had a type indexed table, now we are joining it
> > with each object which means hashes and names are all duplicated.
> > 
> > One alternative to getting rid of hard-coded hash values is to generate them
> > during initialization, like you are doing in constructors.
> 
> What about converting to WideString instead of WideStringC? I believe
WideString
> is copy-on-write, correct? If so, that would save the memory we'd just have
the
> pointers in each file.

Oh, missed that WideStringC is just a thin wrapper of the raw string. Should not
have extra storage. I guess hash values might be just a small extra.

lgtm

[dsinclair, 2016-07-18 13:23:31]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-07-18 13:23:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, weili@chromium.org
Link to the patchset: https://codereview.chromium.org/2101403002/#ps40001
(title: "Rebase to master")

[commit-bot: I haz the power, 2016-07-18 13:23:42]

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-18 13:36:55]

Description was changed from

==========
Pass element hash and name into CXFA_Object constructor.

Providing the element hash and name in the constructor allows us to remove the
calls to XFA_GetElementByID in the get methods.
==========

to

==========
Pass element hash and name into CXFA_Object constructor.

Providing the element hash and name in the constructor allows us to remove the
calls to XFA_GetElementByID in the get methods.

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

[commit-bot: I haz the power, 2016-07-18 13:36:56]

Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/c1df5d4375027aa9d0b88f456f95177c016e...

[commit-bot: I haz the power, 2016-07-18 13:36:58]

CQ bit was unchecked.