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

Issue 700377 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 735477



Sign in to add a comment

[Android O] One notification channel per origin

Project Member Reported by awdf@chromium.org, Mar 10 2017

Issue description

Right now all web notifications are sent through the Sites channel but ideally we would like there to be a notification channel for each website.

This will involve creating a channel for each origin with push permission, setting the correct channel id for web notifications and changing our site settings UI to take users to Android settings to block/re-allow sites to send notifications. See https://docs.google.com/document/d/1K9pjvlHF1oANNI8TqZgy151tap9zs1KUr2qfBXo1s_4/edit for more details.

We'll also need to make sure we unsubscribe from push any time we see a site has been blocked in Android settings.
 

Comment 1 by awdf@chromium.org, Mar 17 2017

Status: Started (was: Assigned)
Components: Privacy

Comment 3 by awdf@chromium.org, Mar 29 2017

Description: Show this description

Comment 4 by awdf@chromium.org, Mar 30 2017

Labels: -Restrict-View-Google

Comment 5 Deleted

Comment 6 Deleted

Comment 7 Deleted

Project Member

Comment 8 by bugdroid1@chromium.org, May 5 2017

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

commit ad369409491735cc0120a7cc090f476a36c55bd6
Author: awdf <awdf@chromium.org>
Date: Fri May 05 12:24:39 2017

[Android] Refactor: move files to chrome/browser/notifications/channels

- The number of files in chrome/browser/notifications was getting rather
large

- This simply moves all the channel-related code into a new sub-package

BUG= 700377 

