Reland "Do not haul suggestions back to browser in AutofillHostMsg_ShowPasswordSuggestions"

Reland "Do not haul suggestions back to browser in AutofillHostMsg_ShowPasswordSuggestions"

This is a reland of https://codereview.chromium.org/688633004.
Patch set 1 here = the landed patch set (2) of the relanded CL, just automatically rebased on ToT
Patch set 2 here = fix of the crash, as outlined in http://crbug.com/429607#c7
Patch set 3 here = Comments from jww@ (https://codereview.chromium.org/688633004#msg12) addressed

Original CL description:
****************************************************************
Currently, PasswordAutofillAgent (in renderer) prepares password autofill suggestions for PasswordAutofillManager (in browser) and sends them via IPC.

But the manager has the data to generate the suggestions itself, so this CL makes it derive the suggestions from that data instead of hauling it through IPC.

This CL also adds one more test case to the PasswordAutofillManager unittest, to compensate for the coverage lost in the PasswordAutofillAgent browsertest by moving some logic out of the agent to the manager.
****************************************************************

BUG=429607, 400186, 377422, 118601
TBR=gcasto@chromium.org,jww@chromium.org

Committed: https://crrev.com/2f5c6cafb187ea63f5edd2314280dd66bc5d7855
Cr-Commit-Position: refs/heads/master@{#302781}

[vabr (Chromium), 2014-11-05 09:31:12]

Patchset #2 (id:20001) has been deleted

[vabr (Chromium), 2014-11-05 09:41:05]

vabr@chromium.org changed reviewers:
+ gcasto@chromium.org, jww@chromium.org, mkwst@chromium.org

[vabr (Chromium), 2014-11-05 09:41:06]

Mike, 

Please have a look at the diff of patch set 3 against 1.
Patch set 1 has already been approved as patch set 2 in
https://codereview.chromium.org/688633004/.

Joel,

I'll TBR you for how I addressed you drive-by.

Garrett,

I'll TBR you for chrome/renderer/autofill/password_autofill_agent_browsertest.cc
-- note that I did not change the browsertest in this CL, so this is landing
exactly as you approved it already in
https://codereview.chromium.org/688633004/.

Cheers,
Vaclav

https://codereview.chromium.org/701073002/diff/1/components/password_manager/...
File components/password_manager/core/browser/password_autofill_manager.cc
(right):

https://codereview.chromium.org/701073002/diff/1/components/password_manager/...
components/password_manager/core/browser/password_autofill_manager.cc:124: if
(!autofill::IsValidString16Vector(suggestions) ||
Repeating the comment from jww:
> Drive by comment: Seems like this validation isn't really doing much good
here;
> shouldn't the validation be on |typed_username|, since that's the untrusted
> content? By the same token, GetSuggestions() returns trusted data, right, so
why
> validate it at this point?

And my answer:
Agreed that validating the browser-computed data makes no longer sense.
There is also not much to validate on |typed_username|: we have to take the
renderer's word for that it is what the user typed in the username field.
Sending invalid strings may result in offering non-matching suggestions (but
still stored for that form) or not not offering any suggestions at all.
Similarly, validating |show_all| is not really possible, and sending an
incorrect value has a minimal impact (all the time, at most all the suggestions
for the current form will be shown to the user, nothing more).

[Mike West, 2014-11-05 09:43:29]

LGTM, based on our offline discussion.

[vabr (Chromium), 2014-11-05 09:46:28]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2014-11-05 09:47:13]

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

[commit-bot: I haz the power, 2014-11-05 10:23:54]

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

[commit-bot: I haz the power, 2014-11-05 10:23:56]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[vabr (Chromium), 2014-11-05 10:50:24]

Mike,

I forgot to negate the DCHECK condition when turning it into an if-statement.
Fixed in patch set 4, I'm sending it back to the CQ.

Cheers,
Vaclav

[vabr (Chromium), 2014-11-05 10:50:39]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2014-11-05 10:52:05]

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

[Mike West, 2014-11-05 11:12:01]

On 2014/11/05 10:50:24, vabr (Chromium) wrote:
> Mike,
> 
> I forgot to negate the DCHECK condition when turning it into an if-statement.
> Fixed in patch set 4, I'm sending it back to the CQ.
> 
> Cheers,
> Vaclav

I should have looked more carefully, sorry. Still LGTM. :)

[commit-bot: I haz the power, 2014-11-05 11:53:38]

Committed patchset #4 (id:80001)

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

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