[Password Generation] Update UMA stats for popups per page not per event.

[Password Generation] Update UMA stats for popups per page not per event.

BUG=421707
Committed: https://crrev.com/cec66775f18b88939595ec51f30d24e8e7996a20
Cr-Commit-Position: refs/heads/master@{#299738}

[Garrett Casto, 2014-10-09 18:37:07]

gcasto@chromium.org changed reviewers:
+ isherman@chromium.org

[Ilya Sherman, 2014-10-09 22:03:58]

https://codereview.chromium.org/643493003/diff/70001/chrome/renderer/autofill...
File chrome/renderer/autofill/password_generation_agent_browsertest.cc (right):

https://codereview.chromium.org/643493003/diff/70001/chrome/renderer/autofill...
chrome/renderer/autofill/password_generation_agent_browsertest.cc:346:
autofill::password_generation::GENERATION_POPUP_SHOWN));
Please use base/test/histogram_tester.h for this.

https://codereview.chromium.org/643493003/diff/70001/components/autofill/cont...
File components/autofill/content/renderer/password_generation_agent.cc (right):

https://codereview.chromium.org/643493003/diff/70001/components/autofill/cont...
components/autofill/content/renderer/password_generation_agent.cc:143: }
Please rename the corresponding histogram, since its semantics are changing.

https://codereview.chromium.org/643493003/diff/70001/components/autofill/cont...
File components/autofill/content/renderer/password_generation_agent.h (right):

https://codereview.chromium.org/643493003/diff/70001/components/autofill/cont...
components/autofill/content/renderer/password_generation_agent.h:111: // track
UMA stats per page visit not per display, since the former is more
nit: "not per display" -> "rather than per display" or "instead of per display"

https://codereview.chromium.org/643493003/diff/70001/components/autofill/cont...
components/autofill/content/renderer/password_generation_agent.h:116: // UMA
stats per page instead of per display since the former is more
nit: "instead of per display since" -> "rather than per display, since" or
"instead of per display, since" (add a comma)

[Garrett Casto, 2014-10-14 18:40:05]

https://codereview.chromium.org/643493003/diff/70001/chrome/renderer/autofill...
File chrome/renderer/autofill/password_generation_agent_browsertest.cc (right):

https://codereview.chromium.org/643493003/diff/70001/chrome/renderer/autofill...
chrome/renderer/autofill/password_generation_agent_browsertest.cc:346:
autofill::password_generation::GENERATION_POPUP_SHOWN));
On 2014/10/09 22:03:58, Ilya Sherman wrote:
> Please use base/test/histogram_tester.h for this.

Neat, didn't know about that. Updated.

https://codereview.chromium.org/643493003/diff/70001/components/autofill/cont...
File components/autofill/content/renderer/password_generation_agent.cc (right):

https://codereview.chromium.org/643493003/diff/70001/components/autofill/cont...
components/autofill/content/renderer/password_generation_agent.cc:143: }
On 2014/10/09 22:03:58, Ilya Sherman wrote:
> Please rename the corresponding histogram, since its semantics are changing.

Done.

https://codereview.chromium.org/643493003/diff/70001/components/autofill/cont...
File components/autofill/content/renderer/password_generation_agent.h (right):

https://codereview.chromium.org/643493003/diff/70001/components/autofill/cont...
components/autofill/content/renderer/password_generation_agent.h:111: // track
UMA stats per page visit not per display, since the former is more
On 2014/10/09 22:03:58, Ilya Sherman wrote:
> nit: "not per display" -> "rather than per display" or "instead of per
display"

Done.

https://codereview.chromium.org/643493003/diff/70001/components/autofill/cont...
components/autofill/content/renderer/password_generation_agent.h:116: // UMA
stats per page instead of per display since the former is more
On 2014/10/09 22:03:58, Ilya Sherman wrote:
> nit: "instead of per display since" -> "rather than per display, since" or
> "instead of per display, since" (add a comma)

Done.

[Ilya Sherman, 2014-10-14 20:17:20]

Thanks.  LGTM % nits:

https://codereview.chromium.org/643493003/diff/170001/chrome/renderer/autofil...
File chrome/renderer/autofill/password_generation_agent_browsertest.cc (right):

https://codereview.chromium.org/643493003/diff/170001/chrome/renderer/autofil...
chrome/renderer/autofill/password_generation_agent_browsertest.cc:9: #include
"base/metrics/statistics_recorder.h"
nit: Are these two still needed?

https://codereview.chromium.org/643493003/diff/170001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/643493003/diff/170001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:48608: +  <int value="5"
label="DEPRECATED:Generation popup shown"/>
nit: Please leave a space after "DEPRECATED:"; ditto below.

[Garrett Casto, 2014-10-15 00:10:01]

https://codereview.chromium.org/643493003/diff/170001/chrome/renderer/autofil...
File chrome/renderer/autofill/password_generation_agent_browsertest.cc (right):

https://codereview.chromium.org/643493003/diff/170001/chrome/renderer/autofil...
chrome/renderer/autofill/password_generation_agent_browsertest.cc:9: #include
"base/metrics/statistics_recorder.h"
On 2014/10/14 20:17:20, Ilya Sherman wrote:
> nit: Are these two still needed?

Done.

https://codereview.chromium.org/643493003/diff/170001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/643493003/diff/170001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:48608: +  <int value="5"
label="DEPRECATED:Generation popup shown"/>
On 2014/10/14 20:17:20, Ilya Sherman wrote:
> nit: Please leave a space after "DEPRECATED:"; ditto below.

Done.

[Garrett Casto, 2014-10-15 00:12:00]

The CQ bit was checked by gcasto@chromium.org

[commit-bot: I haz the power, 2014-10-15 00:13:51]

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

[commit-bot: I haz the power, 2014-10-15 00:42:43]

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

[commit-bot: I haz the power, 2014-10-15 00:42:44]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/73392)
  linux_chromium_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win8_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel...)

[Garrett Casto, 2014-10-15 18:15:28]

The CQ bit was checked by gcasto@chromium.org

[commit-bot: I haz the power, 2014-10-15 18:17:09]

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

[commit-bot: I haz the power, 2014-10-15 19:20:43]

Committed patchset #8 (id:210001)

[commit-bot: I haz the power, 2014-10-15 19:21:25]

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