Clear password values before sending them to the renderer process.

Clear password values before sending them to the renderer process. 

All passwords for additional logins are cleared (that are passwords for non-preferred accounts, that possible to be filled on account select). In case of account select filling the password to the preferred account is also cleared.

BUG=700960
Review-Url: https://codereview.chromium.org/2736393003
Cr-Commit-Position: refs/heads/master@{#456428}
Committed: https://chromium.googlesource.com/chromium/src/+/bb38ea3085965823c24ac636a2d560631448c18d

[dvadym, 2017-03-10 15:07:01]

Description was changed from

==========
Clear not needed passwords

BUG=None
==========

to

==========
Clear not needed in renderer passwords.

BUG=None
==========

[dvadym, 2017-03-10 15:07:02]

dvadym@chromium.org changed reviewers:
+ vasilii@chromium.org

[dvadym, 2017-03-10 15:07:25]

Hi Vasilii,

could you please review this CL?

Regards,
Vadym

[vasilii, 2017-03-10 17:24:51]

https://codereview.chromium.org/2736393003/diff/60001/components/autofill/cor...
File components/autofill/core/common/password_form_fill_data.h (right):

https://codereview.chromium.org/2736393003/diff/60001/components/autofill/cor...
components/autofill/core/common/password_form_fill_data.h:101:
PasswordFormFillData ClearNotNeededInRendererPasswords(
ClearPasswordValues is a better name.

https://codereview.chromium.org/2736393003/diff/60001/components/password_man...
File
components/password_manager/content/browser/content_password_manager_driver_unittest.cc
(right):

https://codereview.chromium.org/2736393003/diff/60001/components/password_man...
components/password_manager/content/browser/content_password_manager_driver_unittest.cc:99:
MATCHER(WerePasswordsCleared, "Passwords not cleared") {
Please make |wait_for_username| a parameter of the matcher. Thus, the test
determines if the password should be cleared.

https://codereview.chromium.org/2736393003/diff/60001/components/password_man...
components/password_manager/content/browser/content_password_manager_driver_unittest.cc:384:
PasswordFormFillData GetTestPasswordFormFillData() {
Please move up to the anonymous namespace.

[dvadym, 2017-03-13 15:49:45]

Thanks for comments Vasilii! I've adressed them. PTAL

https://codereview.chromium.org/2736393003/diff/60001/components/autofill/cor...
File components/autofill/core/common/password_form_fill_data.h (right):

https://codereview.chromium.org/2736393003/diff/60001/components/autofill/cor...
components/autofill/core/common/password_form_fill_data.h:101:
PasswordFormFillData ClearNotNeededInRendererPasswords(
On 2017/03/10 17:24:51, vasilii wrote:
> ClearPasswordValues is a better name.

Done.

https://codereview.chromium.org/2736393003/diff/60001/components/password_man...
File
components/password_manager/content/browser/content_password_manager_driver_unittest.cc
(right):

https://codereview.chromium.org/2736393003/diff/60001/components/password_man...
components/password_manager/content/browser/content_password_manager_driver_unittest.cc:99:
MATCHER(WerePasswordsCleared, "Passwords not cleared") {
On 2017/03/10 17:24:51, vasilii wrote:
> Please make |wait_for_username| a parameter of the matcher. Thus, the test
> determines if the password should be cleared.

Done.

https://codereview.chromium.org/2736393003/diff/60001/components/password_man...
components/password_manager/content/browser/content_password_manager_driver_unittest.cc:384:
PasswordFormFillData GetTestPasswordFormFillData() {
On 2017/03/10 17:24:51, vasilii wrote:
> Please move up to the anonymous namespace.

Done.

[vasilii, 2017-03-13 16:26:05]

Can you create a bug and write a more verbose description something like "Clear
password values before sending them to the renderer process. On autofill [this]
happens, on fill on account select [that] happens".

https://codereview.chromium.org/2736393003/diff/80001/components/password_man...
File
components/password_manager/content/browser/content_password_manager_driver_unittest.cc
(right):

https://codereview.chromium.org/2736393003/diff/80001/components/password_man...
components/password_manager/content/browser/content_password_manager_driver_unittest.cc:116:
static PasswordForm non_preferred_match = preferred_match;
let's drop static. I don't think PasswordFormFillData stores pointers.

https://codereview.chromium.org/2736393003/diff/80001/components/password_man...
components/password_manager/content/browser/content_password_manager_driver_unittest.cc:418:
PasswordFormFillData fill_data = GetTestPasswordFormFillData();
I think you need to move this into the loop. Otherwise, in theory you may clear
the password once and the second iteration of the test simply tests nothing.

[dvadym, 2017-03-13 16:54:43]

Description was changed from

==========
Clear not needed in renderer passwords.

BUG=None
==========

to

==========
Clear password values before sending them to the renderer process. 

All passwords from |PasswordFormFillData.additional_logins| are cleared (that
are passwords for non-preferred accounts, that possible to be filled on account
select). In case of account select filling the password to the prefferred
account is also cleared.


BUG=700960
==========

[dvadym, 2017-03-13 16:57:43]

Description was changed from

==========
Clear password values before sending them to the renderer process. 

All passwords from |PasswordFormFillData.additional_logins| are cleared (that
are passwords for non-preferred accounts, that possible to be filled on account
select). In case of account select filling the password to the prefferred
account is also cleared.


BUG=700960
==========

to

==========
Clear password values before sending them to the renderer process. 

All passwords for additional logins are cleared (that are passwords for
non-preferred accounts, that possible to be filled on account select). In case
of account select filling the password to the preferred account is also cleared.

BUG=700960
==========

[dvadym, 2017-03-13 16:58:10]

PTAL

https://codereview.chromium.org/2736393003/diff/80001/components/password_man...
File
components/password_manager/content/browser/content_password_manager_driver_unittest.cc
(right):

https://codereview.chromium.org/2736393003/diff/80001/components/password_man...
components/password_manager/content/browser/content_password_manager_driver_unittest.cc:116:
static PasswordForm non_preferred_match = preferred_match;
On 2017/03/13 16:26:04, vasilii wrote:
> let's drop static. I don't think PasswordFormFillData stores pointers.

Done.

https://codereview.chromium.org/2736393003/diff/80001/components/password_man...
components/password_manager/content/browser/content_password_manager_driver_unittest.cc:418:
PasswordFormFillData fill_data = GetTestPasswordFormFillData();
On 2017/03/13 16:26:05, vasilii wrote:
> I think you need to move this into the loop. Otherwise, in theory you may
clear
> the password once and the second iteration of the test simply tests nothing.

Done.

[vasilii, 2017-03-13 17:00:33]

lgtm

[dvadym, 2017-03-13 17:02:31]

The CQ bit was checked by dvadym@chromium.org

[commit-bot: I haz the power, 2017-03-13 17:03:12]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-13 18:12:37]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1489424551596340,
"parent_rev": "19e4c274dd7efd8731cceb5f0c0ac0a2860ea064", "commit_rev":
"bb38ea3085965823c24ac636a2d560631448c18d"}

[commit-bot: I haz the power, 2017-03-13 18:13:22]

Description was changed from

==========
Clear password values before sending them to the renderer process. 

All passwords for additional logins are cleared (that are passwords for
non-preferred accounts, that possible to be filled on account select). In case
of account select filling the password to the preferred account is also cleared.

BUG=700960
==========

to

==========
Clear password values before sending them to the renderer process. 

All passwords for additional logins are cleared (that are passwords for
non-preferred accounts, that possible to be filled on account select). In case
of account select filling the password to the preferred account is also cleared.

BUG=700960

Review-Url: https://codereview.chromium.org/2736393003
Cr-Commit-Position: refs/heads/master@{#456428}
Committed:
https://chromium.googlesource.com/chromium/src/+/bb38ea3085965823c24ac636a2d5...
==========

[commit-bot: I haz the power, 2017-03-13 18:13:23]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/bb38ea3085965823c24ac636a2d5...