Autofill: code cleanup.

Autofill: code cleanup.

- Change some AutofillTable methds to return scoped_ptrs.
- Remove dead code.
- Add/update comments for constants copied to JS.

Committed: https://crrev.com/6f0467529c1d941ae4200daf0ad971d28ea7f1d4
Cr-Commit-Position: refs/heads/master@{#344639}

[Lei Zhang, 2015-08-20 00:51:43]

[email protected] changed reviewers:
+ [email protected]

[Evan Stade, 2015-08-20 17:20:41]

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
File components/autofill/core/browser/webdata/autofill_table.cc (right):

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:701: for (const
FormFieldData& data : elements) {
nit: s/data/element

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:891:
std::vector<AutofillProfile*>* profiles) {
how much work would it be to update this to ScopedVector?

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:905:
profiles->push_back(profile.release());
(nit) maybe:

profiles->push_back(GetAutofillProfile(guid).release());
if (!profiles->back())
  return false;

(also this fn is kinda funky in that it can leave data in |profiles| but still
return false)

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:1040:
scoped_ptr<AutofillProfile> old_profile = tmp_profile.Pass();
not sure what the point of this var is any more? can just use tmp_profile or
rename tmp_profile to old_profile?

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:1140:
scoped_ptr<CreditCard> no_card;
you can inline this

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:1162:
credit_cards->push_back(credit_card.release());
ditto with GetAutofillProfiles

[Lei Zhang, 2015-08-20 20:53:54]

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
File components/autofill/core/browser/webdata/autofill_table.cc (right):

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:701: for (const
FormFieldData& data : elements) {
On 2015/08/20 17:20:41, Evan Stade wrote:
> nit: s/data/element

Done.

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:891:
std::vector<AutofillProfile*>* profiles) {
On 2015/08/20 17:20:41, Evan Stade wrote:
> how much work would it be to update this to ScopedVector?

I tried doing this but had trouble with the mock in
profile_sync_service_autofill_unittest.cc. That and this was a good stopping
point last night.

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:905:
profiles->push_back(profile.release());
On 2015/08/20 17:20:41, Evan Stade wrote:
> (nit) maybe:
> 
> profiles->push_back(GetAutofillProfile(guid).release());
> if (!profiles->back())
>   return false;

But that changes the behavior.

> (also this fn is kinda funky in that it can leave data in |profiles| but still
> return false)

Yes it is, but I'm not going to change the behavior here.

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:1040:
scoped_ptr<AutofillProfile> old_profile = tmp_profile.Pass();
On 2015/08/20 17:20:41, Evan Stade wrote:
> not sure what the point of this var is any more? can just use tmp_profile or
> rename tmp_profile to old_profile?

Ya, forgot to take care of this one.

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:1140:
scoped_ptr<CreditCard> no_card;
On 2015/08/20 17:20:41, Evan Stade wrote:
> you can inline this

Done.

https://codereview.chromium.org/1295963005/diff/1/components/autofill/core/br...
components/autofill/core/browser/webdata/autofill_table.cc:1162:
credit_cards->push_back(credit_card.release());
On 2015/08/20 17:20:41, Evan Stade wrote:
> ditto with GetAutofillProfiles

Don't want to change the behavior here either.

[Evan Stade, 2015-08-20 20:56:02]

lgtm

[Lei Zhang, 2015-08-20 21:20:15]

The CQ bit was checked by [email protected]

[commit-bot: I haz the power, 2015-08-20 21:22:16]

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

[commit-bot: I haz the power, 2015-08-20 21:37:28]

The CQ bit was unchecked by [email protected]

[commit-bot: I haz the power, 2015-08-20 21:37:29]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Lei Zhang, 2015-08-20 21:40:03]

[email protected] changed reviewers:
+ [email protected], [email protected]

[Lei Zhang, 2015-08-20 21:40:03]

Oh, right, OWNERS...

+sgurun for android_webview/
+pkasting for components/webdata/

[Peter Kasting, 2015-08-20 21:42:08]

LGTM

[sgurun-gerrit only, 2015-08-20 22:02:48]

On 2015/08/20 21:42:08, Peter Kasting wrote:
> LGTM

lgtm

[Lei Zhang, 2015-08-20 22:13:56]

The CQ bit was checked by [email protected]

[commit-bot: I haz the power, 2015-08-20 22:14:28]

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

[commit-bot: I haz the power, 2015-08-21 01:23:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-08-21 01:24:28]

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