Fix KDE Wallet backward compatibility when (de)serializing FormData.

Fix KDE Wallet backward compatibility when (de)serializing FormData.

The problem was introduced by https://codereview.chromium.org/365783002

BUG=404615
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290677

[joleksy, 2014-08-07 12:06:53]

@Reviewer: does it look ok to you?

[Ilya Sherman, 2014-08-07 19:23:31]

Thanks for the fix!  Could you add test coverage?  Also, please file a bug for
this, and update the CL description to reference the bug.

https://codereview.chromium.org/443873011/diff/1/components/autofill/core/com...
File components/autofill/core/common/form_data.cc (left):

https://codereview.chromium.org/443873011/diff/1/components/autofill/core/com...
components/autofill/core/common/form_data.cc:119: 
nit: Please preserve this newline.

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

https://codereview.chromium.org/443873011/diff/1/components/autofill/core/com...
components/autofill/core/common/form_data.cc:53: << " FormData from pickle.";
nit: Please use DVLOG(1) here.  (I do see that the file is currently using
LOG(ERROR).  If you could update those throughout, that would be great :)

https://codereview.chromium.org/443873011/diff/1/components/autofill/core/com...
components/autofill/core/common/form_data.cc:148: }
nit: No need for these curly braces.

[joleksy, 2014-08-18 13:11:49]

On 2014/08/07 19:23:31, Ilya Sherman wrote:
> Thanks for the fix!  Could you add test coverage?  Also, please file a bug for
> this, and update the CL description to reference the bug.

Done. Sorry for the delay.

[Ilya Sherman, 2014-08-18 21:05:46]

Thanks!  Please also add test coverage for the error cases: a missing version
number and a bad format for the current version number.

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

https://codereview.chromium.org/443873011/diff/20001/components/autofill/core...
components/autofill/core/common/form_data_unittest.cc:14: void
SerializeFormData_v1(const autofill::FormData& form_data, Pickle* pickle) {
nit: Please document this function.

https://codereview.chromium.org/443873011/diff/20001/components/autofill/core...
components/autofill/core/common/form_data_unittest.cc:14: void
SerializeFormData_v1(const autofill::FormData& form_data, Pickle* pickle) {
nit: Please name this something more like "SerializeFormDataInVersion1Format",
or maybe just "SerializeInVersion1Format" to keep the name shorter.

https://codereview.chromium.org/443873011/diff/20001/components/autofill/core...
components/autofill/core/common/form_data_unittest.cc:26: }
nit: Please leave a blank line after this one.

[Ilya Sherman, 2014-08-18 21:06:57]

Also, is there any sort of data cleanup that we can/should do for affected
users?

[joleksy, 2014-08-19 06:53:48]

On 2014/08/18 21:06:57, Ilya Sherman wrote:
> Also, is there any sort of data cleanup that we can/should do for affected
> users?

Nope, the affected users are doomed.

Note: we *could* try to introduce some extremely complex heuristics in order to
discover binary data with incorrect version number, but IMHO it is not really
worth it. And users should expect the unstable version to be, well, unstable;-)

[Ilya Sherman, 2014-08-19 20:45:02]

LGTM, thanks.  Let's make sure that this gets merged into the M38 branch.

[Ilya Sherman, 2014-08-19 20:45:06]

The CQ bit was checked by isherman@chromium.org

[commit-bot: I haz the power, 2014-08-19 20:46:07]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/joleksy@opera.com/443873011/40001

[commit-bot: I haz the power, 2014-08-19 22:12:06]

Committed patchset #3 (40001) as 290677