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

Issue 801535 link

Starred by 1 user
Status: Fixed
Owner: ----
Closed: Jan 2018
Cc:
█ awdf@chromium.org
peter@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Simplify NotificationID and Tag

Project Member Reported by na...@chromium.org, Jan 12 2018 
Explicit AI: NotificationPlatformBridgeAndroid::Display() in  chrome/browser/notifications/notification_platform_bridge_android.cc is setting tag to notification.id and then passing around both.
Simplify this code a bit by removing the tag field from Java and just using the notification id.

Stretch goal: notification id and tag are two overloaded terms in the notification codebase. Walk through various codepaths to understand which is which, and rename to improve code clarity.
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 24 2018 

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

commit 832946ed5eed7db6479d288b9afbe16febcdabbf
Author: Mugdha Lakhani <nator@chromium.org>
Date: Wed Jan 24 11:01:44 2018

[Android Notifications] Remove unused fields

Remove origin and tag fields from various functions since they're no longer used.

Bug:  801221 ,  801535 
Change-Id: I0b105acbf48cd3097b96ab40358dbec7ca0ec1fc
Reviewed-on: https://chromium-review.googlesource.com/879202
Reviewed-by: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531497}
[modify] https://crrev.com/832946ed5eed7db6479d288b9afbe16febcdabbf/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationConstants.java
[modify] https://crrev.com/832946ed5eed7db6479d288b9afbe16febcdabbf/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationJobService.java
[modify] https://crrev.com/832946ed5eed7db6479d288b9afbe16febcdabbf/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
[modify] https://crrev.com/832946ed5eed7db6479d288b9afbe16febcdabbf/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationService.java
[modify] https://crrev.com/832946ed5eed7db6479d288b9afbe16febcdabbf/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridgeIntentTest.java
[modify] https://crrev.com/832946ed5eed7db6479d288b9afbe16febcdabbf/chrome/browser/notifications/notification_platform_bridge_android.cc
[modify] https://crrev.com/832946ed5eed7db6479d288b9afbe16febcdabbf/chrome/browser/notifications/notification_platform_bridge_android.h

Comment 2 by na...@chromium.org, Jan 24 2018

Status: Fixed (was: Untriaged)
►

Sign in to add a comment