Revive PasswordManager.OsPasswordStatus histogram.

Revive PasswordManager.OsPasswordStatus histogram.

It was disabled in https://codereview.chromium.org/184843004. It's safe to collect on Windows starting from Windows 7 only.

BUG=345916
Committed: https://crrev.com/e4b02901ceffb3241c41eb8a14e30b3a92311494
Cr-Commit-Position: refs/heads/master@{#307452}

[vasilii, 2014-12-08 17:13:45]

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

[vasilii, 2014-12-08 17:14:49]

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

[vasilii, 2014-12-08 17:14:49]

Hi guys,

please review.

[Will Harris, 2014-12-08 17:23:39]

On 2014/12/08 17:14:49, vasilii wrote:
> Hi guys,
> 
> please review.

it's actually CheckBlankPassword() that caused the issues with Vista/XP - the
specific call that broke things was the LogonUser().

So, it might be possible to change just that function to return an UNKNOWN
status and still get all the other UMA metrics e.g. whether the machine is
domain connected or not.  Depending on how useful that data would be.

Otherwise, lgtm

[vasilii, 2014-12-09 09:43:23]

On 2014/12/08 17:23:39, Will Harris wrote:
> On 2014/12/08 17:14:49, vasilii wrote:
> > Hi guys,
> > 
> > please review.
> 
> it's actually CheckBlankPassword() that caused the issues with Vista/XP - the
> specific call that broke things was the LogonUser().
> 
> So, it might be possible to change just that function to return an UNKNOWN
> status and still get all the other UMA metrics e.g. whether the machine is
> domain connected or not.  Depending on how useful that data would be.
> 
> Otherwise, lgtm

More data is better. I fixed GetOsPasswordStatus().

[vabr (Chromium), 2014-12-09 10:08:16]

LGTM with a nit.

https://codereview.chromium.org/772413004/diff/20001/chrome/browser/password_...
File chrome/browser/password_manager/chrome_password_manager_client.cc (right):

https://codereview.chromium.org/772413004/diff/20001/chrome/browser/password_...
chrome/browser/password_manager/chrome_password_manager_client.cc:54: void
ReportOsPassword() {
nit: The example snippet for unnamed namespaces in
http://dev.chromium.org/developers/coding-style#TOC-Unnamed-Namespaces suggests
there should be separating blank lines at both ends of the namespace.

[vasilii, 2014-12-09 11:02:53]

https://codereview.chromium.org/772413004/diff/20001/chrome/browser/password_...
File chrome/browser/password_manager/chrome_password_manager_client.cc (right):

https://codereview.chromium.org/772413004/diff/20001/chrome/browser/password_...
chrome/browser/password_manager/chrome_password_manager_client.cc:54: void
ReportOsPassword() {
On 2014/12/09 10:08:16, vabr (Chromium) wrote:
> nit: The example snippet for unnamed namespaces in
> http://dev.chromium.org/developers/coding-style#TOC-Unnamed-Namespaces
suggests
> there should be separating blank lines at both ends of the namespace.

Done. The same snippet suggests wrong indent for '// namespace'.

[vasilii, 2014-12-09 11:03:01]

The CQ bit was checked by vasilii@chromium.org

[commit-bot: I haz the power, 2014-12-09 11:03:37]

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

[vabr (Chromium), 2014-12-09 11:29:16]

https://codereview.chromium.org/772413004/diff/20001/chrome/browser/password_...
File chrome/browser/password_manager/chrome_password_manager_client.cc (right):

https://codereview.chromium.org/772413004/diff/20001/chrome/browser/password_...
chrome/browser/password_manager/chrome_password_manager_client.cc:54: void
ReportOsPassword() {
On 2014/12/09 11:02:53, vasilii wrote:
> On 2014/12/09 10:08:16, vabr (Chromium) wrote:
> > nit: The example snippet for unnamed namespaces in
> > http://dev.chromium.org/developers/coding-style#TOC-Unnamed-Namespaces
> suggests
> > there should be separating blank lines at both ends of the namespace.
> 
> Done. The same snippet suggests wrong indent for '// namespace'.

Good point! :) Time to leave your trace in the Chromium style guide, then! ;-)

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

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2014-12-09 11:51:10]

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