Review-Url: https://codereview.chromium.org/2863623002
Cr-Commit-Position: refs/heads/master@{#469631}

[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadNotificationService.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/incognito/IncognitoNotificationManager.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/init/ProcessInitializationHandler.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/media/MediaCaptureNotificationService.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaNotificationManager.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/notifications/CustomNotificationBuilder.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderFactory.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderForO.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationManagerProxy.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationManagerProxyImpl.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUmaTracker.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java
[rename] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelDefinitions.java
[rename] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelsInitializer.java
[rename] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelsUpdater.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/ntp/ContentSuggestionsNotificationHelper.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/physicalweb/PhysicalWebBroadcastService.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/sync/SyncNotificationController.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java/src/org/chromium/chrome/browser/upgrade/PackageReplacedBroadcastReceiver.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/java_sources.gni
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/junit/src/org/chromium/chrome/browser/media/ui/MediaNotificationManagerServiceLifecycleTest.java
[rename] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/junit/src/org/chromium/chrome/browser/notifications/channels/ChannelDefinitionsTest.java
[rename] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/junit/src/org/chromium/chrome/browser/notifications/channels/ChannelsInitializerTest.java
[rename] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/android/junit/src/org/chromium/chrome/browser/notifications/channels/ChannelsUpdaterTest.java
[modify] https://crrev.com/ad369409491735cc0120a7cc090f476a36c55bd6/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/notifications/MockNotificationManagerProxy.java

Project Member

Comment 9 by bugdroid1@chromium.org, May 15 2017

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

commit 19c23fdc0f568042d316dba0c9bd461e9e4e685b
Author: awdf <awdf@chromium.org>
Date: Mon May 15 18:12:55 2017

[Android] Refactor our notification channel definitions

This refactor means we now have two kinds of channels:
  1. PredefinedChannel, with a resId defining channel name.
    - This is what was previously known as ChannelDefinitions.Channel.
    - Can be converted to a channel when required.
  2. Channel - a drop-in replacement for Android's NotificationChannel.
    - This is useful until we target O and can use NotificationChannel
      directly.
    - The ability to create a channel without a resId is required
      to allow site channels to be created in future.

As part of this change it was necessary to convert the existing
ChannelsInitializerTest to an instrumented test, as we can no longer
access the resId now the code refers to Channels. Since the test is
now instrumented, it makes sense to test against the real
NotificationManager, especially to test the fragile code that uses
reflection.

Finally, I made the ChannelsDefinition class no longer instantiable,
 as it didn't make much sense.

BUG= 700377 ,716503

Review-Url: https://codereview.chromium.org/2859233002
Cr-Commit-Position: refs/heads/master@{#471829}

[modify] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderFactory.java
[modify] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationManagerProxy.java
[modify] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationManagerProxyImpl.java
[add] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/Channel.java
[modify] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelDefinitions.java
[modify] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelsInitializer.java
[modify] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelsUpdater.java
[modify] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/android/java_sources.gni
[add] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/channels/ChannelsInitializerTest.java
[modify] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/android/junit/src/org/chromium/chrome/browser/notifications/channels/ChannelDefinitionsTest.java
[delete] https://crrev.com/727b9633cf8e84b3d79be12f6284248a9e129227/chrome/android/junit/src/org/chromium/chrome/browser/notifications/channels/ChannelsInitializerTest.java
[modify] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/android/junit/src/org/chromium/chrome/browser/notifications/channels/ChannelsUpdaterTest.java
[modify] https://crrev.com/19c23fdc0f568042d316dba0c9bd461e9e4e685b/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/notifications/MockNotificationManagerProxy.java

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 9 2017

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

commit 338aa5e24e36752be190da0009bb23e8f913566c
Author: awdf <awdf@chromium.org>
Date: Fri Jun 09 14:14:11 2017

[Android] Adding content settings provider for notification channels

- Right now it just attempts to create channels on grant, and delete
them on resetting the permission.

- Java side is not yet implemented so this is actually a no-op.

BUG= 700377 

Review-Url: https://codereview.chromium.org/2886433002
Cr-Commit-Position: refs/heads/master@{#478267}

[modify] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/android/BUILD.gn
[add] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationSettingsBridge.java
[modify] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/android/java_sources.gni
[modify] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/browser/BUILD.gn
[modify] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/browser/content_settings/host_content_settings_map_factory.cc
[modify] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/browser/engagement/site_engagement_service_unittest.cc
[add] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/browser/notifications/notification_channels_provider_android.cc
[add] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/browser/notifications/notification_channels_provider_android.h
[add] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/browser/notifications/notification_channels_provider_android_unittest.cc
[modify] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/common/chrome_features.cc
[modify] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/common/chrome_features.h
[modify] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/chrome/test/BUILD.gn
[modify] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/components/content_settings/core/browser/host_content_settings_map.cc
[modify] https://crrev.com/338aa5e24e36752be190da0009bb23e8f913566c/components/content_settings/core/browser/host_content_settings_map.h

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 9 2017

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

commit 1af5bbb0e6f4c1ccc79fac777345d30e9e1c8061
Author: awdf <awdf@chromium.org>
Date: Fri Jun 09 15:09:04 2017

[Android] Implement GetRuleIterator for channels provider

- Now the Provider retrieves all the site notification channels and
uses their statuses (ENABLED/BLOCKED) to return a RuleIterator over
notification permissions.

- Note that the Java side still needs to be implemented, right now
it just returns an empty array.

Bug:  700377 
Review-Url: https://codereview.chromium.org/2922473003
Cr-Commit-Position: refs/heads/master@{#478279}

[modify] https://crrev.com/1af5bbb0e6f4c1ccc79fac777345d30e9e1c8061/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationSettingsBridge.java
[modify] https://crrev.com/1af5bbb0e6f4c1ccc79fac777345d30e9e1c8061/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/1af5bbb0e6f4c1ccc79fac777345d30e9e1c8061/chrome/browser/notifications/notification_channels_provider_android.h
[modify] https://crrev.com/1af5bbb0e6f4c1ccc79fac777345d30e9e1c8061/chrome/browser/notifications/notification_channels_provider_android_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Jun 13 2017

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

commit ae1f454f8d4a04265809c76e61e227d0a370008f
Author: Anita Woodruff <awdf@chromium.org>
Date: Tue Jun 13 11:26:26 2017

Tidying SingleWebsitePreferences.java

Small refactor to improve readability of SingleWebsitePreferences:

- Employed anonymous click listener for Clear Data button instead of 'this'
to make code path easier to follow.

- Added a clarifying comment about maxPermissionOrder.

- Tidied displaySitePermissions - this method was getting overlong so
split it up.

Bug:  700377 
Change-Id: Iadd3c322d4690b17d242c509c32b46bd1e57a06d
Reviewed-on: https://chromium-review.googlesource.com/531048
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#478965}
[modify] https://crrev.com/ae1f454f8d4a04265809c76e61e227d0a370008f/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java

Project Member

Comment 13 by bugdroid1@chromium.org, Jun 15 2017

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

commit a7e9481e33f1cd2ee8a088e32fbd3bbb70ebb3d3
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Jun 15 15:06:20 2017

[Android Notification Channels] Ensure origin channels are initialized

- This change ensures individual site channels ('per-origin' channels)
are initialized at the point a web notification is posted to them.

- Note these 'per-origin' channels are still behind the
'SiteNotificationChannels' flag - web notifications are still
posted to the generic 'Sites' channel unless this flag is enabled.

- Previously, if permission was granted while the flag was disabled,
and then the flag was enabled and the site posted a notification,
this notification would fail to post, since per-origin channels were
only created on permission grant. Now, it should succeed.

- Note that even when the flag is removed, this change will still be
necessary for users upgrading with existing notification permissions.

- Eventually we should only initialize these per-origin channels
from native, so all per-origin channel initialization goes through the
same path, but this is a quick fix for now.

Bug:  700377 
Change-Id: I78321eb8eb2b651e2e25eeb74645a7e62b49cb36
Reviewed-on: https://chromium-review.googlesource.com/536953
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479704}
[modify] https://crrev.com/a7e9481e33f1cd2ee8a088e32fbd3bbb70ebb3d3/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelsInitializer.java
[modify] https://crrev.com/a7e9481e33f1cd2ee8a088e32fbd3bbb70ebb3d3/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/SiteChannelsManager.java
[modify] https://crrev.com/a7e9481e33f1cd2ee8a088e32fbd3bbb70ebb3d3/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/channels/ChannelsInitializerTest.java

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 21 2017

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

commit ed00eba523c0ee50375c789223a63c732992802c
Author: Anita Woodruff <awdf@chromium.org>
Date: Wed Jun 21 11:50:14 2017

[Android Notification Channels] Notify observers on status changes

- Since we don't get a callback when the user toggles notification
channel status, instead we can at least notify observers when the value
is noticed to have changed on reads. (Extra polling may follow).

- The notification channels provider now posts a task from
GetRuleIterator to notify observers if any of the channel statuses
have changed.

- The provider also calls NotifyObservers at the end of SetWebsiteSetting,
if handling the setting.

Bug:  700377 
Change-Id: I5eb1fadda00baad501839c0e933c2c9ada43eb6a
Reviewed-on: https://chromium-review.googlesource.com/538636
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481175}
[modify] https://crrev.com/ed00eba523c0ee50375c789223a63c732992802c/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/ed00eba523c0ee50375c789223a63c732992802c/chrome/browser/notifications/notification_channels_provider_android.h
[modify] https://crrev.com/ed00eba523c0ee50375c789223a63c732992802c/chrome/browser/notifications/notification_channels_provider_android_unittest.cc

Comment 15 by peter@chromium.org, Jun 21 2017

Blocking: 735477
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 22 2017

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

commit 067810b6cf159b12c9956147a6d0217526c627f9
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Jun 22 10:28:26 2017

[Android O] Link to OS channel settings from notification permissions

- Now tapping on a site's notification permission from
Site Settings will launch OS channel settings for toggling the setting.

- Note this only applies if the SiteNotificationChannels
feature flag is enabled and SDK level is O or above.

Bug:  700377 
Change-Id: I75faeb1c4125072c50c42b64f9e2826411bd7c49
Reviewed-on: https://chromium-review.googlesource.com/536898
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481498}
[modify] https://crrev.com/067810b6cf159b12c9956147a6d0217526c627f9/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java

Project Member

Comment 17 by bugdroid1@chromium.org, Jun 22 2017

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

commit 78bc677c65488c2aff3da2d91e4322dfc32c6301
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Jun 22 11:30:43 2017

[Android O] Launch SingleWebsitePreferences from OS channel settings

- All notification channel settings in the OS in Android O have an
 'Additional settings in the app' button at the bottom.

- Previously this button always linked to our top-level notification
permissions screen within Site Settings.

- Now we check the CHANNEL_ID extra and open the appropriate
SingleWebsitePreferences screen, if this came from a channel
associated with a particular origin. (Otherwise we continue to open
the top level notifications screen).

- Note that per-origin channels are currently only available if the
SiteNotificationChannels feature flag is enabled.

Bug:  700377 
Change-Id: I0c18f375d7e2d515c633fa06bafb0190c321fe3b
Reviewed-on: https://chromium-review.googlesource.com/543122
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481506}
[modify] https://crrev.com/78bc677c65488c2aff3da2d91e4322dfc32c6301/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
[modify] https://crrev.com/78bc677c65488c2aff3da2d91e4322dfc32c6301/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/SiteChannelsManager.java
[modify] https://crrev.com/78bc677c65488c2aff3da2d91e4322dfc32c6301/chrome/android/junit/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridgeUnitTest.java

Project Member

Comment 18 by bugdroid1@chromium.org, Jun 22 2017

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

commit fdf9b7412cc52402456000ba9553eb4064c8d096
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Jun 22 17:39:49 2017

[Android Notification Channels] Delete channels on ClearAllCSRules

- This patch implements the ClearAllContentSettingsRules method for the
new NotificationChannelsProviderAndroid.

- This ensures all channels are deleted whenever all notification
content settings are cleared, e.g. when clearing site settings data.

- Further work is required to make sure recently-created channels are
deleted when recent site settings are cleared.

Bug:  700377 
Change-Id: I317b36de010209360de97633f78a7542942636ce
Reviewed-on: https://chromium-review.googlesource.com/544983
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481585}
[modify] https://crrev.com/fdf9b7412cc52402456000ba9553eb4064c8d096/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/fdf9b7412cc52402456000ba9553eb4064c8d096/chrome/browser/notifications/notification_channels_provider_android.h
[modify] https://crrev.com/fdf9b7412cc52402456000ba9553eb4064c8d096/chrome/browser/notifications/notification_channels_provider_android_unittest.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Jul 10 2017

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

