New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 772027 link

Starred by 0 users
Status: Fixed
Owner:
█ awdf@chromium.org
Last visit > 30 days ago
Closed: Jul 31
Cc:
dtrainor@chromium.org
peter@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Task

Blocked on:
issue 867879


Sign in to add a comment

Make it so devs can't forget to update channel id sent to notification tracker

Project Member Reported by awdf@chromium.org, Oct 5 2017 
It's now happened more than once* that I've only noticed in code review that the channel id sent to the NotificationUmaTracker has not been updated when the channel id of a notification is updated.

We should really structure the code so that this mistake is impossible to make (force the onNotificatoinShown method to take the notification object itself? This might be hard in some cases though..)

[*] e.g. https://chromium-review.googlesource.com/c/chromium/src/+/699714

Comment 1 by awdf@chromium.org, Jul 26

Blockedon: 867879
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 27 

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

commit ab9827f4ff3ed76fb2b301aadf08f6f04977f0d1
Author: Anita Woodruff <awdf@chromium.org>
Date: Fri Jul 27 09:09:40 2018

[Android Media Notification Tests] Remove unnecessary stubbing

- This doNothing() is not actually needed for the tests to pass; I
suspect it's a hangover from when these tests were instrumentation
tests.

R=mlamouri@chromium.org

Bug:  772027 
Change-Id: Ia2b2103d2bdf6962c065ed5e34ecb1333d8300f5
Reviewed-on: https://chromium-review.googlesource.com/1151320
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578569}
[modify] https://crrev.com/ab9827f4ff3ed76fb2b301aadf08f6f04977f0d1/chrome/android/junit/src/org/chromium/chrome/browser/media/ui/MediaNotificationManagerServiceLifecycleTest.java
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 27 

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

commit 66fe49e8636056db920eec7220a92ce6b1eb3b74
Author: Anita Woodruff <awdf@chromium.org>
Date: Fri Jul 27 11:07:49 2018

[Android Notifications] Pass notification to UmaTracker

This prevents developers updating the channel id of a notification and
forgetting to update the channel id that is logged.

Bug:  772027 
Change-Id: Ic32cead46f91a24d3164592c196b527fc21edcdd
Reviewed-on: https://chromium-review.googlesource.com/1148321
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578589}
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/browseractions/BrowserActionsService.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/browserservices/TrustedWebActivityClient.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService2.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/incognito/IncognitoNotificationManager.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/media/MediaCaptureNotificationService.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUmaTracker.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContentSuggestionsNotifier.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/prefetch/PrefetchedPagesNotifier.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkDisclosureNotificationManager.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappActionsNotificationManager.java
[modify] https://crrev.com/66fe49e8636056db920eec7220a92ce6b1eb3b74/chrome/android/junit/src/org/chromium/chrome/browser/media/ui/MediaNotificationManagerServiceLifecycleTest.java

Comment 4 by awdf@chromium.org, Jul 31

Status: Fixed (was: Available)
►

Sign in to add a comment