Log password manager UI state changes

Log password manager UI state changes

Log changes of the password manager UI state (bubble and icon) through about:password-manager-internals.

BUG=439064
Committed: https://crrev.com/c5feb0407d876d63c4db686e0b7c8b8f2b73bf6e
Cr-Commit-Position: refs/heads/master@{#310764}

[vabr (Chromium), 2015-01-08 16:30:44]

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

[vabr (Chromium), 2015-01-08 16:30:45]

Hi Mike,

Please have a look.

I'm trying to hunt an intermittent bug where on some sites we don't
automatically pop up the bubble, although we should (and when the user clicks on
it, it opens all right).

I cannot 100% reproduce at the moment (I used to be able to on CrOS, but no
longer am), so I want to have some logging in place for when it emerges again.

I can mark it as temporary if you prefer, but maybe it will be useful in the
future as well.

Cheers,
Vaclav

[Mike West, 2015-01-08 16:52:41]

LGTM. Log all the things, as long as they're behind a toggle there's practically
zero impact.

https://codereview.chromium.org/839023003/diff/1/chrome/browser/ui/passwords/...
File chrome/browser/ui/passwords/manage_passwords_ui_controller.cc (right):

https://codereview.chromium.org/839023003/diff/1/chrome/browser/ui/passwords/...
chrome/browser/ui/passwords/manage_passwords_ui_controller.cc:295: state);
Nit: Is a number going to be enough? It might be nice to map these numbers to
readable strings.

[vabr (Chromium), 2015-01-08 17:00:35]

Thanks, Mike.

Given your approval, I'll send this to the CQ now.

Cheers,
Vaclav

https://codereview.chromium.org/839023003/diff/1/chrome/browser/ui/passwords/...
File chrome/browser/ui/passwords/manage_passwords_ui_controller.cc (right):

https://codereview.chromium.org/839023003/diff/1/chrome/browser/ui/passwords/...
chrome/browser/ui/passwords/manage_passwords_ui_controller.cc:295: state);
On 2015/01/08 16:52:41, Mike West wrote:
> Nit: Is a number going to be enough? It might be nice to map these numbers to
> readable strings.

I thought about that, but because the logger API does not allow free form
strings on purpose, I would either have to define a lot of new string IDs, or a
new log method specific to the State type (which would be a layering violation).
In the end, I was OK with translating from numbers to states on my own. After
all, this kind of translating will not be done too often.

[vabr (Chromium), 2015-01-08 17:00:40]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2015-01-08 17:02:15]

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

[commit-bot: I haz the power, 2015-01-08 17:06:50]

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

[commit-bot: I haz the power, 2015-01-08 17:06:51]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  android_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_compile...)
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[vabr (Chromium), 2015-01-08 17:17:39]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2015-01-08 17:20:03]

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

[commit-bot: I haz the power, 2015-01-08 17:58:58]

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

[commit-bot: I haz the power, 2015-01-08 17:58:58]

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

[vabr (Chromium), 2015-01-09 08:53:05]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2015-01-09 08:53:35]

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

[commit-bot: I haz the power, 2015-01-09 08:57:15]

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

[commit-bot: I haz the power, 2015-01-09 08:57:16]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win_chromium_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[vabr (Chromium), 2015-01-09 09:09:33]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2015-01-09 09:10:39]

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

[commit-bot: I haz the power, 2015-01-09 10:32:06]

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

[commit-bot: I haz the power, 2015-01-09 10:32:07]

Try jobs failed on following builders:
  linux_chromium_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[vabr (Chromium), 2015-01-09 11:56:50]

I had to handle the absence of PasswordManagerClient gracefully for tests, see
patch set 4.

Cheers,
Vaclav

[Mike West, 2015-01-09 11:59:53]

still LGTM.

[vabr (Chromium), 2015-01-09 12:01:10]

The CQ bit was checked by vabr@chromium.org

[commit-bot: I haz the power, 2015-01-09 12:02:05]

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

[commit-bot: I haz the power, 2015-01-09 12:45:23]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-01-09 12:46:09]

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