Add UMA histogram for popup actions on Android.

Add UMA histogram for popup actions on Android.

This CL adds metrics for the pop-up blocked infobar on Android.

BUG=650132
Review-Url: https://codereview.chromium.org/2662163002
Cr-Commit-Position: refs/heads/master@{#457285}
Committed: https://chromium.googlesource.com/chromium/src/+/0ab3b67a846c21a42996cf6d0e19a331b31dfacb

[charleszhao, 2017-01-31 01:52:50]

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

[commit-bot: I haz the power, 2017-01-31 01:53:14]

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

[charleszhao, 2017-01-31 02:07:44]

charleszhao@chromium.org changed reviewers:
+ dominickn@chromium.org

[charleszhao, 2017-01-31 02:07:44]

Thanks, Dom

[commit-bot: I haz the power, 2017-01-31 03:48:25]

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

[commit-bot: I haz the power, 2017-01-31 03:48:26]

Dry run: This issue passed the CQ dry run.

[dominickn, 2017-01-31 18:22:14]

lgtm

Please remove the second sentence from the subject (and references to iOS), so
it's just

"Add UMA histogram for popup actions on Android"

Description:

"Add UMA histogram for popup actions on Android

This CL adds metrics for the pop-up blocked infobar on Android."

[charleszhao, 2017-02-01 02:24:04]

Description was changed from

==========
Add UMA histogram for popup actions on Android. histogram for ios is
not added because of the dependency restriction.

BUG=650132
==========

to

==========
Add UMA histogram for popup actions on Android.

This CL adds metrics for the pop-up blocked infobar on Android.

BUG=650132
==========

[charleszhao, 2017-02-01 02:30:06]

On 2017/01/31 18:22:14, dominickn wrote:
> lgtm
> 
> Please remove the second sentence from the subject (and references to iOS), so
> it's just
> 
> "Add UMA histogram for popup actions on Android"
> 
> Description:
> 
> "Add UMA histogram for popup actions on Android
> 
> This CL adds metrics for the pop-up blocked infobar on Android."

Done, thanks Dom.

Hi thakis, need owner review for files:
chrome/browser/content_settings/chrome_content_settings_utils.h
chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc

Hi jwd, need owner review for file:
tools/metrics/histograms/histograms.xml

Thanks, all!

[charleszhao, 2017-02-01 02:31:05]

charleszhao@chromium.org changed reviewers:
+ jwd@chromium.org, thakis@chromium.org

[charleszhao, 2017-02-01 02:31:06]

Thanks!

[charleszhao, 2017-02-01 02:31:23]

On 2017/02/01 02:31:06, charleszhao wrote:
> Thanks!

Hi thakis, need owner review for files:
chrome/browser/content_settings/chrome_content_settings_utils.h
chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc

Hi jwd, need owner review for file:
tools/metrics/histograms/histograms.xml

[jwd, 2017-02-02 16:04:09]

histograms LGTM

[Nico, 2017-02-02 22:09:50]

https://codereview.chromium.org/2662163002/diff/1/chrome/browser/ui/android/c...
File
chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc
(right):

https://codereview.chromium.org/2662163002/diff/1/chrome/browser/ui/android/c...
chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc:54:
content_settings::POPUPS_ACTION_DISPLAYED_INFOBAR_ON_MOBILE);
This looks like general code, why is the enum called _ON_MOBILE?

https://codereview.chromium.org/2662163002/diff/1/chrome/browser/ui/android/c...
chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc:123:
content_settings::POPUPS_ACTION_CLICKED_ALWAYS_SHOW_ON_MOBILE);
same question

[charleszhao, 2017-02-03 02:20:48]

Thanks, Nico

https://codereview.chromium.org/2662163002/diff/1/chrome/browser/ui/android/c...
File
chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc
(right):

https://codereview.chromium.org/2662163002/diff/1/chrome/browser/ui/android/c...
chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc:54:
content_settings::POPUPS_ACTION_DISPLAYED_INFOBAR_ON_MOBILE);
On 2017/02/02 22:09:49, Nico wrote:
> This looks like general code, why is the enum called _ON_MOBILE?

