[AiS] Remove answer contents from all matches other than the first.

[AiS] Remove answer contents from all matches other than the first.

It's possible to get a copy of an answer from previous matches and get the
same answer attached to a similar server-provided answer.  In the future,
we may decide that we want to have different answers attached to multiple
suggestions, but the current assumption is that there should only ever be
one suggestion with an answer.

BUG=

Committed: https://crrev.com/55f6614194514aad00a134312b3595a57b85f7ee
Cr-Commit-Position: refs/heads/master@{#296818}

[Justin Donnelly, 2014-09-24 22:01:15]

Patchset #1 (id:1) has been deleted

[Justin Donnelly, 2014-09-24 22:03:34]

Patchset #1 (id:20001) has been deleted

[Justin Donnelly, 2014-09-24 22:04:09]

jdonnelly@chromium.org changed reviewers:
+ mpearson@chromium.org

[Justin Donnelly, 2014-09-24 22:09:50]

Hi Mark - here's the change I mentioned in the meeting this afternoon.

[groby-ooo-7-16, 2014-09-24 22:26:38]

Drive-by comments.

https://codereview.chromium.org/603683004/diff/40001/components/omnibox/searc...
File components/omnibox/search_provider.cc (right):

https://codereview.chromium.org/603683004/diff/40001/components/omnibox/searc...
components/omnibox/search_provider.cc:927: it->answer_contents =
base::string16();
nit: answer_contents.clear()/answer_type.clear() might be cleaner

https://codereview.chromium.org/603683004/diff/40001/components/omnibox/searc...
File components/omnibox/search_provider.h (right):

https://codereview.chromium.org/603683004/diff/40001/components/omnibox/searc...
components/omnibox/search_provider.h:15: #include <vector>
Your change doesn't bring in any of these?

https://codereview.chromium.org/603683004/diff/40001/components/omnibox/searc...
components/omnibox/search_provider.h:239: void RemoveExtraAnswers(ACMatches*
matches);
Personally, I'd make it static, since it doesn't depend on any state in
SearchProvider. 

If you'd rather not, please make it const.

[Justin Donnelly, 2014-09-25 01:05:02]

Patchset #2 (id:60001) has been deleted

[Justin Donnelly, 2014-09-25 01:05:13]

https://codereview.chromium.org/603683004/diff/40001/components/omnibox/searc...
File components/omnibox/search_provider.cc (right):

https://codereview.chromium.org/603683004/diff/40001/components/omnibox/searc...
components/omnibox/search_provider.cc:927: it->answer_contents =
base::string16();
On 2014/09/24 22:26:38, groby wrote:
> nit: answer_contents.clear()/answer_type.clear() might be cleaner

Done.

https://codereview.chromium.org/603683004/diff/40001/components/omnibox/searc...
File components/omnibox/search_provider.h (right):

https://codereview.chromium.org/603683004/diff/40001/components/omnibox/searc...
components/omnibox/search_provider.h:15: #include <vector>
On 2014/09/24 22:26:38, groby wrote:
> Your change doesn't bring in any of these?

No, I'm just adding them at the urging of the ICWYU lint.

https://codereview.chromium.org/603683004/diff/40001/components/omnibox/searc...
components/omnibox/search_provider.h:239: void RemoveExtraAnswers(ACMatches*
matches);
On 2014/09/24 22:26:38, groby wrote:
> Personally, I'd make it static, since it doesn't depend on any state in
> SearchProvider. 
> 
> If you'd rather not, please make it const.

Done.

[Justin Donnelly, 2014-09-25 01:06:56]

Patchset #2 (id:80001) has been deleted

[Mark P, 2014-09-25 16:23:47]

Is it hard to add a unit test?

--mark

https://codereview.chromium.org/603683004/diff/100001/components/omnibox/sear...
File components/omnibox/search_provider.cc (right):

https://codereview.chromium.org/603683004/diff/100001/components/omnibox/sear...
components/omnibox/search_provider.cc:885: // same answer attached to a similar
server-provided answer.  In the future,
consider something like:
get the same answer attached to a similar server-provided answer.
->
get the same or a different answer to another server-provided suggestion.

https://codereview.chromium.org/603683004/diff/100001/components/omnibox/sear...
components/omnibox/search_provider.cc:886: // we may decide that we want to have
different answers attached to multiple
omit "different"

[Justin Donnelly, 2014-09-25 19:52:48]

Patchset #3 (id:120001) has been deleted

[Justin Donnelly, 2014-09-25 19:53:40]

On 2014/09/25 16:23:47, Mark P wrote:
> Is it hard to add a unit test?

No, done.  (Didn't realize before that there was an existing set of tests in
c/b/autocomplete.)

https://codereview.chromium.org/603683004/diff/100001/components/omnibox/sear...
File components/omnibox/search_provider.cc (right):

https://codereview.chromium.org/603683004/diff/100001/components/omnibox/sear...
components/omnibox/search_provider.cc:885: // same answer attached to a similar
server-provided answer.  In the future,
On 2014/09/25 16:23:47, Mark P wrote:
> consider something like:
> get the same answer attached to a similar server-provided answer.
> ->
> get the same or a different answer to another server-provided suggestion.

Done.

https://codereview.chromium.org/603683004/diff/100001/components/omnibox/sear...
components/omnibox/search_provider.cc:886: // we may decide that we want to have
different answers attached to multiple
On 2014/09/25 16:23:47, Mark P wrote:
> omit "different"

Done.

[Mark P, 2014-09-25 19:56:11]

lgtm

[Justin Donnelly, 2014-09-25 22:13:14]

The CQ bit was checked by jdonnelly@chromium.org

[commit-bot: I haz the power, 2014-09-25 22:14:53]

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

[commit-bot: I haz the power, 2014-09-25 23:08:33]

Committed patchset #3 (id:140001) as beb87869886725940d0b379d39c8b9a3b438bce9

[commit-bot: I haz the power, 2014-09-25 23:09:10]

Patchset 3 (id:??) landed as
https://crrev.com/55f6614194514aad00a134312b3595a57b85f7ee
Cr-Commit-Position: refs/heads/master@{#296818}