Improvement of password manager logging.

Improvement of password manager logging.

It's nice to have more information in password manager logging in order to debug password generation in Chrome. This CL updates FormStructure presentation.

For example Linkedin SignUp form has the following presentation in logs:

Adding manager for form: {
Signature of form: 5301209056338178556
Signon realm: https://www.linkedin.com/
Origin: https://www.linkedin.com/
Action: https://www.linkedin.com/
Form name:
Form fields:
firstname: 1855613035, text
lastname: 4163345999, text
emailaddress: 4103469401, email
password: 2051817934, password, off
}

BUG=None

Committed: https://crrev.com/528c53ea0acc8feb502b4abeef064299d5311455
Cr-Commit-Position: refs/heads/master@{#414387}

[dvadym, 2016-08-24 10:11:40]

Description was changed from

==========
Improvement of password manager logging.

It's nice to have more information in password manager logging in order to debug
password generation in Chrome.

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

to

==========
Improvement of password manager logging.

It's nice to have more information in password manager logging in order to debug
password generation in Chrome. This CL updates FormStructure presentation.

For example Linkedin SignUp form has the following presentation in logs:
Adding manager for form: {
Signature of form: 5301209056338178556
Signon realm: https://www.linkedin.com/
Origin: https://www.linkedin.com/
Action: https://www.linkedin.com/
Form name: 
Form fields: 
firstname: 1855613035, text
lastname: 4163345999, text
emailaddress: 4103469401, email
password: 2051817934, password, off
}

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

[dvadym, 2016-08-24 10:12:00]

Description was changed from

==========
Improvement of password manager logging.

It's nice to have more information in password manager logging in order to debug
password generation in Chrome. This CL updates FormStructure presentation.

For example Linkedin SignUp form has the following presentation in logs:
Adding manager for form: {
Signature of form: 5301209056338178556
Signon realm: https://www.linkedin.com/
Origin: https://www.linkedin.com/
Action: https://www.linkedin.com/
Form name: 
Form fields: 
firstname: 1855613035, text
lastname: 4163345999, text
emailaddress: 4103469401, email
password: 2051817934, password, off
}

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

to

==========
Improvement of password manager logging.

It's nice to have more information in password manager logging in order to debug
password generation in Chrome. This CL updates FormStructure presentation.

For example Linkedin SignUp form has the following presentation in logs:

Adding manager for form: {
Signature of form: 5301209056338178556
Signon realm: https://www.linkedin.com/
Origin: https://www.linkedin.com/
Action: https://www.linkedin.com/
Form name: 
Form fields: 
firstname: 1855613035, text
lastname: 4163345999, text
emailaddress: 4103469401, email
password: 2051817934, password, off
}

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

[dvadym, 2016-08-24 10:15:44]

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

[dvadym, 2016-08-24 10:15:45]

Hi Vaclav,

 Could you please review this CL?

Regards,
Vadym

[vabr (Chromium), 2016-08-24 10:25:44]

LGTM with comments.
Thanks!
Vaclav

https://codereview.chromium.org/2268313003/diff/20001/components/autofill/cor...
File components/autofill/core/common/save_password_progress_logger.h (right):

https://codereview.chromium.org/2268313003/diff/20001/components/autofill/cor...
components/autofill/core/common/save_password_progress_logger.h:182: // The
UTF-8 version of SavePasswordProgressLogger::ScrubElementID.
optional nit: You could have named this just ScrubElementID; overloading here is
OK, because both functions do exactly the same.
Also, feel free to drop SavePasswordProgressLogger:: in the comment. It is not
necessary and hints at a different class until the user realises it's actually
the same. Perhaps saying "above" at the end of the sentence will make it
clearer?

https://codereview.chromium.org/2268313003/diff/20001/components/password_man...
File
components/password_manager/core/browser/browser_save_password_progress_logger.cc
(right):

https://codereview.chromium.org/2268313003/diff/20001/components/password_man...
components/password_manager/core/browser/browser_save_password_progress_logger.cc:22:
const char kLineEnding[] = "\n";
Would Chrome actually display this incorrectly if you used just "\n" everywhere?

If not, do you need to use just "\r" on Mac?
(Not sure.)

[dvadym, 2016-08-25 08:41:25]

Thanks Vaclav for review! I've fixed all that we talked. PTAL

https://codereview.chromium.org/2268313003/diff/20001/components/autofill/cor...
File components/autofill/core/common/save_password_progress_logger.h (right):

https://codereview.chromium.org/2268313003/diff/20001/components/autofill/cor...
components/autofill/core/common/save_password_progress_logger.h:182: // The
UTF-8 version of SavePasswordProgressLogger::ScrubElementID.
On 2016/08/24 10:25:44, vabr (Chromium) wrote:
> optional nit: You could have named this just ScrubElementID; overloading here
is
> OK, because both functions do exactly the same.
> Also, feel free to drop SavePasswordProgressLogger:: in the comment. It is not
> necessary and hints at a different class until the user realises it's actually
> the same. Perhaps saying "above" at the end of the sentence will make it
> clearer?

Done.

https://codereview.chromium.org/2268313003/diff/20001/components/password_man...
File
components/password_manager/core/browser/browser_save_password_progress_logger.cc
(right):

https://codereview.chromium.org/2268313003/diff/20001/components/password_man...
components/password_manager/core/browser/browser_save_password_progress_logger.cc:22:
const char kLineEnding[] = "\n";
On 2016/08/24 10:25:44, vabr (Chromium) wrote:
> Would Chrome actually display this incorrectly if you used just "\n"
everywhere?
> 
> If not, do you need to use just "\r" on Mac?
> (Not sure.)

I've checked, "\n" is enough on Windows, so I've removed a const and put "\n" in
code

[vabr (Chromium), 2016-08-25 08:44:54]

Thanks for checking the linebreaks and also for spotting the lowercasing
strangeness.

LGTM!
Vaclav

https://codereview.chromium.org/2268313003/diff/40001/components/autofill/cor...
File components/autofill/core/common/save_password_progress_logger.h (right):

https://codereview.chromium.org/2268313003/diff/40001/components/autofill/cor...
components/autofill/core/common/save_password_progress_logger.h:182: // The
UTF-8 version of a function above.
nit: "a function" -> "the function"

[dvadym, 2016-08-25 08:49:41]

https://codereview.chromium.org/2268313003/diff/40001/components/autofill/cor...
File components/autofill/core/common/save_password_progress_logger.h (right):

https://codereview.chromium.org/2268313003/diff/40001/components/autofill/cor...
components/autofill/core/common/save_password_progress_logger.h:182: // The
UTF-8 version of a function above.
On 2016/08/25 08:44:54, vabr (Chromium) wrote:
> nit: "a function" -> "the function"

Done.

[dvadym, 2016-08-25 08:49:46]

The CQ bit was checked by dvadym@chromium.org

[dvadym, 2016-08-25 08:49:46]

The patchset sent to the CQ was uploaded after l-g-t-m from vabr@chromium.org
Link to the patchset: https://codereview.chromium.org/2268313003/#ps60001
(title: "Comment fix")

[commit-bot: I haz the power, 2016-08-25 08:50:02]

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, 2016-08-25 09:02:41]

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

