New issue
Advanced search Search tips

Issue 898269 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Notification metrics centralized layer

Project Member Reported by xingliu@chromium.org, Oct 23

Issue description

Chrome Version: 72.*
OS: Android

Currently the use cases for notification is scattered and Chrome has a shim layer of API to wrap Android notification API.

To collect metrics like click through rates and provide single point entry point and deletion point, we can make most notification started/ended from a single broadcast receiver/deletion receiver. And record common metrics in this layer.
 
Summary: Notification metrics centralized layer (was: Notification metrics)
Cc: heamy@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ac2feaefd888a09a18ea514529b4c5de3858823

commit 0ac2feaefd888a09a18ea514529b4c5de3858823
Author: Xing Liu <xingliu@chromium.org>
Date: Mon Nov 19 19:24:16 2018

Android Notification: Intercept PendingIntent for metrics recording.

This CL introduces a way to intercept all PendingIntents from
notification with broadcast receivers. In the following CL we will
add metrics recording into those broadcast receivers. This class will
eventually be used in ChromeNotificationBuilder.

Also we introduce a test that uses UIAutomator to actually click on
a notification in test instead of mocking Android notification API.
And verify the BroadcastReceiver interception.

Bug: 898269
Change-Id: I16e52f7036db4d30093517e16373547d5e1e862c
Reviewed-on: https://chromium-review.googlesource.com/c/1327803
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609390}
[modify] https://crrev.com/0ac2feaefd888a09a18ea514529b4c5de3858823/chrome/android/java/AndroidManifest.xml
[add] https://crrev.com/0ac2feaefd888a09a18ea514529b4c5de3858823/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationIntentInterceptor.java
[modify] https://crrev.com/0ac2feaefd888a09a18ea514529b4c5de3858823/chrome/android/java_sources.gni
[add] https://crrev.com/0ac2feaefd888a09a18ea514529b4c5de3858823/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/NotificationIntentInterceptorTest.java
[modify] https://crrev.com/0ac2feaefd888a09a18ea514529b4c5de3858823/chrome/android/javatests/src/org/chromium/chrome/browser/partnercustomizations/PartnerDisableIncognitoModeIntegrationTest.java
[modify] https://crrev.com/0ac2feaefd888a09a18ea514529b4c5de3858823/chrome/test/android/javatests/src/org/chromium/chrome/test/ChromeActivityTestRule.java

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b5f509ea5d7043572f7fa6950863f54c4fd1740

commit 1b5f509ea5d7043572f7fa6950863f54c4fd1740
Author: Xing Liu <xingliu@chromium.org>
Date: Fri Dec 14 19:29:01 2018

Android notification: Introduce ChromeNotification/PendingIntentProvider.

The Android API didn't expose notification id/tag and the int flags to
create Notification's PendingIntent, however we need to use these data
when recording Chrome notification metrics.

This CL adds wrapper class that contains these data.
ChromeNotificationBuilder will use these wrapper class in the future to
force metrics tracking and usage of NotificationManagerProxy.

Also we setup a new component for notification.

TBR=dtrainor@chromium.org, peter@chromium.org

Bug: 898269
Change-Id: I63e3976c729d71e761c8c6b5c0aead3763ba5e17
Reviewed-on: https://chromium-review.googlesource.com/c/1358650
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616783}
[add] https://crrev.com/1b5f509ea5d7043572f7fa6950863f54c4fd1740/chrome/android/java/src/org/chromium/chrome/browser/notifications/ChromeNotification.java
[add] https://crrev.com/1b5f509ea5d7043572f7fa6950863f54c4fd1740/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationMetadata.java
[add] https://crrev.com/1b5f509ea5d7043572f7fa6950863f54c4fd1740/chrome/android/java/src/org/chromium/chrome/browser/notifications/PendingIntentProvider.java

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8914c8150d9da36b057ec05358943d90456b2d8

commit f8914c8150d9da36b057ec05358943d90456b2d8
Author: Xing Liu <xingliu@chromium.org>
Date: Fri Dec 14 22:28:32 2018

Revert "Android notification: Introduce ChromeNotification/PendingIntentProvider."

This reverts commit 1b5f509ea5d7043572f7fa6950863f54c4fd1740.

Reason for revert: Didn't update build file when move code back to chrome.

Original change's description:
> Android notification: Introduce ChromeNotification/PendingIntentProvider.
> 
> The Android API didn't expose notification id/tag and the int flags to
> create Notification's PendingIntent, however we need to use these data
> when recording Chrome notification metrics.
> 
> This CL adds wrapper class that contains these data.
> ChromeNotificationBuilder will use these wrapper class in the future to
> force metrics tracking and usage of NotificationManagerProxy.
> 
> Also we setup a new component for notification.
> 
> TBR=dtrainor@chromium.org, peter@chromium.org
> 
> Bug: 898269
> Change-Id: I63e3976c729d71e761c8c6b5c0aead3763ba5e17
> Reviewed-on: https://chromium-review.googlesource.com/c/1358650
> Reviewed-by: Xing Liu <xingliu@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
> Commit-Queue: Xing Liu <xingliu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#616783}