commit f29791467fd8d6906cee7963ef29c73b7f88e321
Author: Anita Woodruff <awdf@chromium.org>
Date: Mon Jul 10 17:58:53 2017

[Android Unit Tests] Switch out mock channels bridge for a fake one

- Previously was using GMock to mock the bridge class that forwards
jni calls when creating/deleting/checking site notification channels.

- Now there is a very simple fake implementation using an in-memory
map from channel origin to NotificationChannel.

- This enables more readable tests and will make it easier to write
tests in future as the implementation develops.

Bug:  700377 
Change-Id: I958758c653ca3d83df3e95ad721f66c990fe03ea
Reviewed-on: https://chromium-review.googlesource.com/565513
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485310}
[modify] https://crrev.com/f29791467fd8d6906cee7963ef29c73b7f88e321/chrome/browser/notifications/notification_channels_provider_android_unittest.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Jul 20 2017

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

commit 10dc6063aaf8fe414ab9338ef556e08e67f2884a
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Jul 20 11:34:53 2017

[Android Notifications] Use timestamp in notification channel IDs

- Implement GetWebsiteSettingLastModified for the
NotificationChannelsAndroidProvider by storing channel creation
timestamp within the channel IDs.

- This also has the beneficial side effect that each time a channel
is created in response to permission grant, it will have a unique ID,
so that channels deleted when site data is cleared will not be
restored the next time permission is granted/blocked.

