Create an autofill Suggestion class

Create an autofill Suggestion class.

There should be no functionality change.

Previously autofill results were passed through as a set of three or four vectors:
  std::vector<base::string16>* popup_values,
  std::vector<base::string16>* popup_labels,
  std::vector<base::string16>* popup_icons
  std::vector<int>* identifiers
all of which have to stay in sync. This makes it impossible to add metadata, function calls are ugly, and the code is error prone. As part of wallet credit card autofill, we'll need to add more metadata here.

This patch replaces this with a new autofill::Suggestion struct containing all of the information, and pipes this around to all the affected places. It also adds a new SuggestionBackendID struct to replace the old GUIDPair typedef which makes the code clearer.

There are three types of IDs for autofill suggestions which is kind of confusing: a GUID created by the database which is actually a GUID + an integer (GUIDPair typedef in the old code), an integer ID used by the frontend and IPC, and an intermediate integer used by the autofill manager to create the frontend int ID.

This patch adds the GUID and frontend integer ID to the new Suggestion class as the backend_id and frontend_id. It is kind of confusing to have two different types of IDs in the same class, and previously most code dealt with only one or the other. But I think the code is much cleaner not having to keep separate parallel lists of IDs all over the place.

Unified the naming in the autofill popup controller. It previously used name/subtext and now that it uses the standard Suggestion struct, I changed them to value/label to match everything else.

Added constants for the card types used by RequestAutocomplete

Added some additional stuff to the CreditCard class for wallet sync. Add additional members to the autofill table for wallet card manipulation. These are currently unused.

Committed: https://crrev.com/5f88815a2b0e55bca6c6ee72afa051e65855906d
Cr-Commit-Position: refs/heads/master@{#308457}

[brettw, 2014-12-10 22:28:02]

brettw@chromium.org changed reviewers:
+ estade@chromium.org

[brettw, 2014-12-10 22:28:03]

I haven't done the Mac-specific stuff yet, doing a compile now, so a few more
files will appear tomorrow.

I recommend starting looking at suggestion.h/cc, then doing the rest of
components/*, then doing chrome/*.

[Evan Stade, 2014-12-10 23:11:14]

went through all non-test files.

https://codereview.chromium.org/772253003/diff/210001/chrome/browser/ui/autof...
File chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc (right):

https://codereview.chromium.org/772253003/diff/210001/chrome/browser/ui/autof...
chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc:2136: for (size_t
i = 0; i < popup_suggestions.size(); i++)
range based loop?

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
File components/autofill/core/browser/autocomplete_history_manager.cc (right):

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
components/autofill/core/browser/autocomplete_history_manager.cc:164: const
std::vector<base::string16>* new_results) {
s/new_results/autocomplete_results imo

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
File components/autofill/core/browser/autofill_manager.cc (right):

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
components/autofill/core/browser/autofill_manager.cc:71:
std::set<std::pair<base::string16, base::string16>> seen_suggestions;
I think you need > >

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
File components/autofill/core/browser/personal_data_manager.h (right):

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
components/autofill/core/browser/personal_data_manager.h:146:
std::vector<Suggestion>* suggestions);
can be a return value now instead of a series of outparams

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
File components/autofill/core/browser/suggestion.h (right):

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
components/autofill/core/browser/suggestion.h:51: base::string16 value;
can we take this opportunity to call this something better than "value"? Like
"label", "sublabel", "icon"

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
File components/autofill/core/browser/webdata/autofill_table.cc (right):

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
components/autofill/core/browser/webdata/autofill_table.cc:1119: card_number =
s.ColumnString16(index++);
I think this would be slightly easier to follow if you pulled the last four
extraction outside the if/else, i.e.:

base::string16 full_card_number = UnencryptedCardFromColumn(s, index++);
base::string16 last_four = s.ColumnString16(index++);
CreditCard::RecordType type = full_card_number.empty() ? MASKED : FULL;
std::string wallet_id = s.ColumnString(index++);

CreditCard* card = new CreditCard(wallet_id, type);
card->SetRawInfo(CREDIT_CARD_NUMBER, type == FULL ? card_number : last_four);

[brettw, 2014-12-12 00:44:14]

I did everything else except the value/label rename which I'm not very excited
about doing. How important do you think this is? It will take some time.

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
File components/autofill/core/browser/autofill_manager.cc (right):

https://codereview.chromium.org/772253003/diff/210001/components/autofill/cor...
components/autofill/core/browser/autofill_manager.cc:71:
std::set<std::pair<base::string16, base::string16>> seen_suggestions;
On 2014/12/10 23:11:14, Evan Stade wrote:
> I think you need > >

This is legal now with C++11.

[Evan Stade, 2014-12-12 00:46:19]

On 2014/12/12 00:44:14, brettw wrote:
> I did everything else except the value/label rename which I'm not very excited
> about doing. How important do you think this is? It will take some time.

That's fine. This is still a huge improvement either way. lgtm

[brettw, 2014-12-12 03:45:31]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-12-12 03:45:43]

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

[commit-bot: I haz the power, 2014-12-12 04:02:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-12 04:02:27]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[brettw, 2014-12-12 05:31:59]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-12-12 05:32:32]

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

[commit-bot: I haz the power, 2014-12-12 05:37:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-12 05:37:07]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[brettw, 2014-12-12 19:35:27]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-12-12 19:37:02]

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

[commit-bot: I haz the power, 2014-12-12 20:02:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-12 20:02:28]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[brettw, 2014-12-12 21:49:40]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-12-12 21:51:07]

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

[commit-bot: I haz the power, 2014-12-12 22:11:04]

The CQ bit was unchecked by commit-bot@chromium.org

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

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[brettw, 2014-12-12 22:23:17]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-12-12 22:24:24]

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

[brettw, 2014-12-12 22:52:32]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-12-12 22:52:35]

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

[commit-bot: I haz the power, 2014-12-12 23:20:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-12 23:20:02]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[brettw, 2014-12-13 00:47:02]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-12-13 00:48:48]

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

[brettw, 2014-12-13 01:04:35]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-12-13 01:06:03]

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

[commit-bot: I haz the power, 2014-12-13 02:50:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-13 02:50:16]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[brettw, 2014-12-15 21:15:07]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-12-15 21:16:08]

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

[brettw, 2014-12-15 21:57:00]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2014-12-15 21:58:00]

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

[commit-bot: I haz the power, 2014-12-15 23:32:07]

Committed patchset #28 (id:510001)

[commit-bot: I haz the power, 2014-12-15 23:32:48]

Patchset 28 (id:??) landed as
https://crrev.com/5f88815a2b0e55bca6c6ee72afa051e65855906d
Cr-Commit-Position: refs/heads/master@{#308457}