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

Issue 800361 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 800363



Sign in to add a comment

Use the notification Id as the tag in Android's notification bridge

Project Member Reported by peter@chromium.org, Jan 9 2018

Issue description

In order to properly replace notifications, Chrome on Android creates a "platform tag" that combines the notification's Id, the origin showing the notification and the developer's tag, if supplied.

https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java?l=381

It later parses this tag to get data back, e.g. the origin, in NotificationPlatformBridge::getOriginFromNotificationTag().

https://cs.chromium.org/chromium/src/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java?l=429

The notificationId already contains the origin and, if necessary, the developer's tag. We can simplify this code by just using that argument as the platform tag.

We will still need to be able to get the origin from the platform tag, which means we'd have to reimplement getOriginFromNotificationTag() to work with the syntax of the NotificationIdGenerator:

https://cs.chromium.org/chromium/src/content/browser/notifications/notification_id_generator.cc

It might make sense to add a separator there?

 

Comment 1 by na...@chromium.org, Jan 9 2018

Blocking: 800363

Comment 2 by awdf@chromium.org, Jan 10 2018

Description: Show this description
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 12 2018

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

commit 3e41705981a543b8bfcc7437efb656b637a6c242
Author: Mugdha Lakhani <nator@chromium.org>
Date: Fri Jan 12 17:48:01 2018

[Android Notifications] Use notificationId as platform tag.

Chrome's notification bridge on Android combines notification id,
origin and a developer tag into a Platform tag. Platform tag is the one
we provide to Android in NotificationManager.notify().
Notification Id contains origin and developer tag info already, so
it's sufficient to use the ID on its own as the platform tag.

Bug:  800361 
Change-Id: I6947dafdcb54e520c7ccddc910a643826e028f83
Reviewed-on: https://chromium-review.googlesource.com/857512
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528989}
[modify] https://crrev.com/3e41705981a543b8bfcc7437efb656b637a6c242/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationConstants.java
[modify] https://crrev.com/3e41705981a543b8bfcc7437efb656b637a6c242/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
[modify] https://crrev.com/3e41705981a543b8bfcc7437efb656b637a6c242/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridgeIntentTest.java
[modify] https://crrev.com/3e41705981a543b8bfcc7437efb656b637a6c242/chrome/android/junit/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridgeUnitTest.java
[modify] https://crrev.com/3e41705981a543b8bfcc7437efb656b637a6c242/chrome/browser/notifications/notification_platform_bridge_android.cc
[modify] https://crrev.com/3e41705981a543b8bfcc7437efb656b637a6c242/chrome/browser/notifications/notification_platform_bridge_android.h
[modify] https://crrev.com/3e41705981a543b8bfcc7437efb656b637a6c242/chrome/browser/notifications/platform_notification_service_interactive_uitest.cc
[modify] https://crrev.com/3e41705981a543b8bfcc7437efb656b637a6c242/content/browser/notifications/notification_id_generator.cc
[modify] https://crrev.com/3e41705981a543b8bfcc7437efb656b637a6c242/content/browser/notifications/notification_id_generator.h

Comment 4 by na...@chromium.org, Jan 15 2018

Status: Fixed (was: Assigned)

Sign in to add a comment