- Note that NCAP.GetWebsiteSettingLastModified is currently only
called from tests, but in a future patch it will be pulled up to a
new provider interface and called by the HostContentSettingsMap
where it currently only calls the Pref Provider. 

Bug:  700377 
Change-Id: I5363ae2baa7e0c92bb7768361d459cd024622b32
Reviewed-on: https://chromium-review.googlesource.com/567153
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488198}
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationSettingsBridge.java
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelDefinitions.java
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelsInitializer.java
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/SiteChannelsManager.java
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/channels/ChannelsInitializerTest.java
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/channels/SiteChannelsManagerTest.java
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/android/junit/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridgeUnitTest.java
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/browser/notifications/notification_channels_provider_android.h
[modify] https://crrev.com/10dc6063aaf8fe414ab9338ef556e08e67f2884a/chrome/browser/notifications/notification_channels_provider_android_unittest.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Aug 16 2017

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

commit 09503699783ba50bb017f498dbc7972f20744623
Author: Anita Woodruff <awdf@chromium.org>
Date: Wed Aug 16 15:15:11 2017

[Android Notifications] Introduce UserModifiableProvider interface

- This patch pulls up the GetWebsiteSettingLastModified method into
a new interface, implemented by the PrefProvider and the
NotificationChannelsAndroidProvider.

