autofill: bumped pickle version of FormFieldData

autofill: bumped pickle version of FormFieldData

Binary format was changed with
https://codereview.chromium.org/734983006.

BUG=450162
Committed: https://crrev.com/97dc995002ed64b09d7e49e43039ad243ff68bce
Cr-Commit-Position: refs/heads/master@{#312388}

[Lukasz Jagielski, 2015-01-20 11:01:18]

ljagielski@opera.com changed reviewers:
+ vabr@chromium.org

[Lukasz Jagielski, 2015-01-20 11:01:18]

Hi,
PTAL at my CL proposition.
Thanks,
Łukasz

[vabr (Chromium), 2015-01-20 11:19:57]

Thanks for fixing this!

The CL LGTM with minor nits.

Please note that you still need an approval from isherman@ or estade@. I can
only approve changes to password-only code, as specified in
components/autofill/OWNERS.

Cheers,
Vaclav

https://codereview.chromium.org/800543009/diff/1/components/autofill/core/com...
File components/autofill/core/common/form_field_data.cc (right):

https://codereview.chromium.org/800543009/diff/1/components/autofill/core/com...
components/autofill/core/common/form_field_data.cc:13: const int kPickleVersion
= 2;
Please add a big fat warning to bump the version if the FormFieldData struct is
changed.

https://codereview.chromium.org/800543009/diff/1/components/autofill/core/com...
components/autofill/core/common/form_field_data.cc:40: bool
ReadAsInt(PickleIterator* iter, T* direction) {
optional nit: While at it, could you please rename |direction| to something more
generic, like |destination|, |target_value| or similar?

[Lukasz Jagielski, 2015-01-20 11:43:32]

ljagielski@opera.com changed reviewers:
+ estade@chromium.org

[Lukasz Jagielski, 2015-01-20 12:13:35]

@estade, could you please take a look?
Thanks,
Łukasz

[Evan Stade, 2015-01-20 22:15:03]

whoops. lgtm. thanks.

[Lukasz Jagielski, 2015-01-21 08:18:46]

The CQ bit was checked by ljagielski@opera.com

[commit-bot: I haz the power, 2015-01-21 08:20:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/800543009/20001

[commit-bot: I haz the power, 2015-01-21 14:48:44]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-21 14:50:13]

Patchset 2 (id:??) landed as
https://crrev.com/97dc995002ed64b09d7e49e43039ad243ff68bce
Cr-Commit-Position: refs/heads/master@{#312388}

[Lei Zhang, 2015-01-23 06:43:01]

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

[Lei Zhang, 2015-01-23 06:43:01]

https://codereview.chromium.org/800543009/diff/20001/components/autofill/core...
File components/autofill/core/common/form_field_data.cc (right):

https://codereview.chromium.org/800543009/diff/20001/components/autofill/core...
components/autofill/core/common/form_field_data.cc:181: !ReadAsInt(iter,
&field_data->role) ||
I wish you put "role" at the end, so (de)serializing version 2 is the same as
version 1, but with one more bit of extra work. Oh well.

[Lukasz Jagielski, 2015-01-23 08:43:00]

On 2015/01/23 06:43:01, Lei Zhang wrote:
>
https://codereview.chromium.org/800543009/diff/20001/components/autofill/core...
> File components/autofill/core/common/form_field_data.cc (right):
> 
>
https://codereview.chromium.org/800543009/diff/20001/components/autofill/core...
> components/autofill/core/common/form_field_data.cc:181: !ReadAsInt(iter,
> &field_data->role) ||
> I wish you put "role" at the end, so (de)serializing version 2 is the same as
> version 1, but with one more bit of extra work. Oh well.

You're right, but can I still make changes in this review?
Regards,
Łukasz

[Evan Stade, 2015-01-23 16:50:14]

On 2015/01/23 08:43:00, Lukasz Jagielski wrote:
> On 2015/01/23 06:43:01, Lei Zhang wrote:
> >
>
https://codereview.chromium.org/800543009/diff/20001/components/autofill/core...
> > File components/autofill/core/common/form_field_data.cc (right):
> > 
> >
>
https://codereview.chromium.org/800543009/diff/20001/components/autofill/core...
> > components/autofill/core/common/form_field_data.cc:181: !ReadAsInt(iter,
> > &field_data->role) ||
> > I wish you put "role" at the end, so (de)serializing version 2 is the same
as
> > version 1, but with one more bit of extra work. Oh well.
> 
> You're right, but can I still make changes in this review?
> Regards,
> Łukasz

We can probably fix this with a follow up change, right? This patch is
merge-requested here https://code.google.com/p/chromium/issues/detail?id=450162
but as of yet hasn't gotten merged.