These two enum 
POPUPS_ACTION_DISPLAYED_INFOBAR_ON_MOBILE,
POPUPS_ACTION_CLICKED_ALWAYS_SHOW_ON_MOBILE
are only meaningful on Android and IOS,

And on desktop, the corespondent one is:
POPUPS_ACTION_DISPLAYED_BLOCKED_ICON_IN_OMNIBOX,
POPUPS_ACTION_SELECTED_ALWAYS_ALLOW_POPUPS_FROM,

That's the reason.

[charleszhao, 2017-02-05 23:03:13]

Thanks Nico

https://codereview.chromium.org/2662163002/diff/1/chrome/browser/ui/android/c...
File
chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc
(right):

https://codereview.chromium.org/2662163002/diff/1/chrome/browser/ui/android/c...
chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc:123:
content_settings::POPUPS_ACTION_CLICKED_ALWAYS_SHOW_ON_MOBILE);
On 2017/02/02 22:09:49, Nico wrote:
> same question

Done.

[charleszhao, 2017-02-08 01:53:20]

On 2017/02/05 23:03:13, charleszhao wrote:
> Thanks Nico
> 
>
https://codereview.chromium.org/2662163002/diff/1/chrome/browser/ui/android/c...
> File
> chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc
> (right):
> 
>
https://codereview.chromium.org/2662163002/diff/1/chrome/browser/ui/android/c...
>
chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc:123:
> content_settings::POPUPS_ACTION_CLICKED_ALWAYS_SHOW_ON_MOBILE);
> On 2017/02/02 22:09:49, Nico wrote:
> > same question
> 
> Done.
Hi Nico,  the function RecordPopupsAction is a general function.
but the enum are distinguished on Desktop and Mobile as:

These two enum 
POPUPS_ACTION_DISPLAYED_INFOBAR_ON_MOBILE,
POPUPS_ACTION_CLICKED_ALWAYS_SHOW_ON_MOBILE
are only meaningful on Android and IOS,

And on desktop, the corespondent one is:
POPUPS_ACTION_DISPLAYED_BLOCKED_ICON_IN_OMNIBOX,
POPUPS_ACTION_SELECTED_ALWAYS_ALLOW_POPUPS_FROM,

That's the reason.

[charleszhao, 2017-03-07 23:36:46]

charleszhao@chromium.org changed reviewers:
+ jochen@chromium.org
- thakis@chromium.org

[charleszhao, 2017-03-07 23:37:20]

Hi jochen

need owner review on these two files:
chrome/browser/content_settings/chrome_content_settings_utils.h
chrome/browser/ui/android/content_settings/popup_blocked_infobar_delegate.cc

Thanks!

[jochen (gone - plz use gerrit), 2017-03-08 18:50:51]

lgtm

[charleszhao, 2017-03-15 22:59:48]

The CQ bit was checked by charleszhao@chromium.org

[commit-bot: I haz the power, 2017-03-15 23:00:54]

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-16 00:35:34]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1489618788066630, "parent_rev":
"6020d58dc2052d5bfc6ec36f8eacf4677bda6954", "commit_rev":
"0ab3b67a846c21a42996cf6d0e19a331b31dfacb"}

[commit-bot: I haz the power, 2017-03-16 00:36:18]

Description was changed from

==========
Add UMA histogram for popup actions on Android.

This CL adds metrics for the pop-up blocked infobar on Android.

BUG=650132
==========

to

==========
Add UMA histogram for popup actions on Android.

This CL adds metrics for the pop-up blocked infobar on Android.

BUG=650132

Review-Url: https://codereview.chromium.org/2662163002
Cr-Commit-Position: refs/heads/master@{#457285}
Committed:
https://chromium.googlesource.com/chromium/src/+/0ab3b67a846c21a42996cf6d0e19...
==========

[commit-bot: I haz the power, 2017-03-16 00:36:19]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/0ab3b67a846c21a42996cf6d0e19...