- Where previously the HostContentSettingsMap only queried the
PrefProvider for getting the last-modified date of a content setting
or clearing recently-modified content settings, now it goes through
all the UserModifiableProviders.

Bug:  700377 
Change-Id: Iba8c6dba82882ca29dc56b0a6cb329a4ebe8579d
Reviewed-on: https://chromium-review.googlesource.com/577867
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494788}
[modify] https://crrev.com/09503699783ba50bb017f498dbc7972f20744623/chrome/browser/content_settings/host_content_settings_map_factory.cc
[modify] https://crrev.com/09503699783ba50bb017f498dbc7972f20744623/chrome/browser/content_settings/host_content_settings_map_unittest.cc
[modify] https://crrev.com/09503699783ba50bb017f498dbc7972f20744623/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/09503699783ba50bb017f498dbc7972f20744623/chrome/browser/notifications/notification_channels_provider_android.h
[modify] https://crrev.com/09503699783ba50bb017f498dbc7972f20744623/components/content_settings/core/browser/BUILD.gn
[modify] https://crrev.com/09503699783ba50bb017f498dbc7972f20744623/components/content_settings/core/browser/content_settings_pref_provider.h
[modify] https://crrev.com/09503699783ba50bb017f498dbc7972f20744623/components/content_settings/core/browser/host_content_settings_map.cc
[modify] https://crrev.com/09503699783ba50bb017f498dbc7972f20744623/components/content_settings/core/browser/host_content_settings_map.h
[add] https://crrev.com/09503699783ba50bb017f498dbc7972f20744623/components/content_settings/core/browser/user_modifiable_provider.h

Project Member

Comment 22 by bugdroid1@chromium.org, Aug 16 2017

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

commit 243f6992645fefb89539a067a0dd09b4ba5c385b
Author: Anita Woodruff <awdf@chromium.org>
Date: Wed Aug 16 19:17:26 2017

[Android Notification Channels] Only retrieve channel ID on O+

- There is no point calling getChannelIdForOrigin on versions of the
platform which don't support channels.

Bug:  700377 
Change-Id: I1ac9b15378b62ff0b738e549b1f9e3316a1c20cc
Reviewed-on: https://chromium-review.googlesource.com/617420
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#494891}
[modify] https://crrev.com/243f6992645fefb89539a067a0dd09b4ba5c385b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java

Project Member

Comment 23 by bugdroid1@chromium.org, Aug 18 2017

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

commit 59ec2e23fc0d29b0883628d7cc3308a0c671fe1e
Author: Anita Woodruff <awdf@chromium.org>
Date: Fri Aug 18 15:33:55 2017

[Tests] Allow MockProvider to store >1 content setting

- Originally the mock provider had a very simple implementation where
it only stored a single value, so its documented behaviour of
SetWebsiteSetting overriding previous settings made some sense.

- Now that it wraps an OriginIdentifierValueMap there is no point in
preserving this limitation.

Bug:  700377 
Change-Id: I9521cb84cf3b2f3c0721d125aa3250a8afb1f3d0
Reviewed-on: https://chromium-review.googlesource.com/620751
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495552}
[modify] https://crrev.com/59ec2e23fc0d29b0883628d7cc3308a0c671fe1e/components/content_settings/core/test/content_settings_mock_provider.cc
[modify] https://crrev.com/59ec2e23fc0d29b0883628d7cc3308a0c671fe1e/components/content_settings/core/test/content_settings_mock_provider.h

Project Member

Comment 24 by bugdroid1@chromium.org, Aug 22 2017

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

commit da0cdb648cf1f2fec4781400410ff4043bb9adf1
Author: Anita Woodruff <awdf@chromium.org>
Date: Tue Aug 22 18:36:50 2017

