AutoFill crash in CreditCard::SetInfo().

AutoFill crash in CreditCard::SetInfo().

Fix for crash seen in the field.  The AutoFillType class was not thread-safe.  These changes
make it so.  Also adds unit tests that cover mappings from type to group and subgroup, and
out of range conditions.

BUG=42341
TEST=AutoFillTypeTest

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=45650

[James Hawkins, 2010-04-26 06:24:06]

Please wait for my lgtm on this one.

[James Hawkins, 2010-04-26 17:32:51]

http://codereview.chromium.org/1789001/diff/4001/5001
File chrome/browser/autofill/autofill_type.cc (right):

http://codereview.chromium.org/1789001/diff/4001/5001#newcode13
chrome/browser/autofill/autofill_type.cc:13: 
namespace {

http://codereview.chromium.org/1789001/diff/4001/5004
File chrome/browser/webdata/web_database.cc (right):

http://codereview.chromium.org/1789001/diff/4001/5004#newcode1672
chrome/browser/webdata/web_database.cc:1672: bool
WebDatabase::GetCreditCardForID(int credit_card_id, CreditCard** credit_card) {
80cols

http://codereview.chromium.org/1789001/diff/4001/5005
File chrome/browser/webdata/web_database.h (right):

http://codereview.chromium.org/1789001/diff/4001/5005#newcode230
chrome/browser/webdata/web_database.h:230: virtual bool
GetAutoFillProfileForID(int profile_id, AutoFillProfile** profile);
Why have you made all of these virtual?

[dhollowa, 2010-04-26 18:26:51]

http://codereview.chromium.org/1789001/diff/4001/5001
File chrome/browser/autofill/autofill_type.cc (right):

http://codereview.chromium.org/1789001/diff/4001/5001#newcode13
chrome/browser/autofill/autofill_type.cc:13: 
On 2010/04/26 17:32:51, James Hawkins wrote:
> namespace {

Done.

http://codereview.chromium.org/1789001/diff/4001/5004
File chrome/browser/webdata/web_database.cc (right):

http://codereview.chromium.org/1789001/diff/4001/5004#newcode1672
chrome/browser/webdata/web_database.cc:1672: bool
WebDatabase::GetCreditCardForID(int credit_card_id, CreditCard** credit_card) {
On 2010/04/26 17:32:51, James Hawkins wrote:
> 80cols

Done.

http://codereview.chromium.org/1789001/diff/4001/5005
File chrome/browser/webdata/web_database.h (right):

http://codereview.chromium.org/1789001/diff/4001/5005#newcode230
chrome/browser/webdata/web_database.h:230: virtual bool
GetAutoFillProfileForID(int profile_id, AutoFillProfile** profile);
On 2010/04/26 17:32:51, James Hawkins wrote:
> Why have you made all of these virtual?

To make consistent with the profile versions (above).  Also, this allows mocking
in unit tests.

[James Hawkins, 2010-04-26 18:39:03]

http://codereview.chromium.org/1789001/diff/4001/5005
File chrome/browser/webdata/web_database.h (right):

http://codereview.chromium.org/1789001/diff/4001/5005#newcode230
chrome/browser/webdata/web_database.h:230: virtual bool
GetAutoFillProfileForID(int profile_id, AutoFillProfile** profile);
On 2010/04/26 18:26:51, dhollowa wrote:
> On 2010/04/26 17:32:51, James Hawkins wrote:
> > Why have you made all of these virtual?
> 
> To make consistent with the profile versions (above).  Also, this allows
mocking
> in unit tests.

I think I wrote this, but I don't remember adding the virtual keyword.  If
they're not currently overridden, the virtual keyword needs to be removed.

[dhollowa, 2010-04-26 19:26:40]

http://codereview.chromium.org/1789001/diff/4001/5005
File chrome/browser/webdata/web_database.h (right):

http://codereview.chromium.org/1789001/diff/4001/5005#newcode230
chrome/browser/webdata/web_database.h:230: virtual bool
GetAutoFillProfileForID(int profile_id, AutoFillProfile** profile);
On 2010/04/26 18:39:04, James Hawkins wrote:
> On 2010/04/26 18:26:51, dhollowa wrote:
> > On 2010/04/26 17:32:51, James Hawkins wrote:
> > > Why have you made all of these virtual?
> > 
> > To make consistent with the profile versions (above).  Also, this allows
> mocking
> > in unit tests.
> 
> I think I wrote this, but I don't remember adding the virtual keyword.  If
> they're not currently overridden, the virtual keyword needs to be removed.

Done.
I left the destructor virtual for safety's sake.  Though it doesn't strictly
need to be so.  Let me know if you feel strongly about that.

[dhollowa, 2010-04-26 19:44:34]

Reverting virtual method change. They're needed for mock in sync unit tests.

[James Hawkins, 2010-04-26 19:47:06]

http://codereview.chromium.org/1789001/diff/25001/26005
File chrome/browser/webdata/web_database.h (right):

http://codereview.chromium.org/1789001/diff/25001/26005#newcode230
chrome/browser/webdata/web_database.h:230: virtual bool
GetAutoFillProfileForID(int profile_id, AutoFillProfile** profile);
Going back to the original comment, please don't add virtual if they're not
needed.

[dhollowa, 2010-04-26 20:06:54]

http://codereview.chromium.org/1789001/diff/25001/26005
File chrome/browser/webdata/web_database.h (right):

http://codereview.chromium.org/1789001/diff/25001/26005#newcode230
chrome/browser/webdata/web_database.h:230: virtual bool
GetAutoFillProfileForID(int profile_id, AutoFillProfile** profile);
On 2010/04/26 19:47:06, James Hawkins wrote:
> Going back to the original comment, please don't add virtual if they're not
> needed.

Done.

[James Hawkins, 2010-04-26 20:46:38]

LGTM