[commit-bot: I haz the power, 2016-08-25 09:02:42]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[dvadym, 2016-08-25 10:06:33]

The CQ bit was checked by dvadym@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-25 10:06:59]

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

[dvadym, 2016-08-25 10:54:33]

The CQ bit was unchecked by dvadym@chromium.org

[dvadym, 2016-08-25 10:54:38]

The CQ bit was checked by dvadym@chromium.org

[dvadym, 2016-08-25 10:54:38]

The patchset sent to the CQ was uploaded after l-g-t-m from vabr@chromium.org
Link to the patchset: https://codereview.chromium.org/2268313003/#ps80001
(title: "fix test")

[commit-bot: I haz the power, 2016-08-25 10:55:02]

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, 2016-08-25 10:59:35]

Description was changed from

==========
Improvement of password manager logging.

It's nice to have more information in password manager logging in order to debug
password generation in Chrome. This CL updates FormStructure presentation.

For example Linkedin SignUp form has the following presentation in logs:

Adding manager for form: {
Signature of form: 5301209056338178556
Signon realm: https://www.linkedin.com/
Origin: https://www.linkedin.com/
Action: https://www.linkedin.com/
Form name: 
Form fields: 
firstname: 1855613035, text
lastname: 4163345999, text
emailaddress: 4103469401, email
password: 2051817934, password, off
}

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

to

==========
Improvement of password manager logging.

It's nice to have more information in password manager logging in order to debug
password generation in Chrome. This CL updates FormStructure presentation.

For example Linkedin SignUp form has the following presentation in logs:

Adding manager for form: {
Signature of form: 5301209056338178556
Signon realm: https://www.linkedin.com/
Origin: https://www.linkedin.com/
Action: https://www.linkedin.com/
Form name: 
Form fields: 
firstname: 1855613035, text
lastname: 4163345999, text
emailaddress: 4103469401, email
password: 2051817934, password, off
}

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

[commit-bot: I haz the power, 2016-08-25 10:59:36]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-25 11:01:18]

Description was changed from

==========
Improvement of password manager logging.

It's nice to have more information in password manager logging in order to debug
password generation in Chrome. This CL updates FormStructure presentation.

For example Linkedin SignUp form has the following presentation in logs:

Adding manager for form: {
Signature of form: 5301209056338178556
Signon realm: https://www.linkedin.com/
Origin: https://www.linkedin.com/
Action: https://www.linkedin.com/
Form name: 
Form fields: 
firstname: 1855613035, text
lastname: 4163345999, text
emailaddress: 4103469401, email
password: 2051817934, password, off
}

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

to

==========
Improvement of password manager logging.

It's nice to have more information in password manager logging in order to debug
password generation in Chrome. This CL updates FormStructure presentation.

For example Linkedin SignUp form has the following presentation in logs:

Adding manager for form: {
Signature of form: 5301209056338178556
Signon realm: https://www.linkedin.com/
Origin: https://www.linkedin.com/
Action: https://www.linkedin.com/
Form name:
Form fields:
firstname: 1855613035, text
lastname: 4163345999, text
emailaddress: 4103469401, email
password: 2051817934, password, off
}

BUG=None

Committed: https://crrev.com/528c53ea0acc8feb502b4abeef064299d5311455
Cr-Commit-Position: refs/heads/master@{#414387}
==========

[commit-bot: I haz the power, 2016-08-25 11:01:20]

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