[Android Notification Channels] Migrate to site channels on upgrade

- Migrates any pre-existing notification permissions to channels
if we detect channels should be used on profile creation.

- This transfers ownership of the site settings previously
stored in the pref provider to the notification channnels provider.

- This means any existing user allow/block permissions for particular
sites will not be lost on upgrade to Android O, or on upgrade of
Chrome to a version supporting site channels. (Note, site channels
are still behind a feature flag).

Bug:  700377 
Change-Id: I3b8ecbb0690a37bec7c569c27ae95a312bd66290
Reviewed-on: https://chromium-review.googlesource.com/619154
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496380}
[modify] https://crrev.com/da0cdb648cf1f2fec4781400410ff4043bb9adf1/chrome/browser/content_settings/host_content_settings_map_factory.cc
[modify] https://crrev.com/da0cdb648cf1f2fec4781400410ff4043bb9adf1/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/da0cdb648cf1f2fec4781400410ff4043bb9adf1/chrome/browser/notifications/notification_channels_provider_android.h
[modify] https://crrev.com/da0cdb648cf1f2fec4781400410ff4043bb9adf1/chrome/browser/notifications/notification_channels_provider_android_unittest.cc
[modify] https://crrev.com/da0cdb648cf1f2fec4781400410ff4043bb9adf1/chrome/browser/prefs/browser_prefs.cc
[modify] https://crrev.com/da0cdb648cf1f2fec4781400410ff4043bb9adf1/chrome/common/pref_names.cc
[modify] https://crrev.com/da0cdb648cf1f2fec4781400410ff4043bb9adf1/chrome/common/pref_names.h
[modify] https://crrev.com/da0cdb648cf1f2fec4781400410ff4043bb9adf1/components/content_settings/core/browser/host_content_settings_map.h

Project Member

Comment 25 by bugdroid1@chromium.org, Aug 30 2017

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

commit 06a6954fe08b6d4adce14a4e256c6499f197b1e7
Author: Anita Woodruff <awdf@chromium.org>
Date: Wed Aug 30 17:27:23 2017

[Android Notifications] Unmigrate site channels if flag gets disabled

- If the kSiteNotificationChannels flag gets disabled after
notification content settings were migrated to channels, we will now
'unmigrate' the settings back to the pref provider. This ensures that:

1. We don't lose user preferences for users who toggled site channels
while the flag was enabled.

2. Site channels disappear from the settings when the flag is disabled.

Bug:  700377 
Change-Id: I32da9046139bf7776cd786fa954a3b3d50c7c7ba
Reviewed-on: https://chromium-review.googlesource.com/628680
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498521}
[modify] https://crrev.com/06a6954fe08b6d4adce14a4e256c6499f197b1e7/chrome/browser/content_settings/host_content_settings_map_factory.cc
[modify] https://crrev.com/06a6954fe08b6d4adce14a4e256c6499f197b1e7/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/06a6954fe08b6d4adce14a4e256c6499f197b1e7/chrome/browser/notifications/notification_channels_provider_android.h
[modify] https://crrev.com/06a6954fe08b6d4adce14a4e256c6499f197b1e7/chrome/browser/notifications/notification_channels_provider_android_unittest.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Aug 30 2017

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

commit caea8a624d066a8f44357ef9d6eca25a7e790956
Author: Anita Woodruff <awdf@chromium.org>
Date: Wed Aug 30 18:52:00 2017

[Android Notification Channels] Enable site channels by default

- This enables the kSiteNotificationChannels feature flag by default.

- When this flag is enabled we use separate notification channels on
Android O+ for notifications from different origins (instead of
sending them all to the Sites channel).

Bug:  700377 
Change-Id: I44e0b30ffa0c6bd24419b2504a1e2ac91b4cd5e6
Reviewed-on: https://chromium-review.googlesource.com/626298
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498552}
[modify] https://crrev.com/caea8a624d066a8f44357ef9d6eca25a7e790956/chrome/browser/content_settings/host_content_settings_map_unittest.cc
[modify] https://crrev.com/caea8a624d066a8f44357ef9d6eca25a7e790956/chrome/common/chrome_features.cc
[modify] https://crrev.com/caea8a624d066a8f44357ef9d6eca25a7e790956/components/content_settings/core/browser/host_content_settings_map.cc
[modify] https://crrev.com/caea8a624d066a8f44357ef9d6eca25a7e790956/components/content_settings/core/browser/host_content_settings_map.h