TBR=peter@chromium.org,nyquist@chromium.org,dtrainor@chromium.org,blundell@chromium.org,xingliu@chromium.org

Change-Id: Ic33e9232084c76c318caece0b133480f26cf43b1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 898269
Reviewed-on: https://chromium-review.googlesource.com/c/1379058
Reviewed-by: Xing Liu <xingliu@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616852}
[delete] https://crrev.com/2be65e2d62ae3dbceb2b1815cf31fc3721f3cd4b/chrome/android/java/src/org/chromium/chrome/browser/notifications/ChromeNotification.java
[delete] https://crrev.com/2be65e2d62ae3dbceb2b1815cf31fc3721f3cd4b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationMetadata.java
[delete] https://crrev.com/2be65e2d62ae3dbceb2b1815cf31fc3721f3cd4b/chrome/android/java/src/org/chromium/chrome/browser/notifications/PendingIntentProvider.java

Project Member

Comment 7 by bugdroid1@chromium.org, Jan 16 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b8d118134dd7a005e2bbad8ea9007e14062bf16c

commit b8d118134dd7a005e2bbad8ea9007e14062bf16c
Author: Xing Liu <xingliu@chromium.org>
Date: Wed Jan 16 22:37:47 2019

Android notification: Add setContentIntent API with metric tracking.

This CL adds setContentIntent(PendingIntentProvider) in
ChromeNotificationBuilder. The old API is marked as deprecated and we
will gradually convert all notification use cases to new API.

Consumer can use this API to track notification body click events.

TBR=holte@chromium.org

Bug: 898269
Change-Id: If5aad3b4a7cf949b2d7fc4e3baf0e2b929931462
Reviewed-on: https://chromium-review.googlesource.com/c/1379223
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623412}
[modify] https://crrev.com/b8d118134dd7a005e2bbad8ea9007e14062bf16c/chrome/android/java/src/org/chromium/chrome/browser/notifications/ChromeNotificationBuilder.java
[modify] https://crrev.com/b8d118134dd7a005e2bbad8ea9007e14062bf16c/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilder.java
[modify] https://crrev.com/b8d118134dd7a005e2bbad8ea9007e14062bf16c/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderFactory.java
[modify] https://crrev.com/b8d118134dd7a005e2bbad8ea9007e14062bf16c/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationCompatBuilder.java
[modify] https://crrev.com/b8d118134dd7a005e2bbad8ea9007e14062bf16c/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationIntentInterceptor.java
[modify] https://crrev.com/b8d118134dd7a005e2bbad8ea9007e14062bf16c/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationMetadata.java
[modify] https://crrev.com/b8d118134dd7a005e2bbad8ea9007e14062bf16c/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUmaTracker.java
[modify] https://crrev.com/b8d118134dd7a005e2bbad8ea9007e14062bf16c/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/NotificationIntentInterceptorTest.java
[modify] https://crrev.com/b8d118134dd7a005e2bbad8ea9007e14062bf16c/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/NotificationTestUtil.java
[modify] https://crrev.com/b8d118134dd7a005e2bbad8ea9007e14062bf16c/tools/metrics/histograms/histograms.xml

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 18 (4 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3c6f50faffdaeafac2683aee95e2d5364a86a97d

commit 3c6f50faffdaeafac2683aee95e2d5364a86a97d
Author: Xing Liu <xingliu@chromium.org>
Date: Fri Jan 18 23:31:56 2019

Tracks Android notification dismiss event.

This CL adds an API in ChromeNotificationBuilder to track notification
dismiss in UMA.

Bug: 898269
Change-Id: I0d33150eb98dfcd96c6807ab490519a39fe79101
Reviewed-on: https://chromium-review.googlesource.com/c/1387811
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624351}
[modify] https://crrev.com/3c6f50faffdaeafac2683aee95e2d5364a86a97d/chrome/android/java/src/org/chromium/chrome/browser/notifications/ChromeNotificationBuilder.java
[modify] https://crrev.com/3c6f50faffdaeafac2683aee95e2d5364a86a97d/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilder.java
[modify] https://crrev.com/3c6f50faffdaeafac2683aee95e2d5364a86a97d/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationCompatBuilder.java
[modify] https://crrev.com/3c6f50faffdaeafac2683aee95e2d5364a86a97d/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationIntentInterceptor.java
[modify] https://crrev.com/3c6f50faffdaeafac2683aee95e2d5364a86a97d/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUmaTracker.java
[modify] https://crrev.com/3c6f50faffdaeafac2683aee95e2d5364a86a97d/tools/metrics/histograms/histograms.xml

Sign in to add a comment