Add Opt-in notification UMA for the Physical Web

Add Opt-in notification UMA for the Physical Web

BUG=529962
Committed: https://crrev.com/38330845ad64d955805d1849a7b13191e3d0bf3f
Cr-Commit-Position: refs/heads/master@{#367125}

[cco3, 2015-12-22 20:27:22]

cco3@chromium.org changed reviewers:
+ newt@chromium.org

[cco3, 2015-12-22 20:27:39]

Description was changed from

==========
Add Opt-in notification UMA for the Physical Web

BUG=529962
==========

to

==========
Add Opt-in notification UMA for the Physical Web

BUG=529962
==========

[cco3, 2015-12-29 18:28:17]

cco3@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[mattreynolds, 2015-12-29 18:35:45]

mattreynolds@chromium.org changed reviewers:
+ mattreynolds@chromium.org

[mattreynolds, 2015-12-29 18:35:46]

https://codereview.chromium.org/1544863003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
(right):

https://codereview.chromium.org/1544863003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java:36:
private static final String OPT_IN_LOW_PRIORITY_NOTIFICATION_COUNT =
We should use MIN_PRIORITY in our naming in case we actually use LOW_PRIORITY
notifications in the future.

[cco3, 2015-12-29 18:47:03]

https://codereview.chromium.org/1544863003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
(right):

https://codereview.chromium.org/1544863003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java:36:
private static final String OPT_IN_LOW_PRIORITY_NOTIFICATION_COUNT =
On 2015/12/29 18:35:46, mattreynolds wrote:
> We should use MIN_PRIORITY in our naming in case we actually use LOW_PRIORITY
> notifications in the future.

Done.

[cco3, 2015-12-29 18:57:32]

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

[gone, 2015-12-29 19:09:14]

lgtm, same comments about the histograms as the other CL.

https://codereview.chromium.org/1544863003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
(right):

https://codereview.chromium.org/1544863003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java:82:
* Records a display of a low priority opt-in notification.
min priority

https://codereview.chromium.org/1544863003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java:209:
private static class UmaUploader implements Runnable {
Bit of a misnomer.  UmaRecorder would be more appropriate because the
MetricsService is the thing that handles scheduling uploads and such.

[cco3, 2015-12-29 19:15:27]

https://codereview.chromium.org/1544863003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
(right):

https://codereview.chromium.org/1544863003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java:82:
* Records a display of a low priority opt-in notification.
On 2015/12/29 19:09:14, dfalcantara wrote:
> min priority

Done.

https://codereview.chromium.org/1544863003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java:209:
private static class UmaUploader implements Runnable {
On 2015/12/29 19:09:14, dfalcantara wrote:
> Bit of a misnomer.  UmaRecorder would be more appropriate because the
> MetricsService is the thing that handles scheduling uploads and such.

I think UmaRecorder might also be a misnomer given that that we've already
recorded these metrics to disk and are now opportunistically calling
RecordUserAction.  If you think UmaRecorder would be better though, I can change
it.

[gone, 2015-12-29 19:31:45]

https://codereview.chromium.org/1544863003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
(right):

https://codereview.chromium.org/1544863003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java:209:
private static class UmaUploader implements Runnable {
On 2015/12/29 19:15:27, cco3 wrote:
> On 2015/12/29 19:09:14, dfalcantara wrote:
> > Bit of a misnomer.  UmaRecorder would be more appropriate because the
> > MetricsService is the thing that handles scheduling uploads and such.
> 
> I think UmaRecorder might also be a misnomer given that that we've already
> recorded these metrics to disk and are now opportunistically calling
> RecordUserAction.  If you think UmaRecorder would be better though, I can
change
> it.

RecordUserAction just stores the values in the MetricsService until the log is
ready to be cut and uploaded, though.  I don't mind keeping the class name
as-is, but just keep in mind that these records aren't guaranteed to be uploaded
to the server straightaway.

[cco3, 2015-12-29 19:35:56]

https://codereview.chromium.org/1544863003/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
(right):

https://codereview.chromium.org/1544863003/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java:209:
private static class UmaUploader implements Runnable {
On 2015/12/29 19:31:45, dfalcantara wrote:
> On 2015/12/29 19:15:27, cco3 wrote:
> > On 2015/12/29 19:09:14, dfalcantara wrote:
> > > Bit of a misnomer.  UmaRecorder would be more appropriate because the
> > > MetricsService is the thing that handles scheduling uploads and such.
> > 
> > I think UmaRecorder might also be a misnomer given that that we've already
> > recorded these metrics to disk and are now opportunistically calling
> > RecordUserAction.  If you think UmaRecorder would be better though, I can
> change
> > it.
> 
> RecordUserAction just stores the values in the MetricsService until the log is
> ready to be cut and uploaded, though.  I don't mind keeping the class name
> as-is, but just keep in mind that these records aren't guaranteed to be
uploaded
> to the server straightaway.

Understood.

[cco3, 2015-12-29 20:20:40]

Hi isherman@, would you be able to lgtm actions.xml?

[Ilya Sherman, 2015-12-29 22:50:52]

actions.xml LGTM.  Same comments as on the other CL in terms of user actions vs.
histograms.

[cco3, 2015-12-29 22:56:15]

The CQ bit was checked by cco3@chromium.org

[cco3, 2015-12-29 22:56:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/1544863003/#ps60001
(title: "Fix comment to speak of "min" priority")

[commit-bot: I haz the power, 2015-12-29 22:56:25]

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

[commit-bot: I haz the power, 2015-12-29 23:39:00]

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

[commit-bot: I haz the power, 2015-12-29 23:39:01]

Failed to apply patch for
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java:
While running git apply --index -3 -p1;
  error: patch failed:
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java:27
  Falling back to three-way merge...
  Applied patch to
'chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java'
with conflicts.
  U
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java

Patch:      
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
Index:
chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
diff --git
a/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
b/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
index
9c6d6f50dbdfd2a54bb06719f0ad5a183ab7f6a5..d6bbe837745863cb3f159e6cf26c99fdeae7504a
100644
---
a/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
+++
b/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebUma.java
@@ -27,6 +27,16 @@ import javax.annotation.concurrent.ThreadSafe;
 class PhysicalWebUma {
     private static final String TAG = "PhysicalWeb";
     private static final String NOTIFICATION_PRESS_COUNT =
"PhysicalWeb.NotificationPressed";
+    private static final String OPT_IN_DECLINE_BUTTON_PRESS_COUNT =
+            "PhysicalWeb.OptIn.DeclineButtonPressed";
+    private static final String OPT_IN_ENABLE_BUTTON_PRESS_COUNT =
+            "PhysicalWeb.OptIn.EnableButtonPressed";
+    private static final String OPT_IN_HIGH_PRIORITY_NOTIFICATION_COUNT =
+            "PhysicalWeb.OptIn.HighPriorityNotificationShown";
+    private static final String OPT_IN_MIN_PRIORITY_NOTIFICATION_COUNT =
+            "PhysicalWeb.OptIn.MinPriorityNotificationShown";
+    private static final String OPT_IN_NOTIFICATION_PRESS_COUNT =
+            "PhysicalWeb.OptIn.NotificationPressed";
     private static final String PWS_BACKGROUND_RESOLVE_TIMES =
"PhysicalWeb.ResolveTime.Background";
     private static final String PWS_FOREGROUND_RESOLVE_TIMES =
"PhysicalWeb.ResolveTime.Foreground";
     private static final String URL_SELECTED_COUNT = "PhysicalWeb.UrlSelected";
@@ -48,6 +58,41 @@ class PhysicalWebUma {
     }
 
     /**
+     * Records a tap on the opt-in decline button.
+     */
+    public static void onOptInDeclineButtonPressed(Context context) {
+        handleAction(context, OPT_IN_DECLINE_BUTTON_PRESS_COUNT);
+    }
+
+    /**
+     * Records a tap on the opt-in enable button.
+     */
+    public static void onOptInEnableButtonPressed(Context context) {
+        handleAction(context, OPT_IN_ENABLE_BUTTON_PRESS_COUNT);
+    }
+
+    /**
+     * Records a display of a high priority opt-in notification.
+     */
+    public static void onOptInHighPriorityNotificationShown(Context context) {
+        handleAction(context, OPT_IN_HIGH_PRIORITY_NOTIFICATION_COUNT);
+    }
+
+    /**
+     * Records a display of a min priority opt-in notification.
+     */
+    public static void onOptInMinPriorityNotificationShown(Context context) {
+        handleAction(context, OPT_IN_MIN_PRIORITY_NOTIFICATION_COUNT);
+    }
+
+    /**
+     * Records a display of the opt-in activity.
+     */
+    public static void onOptInNotificationPressed(Context context) {
+        handleAction(context, OPT_IN_NOTIFICATION_PRESS_COUNT);
+    }
+
+    /**
      * Records a response time from PWS for a resolution during a background
scan.
      * @param duration The length of time PWS took to respond.
      */
@@ -89,6 +134,13 @@ class PhysicalWebUma {
         SharedPreferences prefs =
PreferenceManager.getDefaultSharedPreferences(context);
         uploader.notificationPressCount =
prefs.getInt(NOTIFICATION_PRESS_COUNT, 0);
         uploader.urlSelectedCount = prefs.getInt(URL_SELECTED_COUNT, 0);
+        uploader.optInDeclineButtonTapCount =
prefs.getInt(OPT_IN_DECLINE_BUTTON_PRESS_COUNT, 0);
+        uploader.optInEnableButtonTapCount =
prefs.getInt(OPT_IN_ENABLE_BUTTON_PRESS_COUNT, 0);
+        uploader.optInHighPriorityNotificationCount =
+                prefs.getInt(OPT_IN_HIGH_PRIORITY_NOTIFICATION_COUNT, 0);
+        uploader.optInMinPriorityNotificationCount =
+                prefs.getInt(OPT_IN_MIN_PRIORITY_NOTIFICATION_COUNT, 0);
+        uploader.optInNotificationPressCount =
prefs.getInt(OPT_IN_NOTIFICATION_PRESS_COUNT, 0);
         uploader.pwsBackgroundResolveTimes =
prefs.getString(PWS_BACKGROUND_RESOLVE_TIMES, "[]");
         uploader.pwsForegroundResolveTimes =
prefs.getString(PWS_FOREGROUND_RESOLVE_TIMES, "[]");
         uploader.urlsDisplayedCounts = prefs.getString(URLS_DISPLAYED_COUNTS,
"[]");
@@ -102,6 +154,11 @@ class PhysicalWebUma {
         prefs.edit()
                 .remove(NOTIFICATION_PRESS_COUNT)
                 .remove(URL_SELECTED_COUNT)
+                .remove(OPT_IN_DECLINE_BUTTON_PRESS_COUNT)
+                .remove(OPT_IN_ENABLE_BUTTON_PRESS_COUNT)
+                .remove(OPT_IN_HIGH_PRIORITY_NOTIFICATION_COUNT)
+                .remove(OPT_IN_MIN_PRIORITY_NOTIFICATION_COUNT)
+                .remove(OPT_IN_NOTIFICATION_PRESS_COUNT)
                 .remove(PWS_BACKGROUND_RESOLVE_TIMES)
                 .remove(PWS_FOREGROUND_RESOLVE_TIMES)
                 .remove(URLS_DISPLAYED_COUNTS)
@@ -152,6 +209,11 @@ class PhysicalWebUma {
     private static class UmaUploader implements Runnable {
         public int notificationPressCount;
         public int urlSelectedCount;
+        public int optInDeclineButtonTapCount;
+        public int optInEnableButtonTapCount;
+        public int optInHighPriorityNotificationCount;
+        public int optInMinPriorityNotificationCount;
+        public int optInNotificationPressCount;
         public String pwsBackgroundResolveTimes;
         public String pwsForegroundResolveTimes;
         public String urlsDisplayedCounts;
@@ -159,6 +221,11 @@ class PhysicalWebUma {
         public boolean isEmpty() {
             return notificationPressCount == 0
                     && urlSelectedCount == 0
+                    && optInDeclineButtonTapCount == 0
+                    && optInEnableButtonTapCount == 0
+                    && optInHighPriorityNotificationCount == 0
+                    && optInMinPriorityNotificationCount == 0
+                    && optInNotificationPressCount == 0
                     && pwsBackgroundResolveTimes.equals("[]")
                     && pwsForegroundResolveTimes.equals("[]")
                     && urlsDisplayedCounts.equals("[]");
@@ -171,6 +238,13 @@ class PhysicalWebUma {
         public void run() {
             uploadActions(notificationPressCount, NOTIFICATION_PRESS_COUNT);
             uploadActions(urlSelectedCount, URL_SELECTED_COUNT);
+            uploadActions(optInDeclineButtonTapCount,
OPT_IN_DECLINE_BUTTON_PRESS_COUNT);
+            uploadActions(optInEnableButtonTapCount,
OPT_IN_ENABLE_BUTTON_PRESS_COUNT);
+            uploadActions(optInHighPriorityNotificationCount,
+                    OPT_IN_HIGH_PRIORITY_NOTIFICATION_COUNT);
+            uploadActions(optInMinPriorityNotificationCount,
+                    OPT_IN_MIN_PRIORITY_NOTIFICATION_COUNT);
+            uploadActions(optInNotificationPressCount,
OPT_IN_NOTIFICATION_PRESS_COUNT);
             uploadTimes(pwsBackgroundResolveTimes,
PWS_BACKGROUND_RESOLVE_TIMES,
                     TimeUnit.MILLISECONDS);
             uploadTimes(pwsForegroundResolveTimes,
PWS_FOREGROUND_RESOLVE_TIMES,

[commit-bot: I haz the power, 2015-12-29 23:39:18]

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

[commit-bot: I haz the power, 2015-12-29 23:39:19]

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

[cco3, 2015-12-30 00:59:59]

The CQ bit was checked by cco3@chromium.org

[cco3, 2015-12-30 01:00:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/1544863003/#ps80001
(title: "Rebased")

[commit-bot: I haz the power, 2015-12-30 01:00:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1544863003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1544863003/80001

[commit-bot: I haz the power, 2015-12-30 01:47:56]

Description was changed from

==========
Add Opt-in notification UMA for the Physical Web

BUG=529962
==========

to

==========
Add Opt-in notification UMA for the Physical Web

BUG=529962
==========

[commit-bot: I haz the power, 2015-12-30 01:47:57]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-12-30 01:48:49]

Description was changed from

==========
Add Opt-in notification UMA for the Physical Web

BUG=529962
==========

to

==========
Add Opt-in notification UMA for the Physical Web

BUG=529962

Committed: https://crrev.com/38330845ad64d955805d1849a7b13191e3d0bf3f
Cr-Commit-Position: refs/heads/master@{#367125}
==========

[commit-bot: I haz the power, 2015-12-30 01:48:50]

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