Project Member

Comment 27 by bugdroid1@chromium.org, Aug 30 2017

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

commit 04b0b92cb15537006252db313219c806d187fe9b
Author: Anita Woodruff <awdf@chromium.org>
Date: Wed Aug 30 18:52:37 2017

[Android Notification Channels] Small caching fixes in channels provider

- Clear cache when deleting channels on unmigrate

- Call InitCachedChannels before calling CreateChannelIfRequired via
CreateChannelForRule

Bug:  700377 
Change-Id: I4ff233fd99263de0bf25da71c07c878a774385a4
Reviewed-on: https://chromium-review.googlesource.com/643213
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498553}
[modify] https://crrev.com/04b0b92cb15537006252db313219c806d187fe9b/chrome/browser/notifications/notification_channels_provider_android.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Aug 31 2017

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

commit 492201f38f1a8dfe789ba3018fce206dfa5bbdb7
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Aug 31 10:49:28 2017

Rename should_use_channels_ -> platform_supports_channels_

- platform_supports_channels_ is a more accurate name for this variable.

Bug:  700377 
Change-Id: I4b9780c6482476d8217d1ffc3e87096aae833d3e
Reviewed-on: https://chromium-review.googlesource.com/632681
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498809}
[modify] https://crrev.com/492201f38f1a8dfe789ba3018fce206dfa5bbdb7/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/492201f38f1a8dfe789ba3018fce206dfa5bbdb7/chrome/browser/notifications/notification_channels_provider_android.h

Project Member

Comment 29 by bugdroid1@chromium.org, Aug 31 2017

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

commit b7b0206e1449f03d00a87fe8a3c25afea6a66f64
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Aug 31 12:40:16 2017

[Android Tests] Fixup channel test on Android O

- Previously testInitializeStartupChannels_groupCreated
 sometimes failed locally because a Sites channel group was 
sometimes present as well as the expected General group.

- This happened because the Sites group was leftover
from previous manual testing on the device.

- Now any existing channel groups are deleted in setUp so
only the expected General channel is created.

Bug:  700377 
Change-Id: Ibb20ef6e48b51d96849054db62b8901b0f91af1e
Reviewed-on: https://chromium-review.googlesource.com/645687
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498830}
[modify] https://crrev.com/b7b0206e1449f03d00a87fe8a3c25afea6a66f64/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationManagerProxy.java
[modify] https://crrev.com/b7b0206e1449f03d00a87fe8a3c25afea6a66f64/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationManagerProxyImpl.java
[modify] https://crrev.com/b7b0206e1449f03d00a87fe8a3c25afea6a66f64/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/channels/ChannelsInitializerTest.java
[modify] https://crrev.com/b7b0206e1449f03d00a87fe8a3c25afea6a66f64/chrome/test/android/javatests/src/org/chromium/chrome/test/util/browser/notifications/MockNotificationManagerProxy.java

Project Member

Comment 30 by bugdroid1@chromium.org, Aug 31 2017

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

commit 71bf8d1f5e6097102ce735cceb60e21802f6ef42
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Aug 31 14:36:08 2017

[Android Notifications] Fix NPE in NotificationUmaTracker

- Previously we always tried to check whether the sites channel
was enabled for site notifications in the NotificationUmaTracker. This
means we get a NPE since the site notifications channel was deprecated.

- Now we will be checking the actual site channel, but still reporting
this as a 'Sites' notification type in UMA.

- Also added an extra nullcheck just in case of race conditions
between deleting channels and sending UMA.

Bug:  700377 
Change-Id: I7112fd6dd7ec50619d2d93a5863a310de90ecfc4
Reviewed-on: https://chromium-review.googlesource.com/645851
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498858}
[modify] https://crrev.com/71bf8d1f5e6097102ce735cceb60e21802f6ef42/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationPlatformBridge.java
[modify] https://crrev.com/71bf8d1f5e6097102ce735cceb60e21802f6ef42/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationUmaTracker.java

Project Member

Comment 31 by bugdroid1@chromium.org, Aug 31 2017

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

commit 3c140a2de8531073a5ec40104eaf5e74759e8660
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Aug 31 16:44:44 2017

[Android Notifications] Deprecate the Sites channel when flag enabled

- Previously the Sites channel would always be created on first launch
on Android O.

- Now it is only created if/when a notification is posted to it. Note
that when the SiteNotificationChannels flag is enabled, we no longer
post notifications to the Sites channel, because they are posted to
site-specific dynamically-created channels instead. So we expect users
with this flag enabled never to see a Sites channel.

- Also, when the SiteNotificationChannels flag is enabled, the old
Sites channel will now be deleted on first launch.

Bug:  700377 
Change-Id: I2ac4970da4ea4492bd21ac0120f89d012b45eae8
Reviewed-on: https://chromium-review.googlesource.com/645966
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498889}
[modify] https://crrev.com/3c140a2de8531073a5ec40104eaf5e74759e8660/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelDefinitions.java
[modify] https://crrev.com/3c140a2de8531073a5ec40104eaf5e74759e8660/chrome/android/javatests/src/org/chromium/chrome/browser/notifications/channels/ChannelsInitializerTest.java
[modify] https://crrev.com/3c140a2de8531073a5ec40104eaf5e74759e8660/chrome/android/junit/src/org/chromium/chrome/browser/notifications/channels/ChannelDefinitionsTest.java

Project Member

Comment 32 by bugdroid1@chromium.org, Aug 31 2017

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

commit 7d1a25a91542548b4e8c2e8ec58b489cb9042f2c
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Aug 31 17:38:12 2017

[Android Notification Channels] Bump channels version

- This is a follow-up to crrev.com/c/645966 - since that commit
removed the Sites channel from the list of start-up channels, it should
also have bumped the channels version number so that updaters pick up
the change.

Bug:  700377 
Change-Id: I7a449fdf4648e4438170a8e6fcba3aeeb02f9c81
Reviewed-on: https://chromium-review.googlesource.com/645982
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498912}
[modify] https://crrev.com/7d1a25a91542548b4e8c2e8ec58b489cb9042f2c/chrome/android/java/src/org/chromium/chrome/browser/notifications/channels/ChannelDefinitions.java

Project Member

Comment 33 by bugdroid1@chromium.org, Sep 6 2017

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

commit 4f73440bce9a4eec157a550d905698a2447a43e7
Author: Anita Woodruff <awdf@chromium.org>
Date: Wed Sep 06 12:35:43 2017

[Android Settings] Update notification preference on navigation

- Previously, the Allow/Block status of a site's notification
permission in site settings would display an out-of-date status if
the user tapped on it, toggled it via channel settings (on O), and
then navigated back to site settings immediately.

- Now, we actively refresh the notification permission when we detect
that the user is returning from channel settings, so the correct
permission is always displayed.

Bug:  700377 
Change-Id: I62b0f16659977928487657149c3ff36bc72796f5
Reviewed-on: https://chromium-review.googlesource.com/650409
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499940}
[modify] https://crrev.com/4f73440bce9a4eec157a550d905698a2447a43e7/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java

Comment 34 by awdf@chromium.org, Sep 11 2017

Status: Fixed (was: Started)
Project Member

Comment 35 by bugdroid1@chromium.org, Nov 20 2017

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

commit 25960b4328f9a1e69da9c499742dba4c83b1bb21
Author: Anita Woodruff <awdf@chromium.org>
Date: Mon Nov 20 15:57:14 2017

[Android Notification Channels] Remove TODO

- I no longer think this TODO to remove the DCHECK is necessary:
 It's appropriate to DCHECK since we should only ever create a channel
if the old status is UNAVAILABLE, or the same as the new status (to
handle the possibility that two code paths try to create a channel at
the same time, e.g. during channel migration).

Bug:  700377 
Change-Id: I0554d2962051863a6c38a9f9ccb262f905f6a241
Reviewed-on: https://chromium-review.googlesource.com/544842
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517834}
[modify] https://crrev.com/25960b4328f9a1e69da9c499742dba4c83b1bb21/chrome/browser/notifications/notification_channels_provider_android.cc

Sign in to add a comment