Unable to reproduce the issue in Android. 

Steps Followed:

1. Launched the Chrome Browser.
2. Navigated to the example website provided 'https://goroost.com/try-web-push' and website displayed in the screenshot 'https://talkrrr.rejh.nl'.
3. Tapped on the "Allow" button when requested by the website.
4. Navigated to the Chrome > Settings > Site settings.
5. Clicked on Notifications.

Observations: 
1. Websites mentioned in Step-2 are listed.
2. If the notifications are switched of from Chrome > Settings > Site settings > Notifications, then the mentioned website is displayed under Blocked list. And, notifications are not displayed when tried to access the sample websites provided.

Chrome versions tested:
62.0.3202.84

OS
Android 8.1.0

Android Devices
Pixel XL Build/OPM1.171019.005

@chemo.b -- Thanks for reporting this issue. Could you please let us know if we have missed anything or could you please provide the screencast reproducing this issue and the details of the device used. This will help us in further triaging the issue.

Thanks in advance!

@pnangunoori Did you install Talkrrr on the home screen (actually, it's installed as an enhanced home screen app, as in: it shows up in my app drawer).

I'll try and make a screencast this evening (European time :))

Thanks so far for checking it out!

Thank you for providing more feedback. Adding requester "pnangunoori@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

@chemo.b-- Yes application is installed. And I was able to send message in Public Chat within the application after logging in.

Thanks!

@pnangunoori Here's the screencast, as promised:
https://stor4ge.rejh.nl/_stored/dump/videos/chrome-cant-revoke-notif-permission.mp4

Whatever I do, the notification permission just keeps saying "Allowed"

Thank you for providing more feedback. Adding requester "pnangunoori@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Unable to reproduce the issue in Android. 

Steps Followed:

1. Launched the Chrome Browser.
2. Navigated to the website provided 'https://talkrrr.rejh.nl'.
3. Logged into the website by using Gmail credentials.
4. Tap on the "Allow" button in the Notifications bubble when requested by the website.
5. Navigated to the Chrome > Settings.
6. Tap on Notifications.
7. Tap on "Additional settings in the app"
8. Tap on the respective website.
9. Tap on "CLEAR & RESET" button.
10. Tap on "CLEAR & RESET" button in the pop-up displayed.

Observations: 
1. Observe that website is not displayed in the Notifications. 
2. User is also able to clear notifications from Chrome > Settings > Site settings > Notifications.

Chrome versions tested:
62.0.3202.84

OS:
Android 8.1.0 & Android 8.0.0

Android Devices
Pixel XL Build/OPM1.171019.005 & Pixel XL Build/OPR3.170623.008

@chemo.b -- Thanks for providing the screencast. Could you please let us know the device details used by you to reproduce the issue. This would help us in further triaging the issue.

Thanks in advance!

@pnangunoori Device: Pixel (2016), Android 8.0 (Oct patch).

I don't see "add to home screen" in the steps you followed. I just checked the chrome://flags page and can't find the "enhanced add to home screen" option anymore (maybe looking for the wrong name?) so I assume this is default behavior now? I think I did add the PWA to the home screen with that flag enabled when it was still there.

I'd have to try it without adding it to the home screen (like your steps describe), see if that fixes it.

Thank you for providing more feedback. Adding requester "pnangunoori@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

I'm not able to reproduce this either on a Pixel 2 XL running 8.1, but the video in #c6 is quite clear on where the problem occurs.

In short, notification permission in Android O is delegated to notification channels instead of our own data store. We don't get informed about when the status of a channel changes, so in a number of places we opportunistically query channel status. The Site Settings page is one of those places.

What seems to be happening here is that the channel, and thereby the permission, is in fact deleted, but that the Site Settings page isn't querying for updated permission status at the right moment. This causes it to show outdated information.

I thought we did this as part of OnResume() on the View. Should we somehow force an update after the [reset] button navigates back to it?

I can't repro either. We do requery in SingleCategoryPreferences#onResume so this is v odd. 

I note in the screencast there is never a Site Channel in Android settings for the web app. 

chemo.b@ - Can the site actually send notifications now that you are in the state seen in the screencast? I'm wondering like Peter if the permission was actually revoked under the hood, in which case notifications should not show up. 

That button ends up in DesktopNotificationProfileUtil::ClearSetting() which eventually calls HostContentSettingsMap::SetWebsiteSettingCustomScope(). It iterates over the (prioritised) content setting providers until it finds one that returns true for SetWebsiteSetting().

The NotificationChannelsProviderAndroid::SetWebsiteSetting() implementation returns TRUE when requesting to delete settings even when they don't exist.

Is it possible that their device somehow has a left-over content setting in preferences that's not represented as a channel? That'd explain the behaviour as it would be included when getting the rule iterator (which the settings' page display is based on), but we wouldn't implement deleting it later on because NCPA::SetWebsiteSetting() returns TRUE.

Two questions:

1. Should NCPA::SetWebsiteSetting() return FALSE when the channel that's requested to be deleted does not exist?
2. Why would there be a stray content setting in the pref provider? Is our migration code incomplete?

#c13: Yes, notifications are still showing up. I don't get asked for granting permission.

I will take a look at this tomorrow and report back on this bug.

@raymes - Meanwhile let us know if anything obvious jumps out to you.

My current suspicion is this may be an upgrade-path issue from permission being granted pre-Chrome-62 in Content Settings.

Good questions in #14. I'm not sure.

Also I was wondering whether having "Enhanced Add to Homescreen" flag enabled has any impact here but after watching the video, that might just be a red-herring. 

Ok I have a repro, and it *is* related to migrating site settings -> channels in Chrome 62:

1. Grant permission to a site on Chrome 61

2. Upgrade to Chrome 62
 (Note a channel has been created for the site as expected)

3. Clear & Reset the notification permission
 (Note channel has been deleted as expected)

4. Go to 'Additional settings in the app' or Chrome > Settings > Site Settings 

The site is still listed as allowed (BUG).

I know why this is happening - it's because we are not wiping the pref provider during the migration, we're just copying the permissions into the channels provider, and then we query all providers to build the list of sites with notification permission displayed in Chrome settings.

What I'm less clear on is why the site can still send notifications - I thought the channels provider should return ASK and no other providers should be queried.

I'll look into this next and report back later today.

Ah, my bad, the channels provider should never return ASK. Instead we rely on the default provider to return that, further down the precedence chain of providers.

BUT, since we don't clear the pref provider on migrate, and it continues to have precedence over the default provider, the pref provider's value is used instead.

It's clear we should be deleting the settings from the pref provider on migrate, less so what to do about users who have got into this state.

I like Peter's suggestion of returning false from SetWebsiteSetting(ASK) in the channels provider, so that way at least if users try to reset again once we've done that then the rogue content setting will be cleared from the pref provider. (Note the channel will be recreated on the first notification after attempting to clear the site's permission, so we'd need to return false regardless of whether the channel exists to capture all cases).

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

commit 3ecbacc4b37d37a19b2f1ece0bd72e983f836d26
Author: Anita Woodruff <awdf@chromium.org>
Date: Fri Nov 10 12:20:50 2017

[Android O] Clear notification permission from PrefProvider on Reset

- Previously when users on O pressed the 'Clear and Reset' button to
clear notification settings for a site, the permission could remain
granted if it was granted on an older version of the browser, due to
a bug in the migration of settings to notification channels.

- This was because settings were never deleted from the PrefProvider
either during the migration process or on clearing notification
permission for a site.

- Now we return false from NCPA::SetWebsiteSetting when clearing
permission.

- This means the HostContentSettingsMap continues to iterate through the
content setting providers and clears the permission from the PrefProvider
too if present.

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

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

commit 7860090b10225bc21f1141bf84795ef96cd07c01
Author: Anita Woodruff <awdf@chromium.org>
Date: Fri Nov 10 13:08:20 2017

[Android O] Clean up notification settings on channel migration

- Previously when migrating to channels, the content settings were
simply copied over to the NotificationChannelsProviderAndroid from
the PrefProvider.

- Now they are deleted from the PrefProvider as they are copied over
to the NCPA.

- This avoids old content settings hanging around and getting
reinstated on channel deletion.

Bug:  781524 
Change-Id: Ic00ddd4996e8a17775678193b6ffdb0b4d62d5fa
Reviewed-on: https://chromium-review.googlesource.com/760936
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515538}
[modify] https://crrev.com/7860090b10225bc21f1141bf84795ef96cd07c01/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/7860090b10225bc21f1141bf84795ef96cd07c01/chrome/browser/notifications/notification_channels_provider_android_unittest.cc
[modify] https://crrev.com/7860090b10225bc21f1141bf84795ef96cd07c01/components/content_settings/core/test/content_settings_mock_provider.cc

As a result of this bug we have now triggered the kill-switch for the Site-channels feature in M62. This means Oreo users on 62 will no longer see separate notification channels for each site with notification permission. Instead, they will see a single 'Sites' channel and their notification settings will be entirely managed within Chrome settings again, allowing them to successfully revoke permissions with the 'Clear and Reset' button.

Note the kill switch has only been activated for 62. The feature is still enabled in 63 with the bug at present.

Requesting merge of #21 for 63 to fix the bug. It is a one-line change which will ensure the 'Clear and Reset' button allows Oreo users to revoke notification permission while still allowing the 'Site notifications' feature to be enabled.

This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Please verify the fix in Canary

Merge approved upon verification of the fix in canary.

Thanks.

Attempted to verify in Canary but discovered a failing DCHECK, I believe caused by the commit in #22 (NOT the one I want to cherry-pick, but they went into the same version of Canary). Cl to revert #22 in-flight here: https://chromium-review.googlesource.com/c/chromium/src/+/768787 Tracking fix for the DCHECK at  Issue 784823 

I did check the fix in #21 extensively on ToT before I submitted it, but will need to wait for the aforementioned revert to land before I can verify in Canary.

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

commit 2aac74d3581683516ed149948903875b741c125a
Author: Anita Woodruff <awdf@chromium.org>
Date: Tue Nov 14 14:44:33 2017

Revert "[Android O] Clean up notification settings on channel migration"

This reverts commit 7860090b10225bc21f1141bf84795ef96cd07c01.

Reason for revert: Likely cause of ANR / DCHECK

Original change's description:
> [Android O] Clean up notification settings on channel migration
> 
> - Previously when migrating to channels, the content settings were
> simply copied over to the NotificationChannelsProviderAndroid from
> the PrefProvider.
> 
> - Now they are deleted from the PrefProvider as they are copied over
> to the NCPA.
> 
> - This avoids old content settings hanging around and getting
> reinstated on channel deletion.
> 
> Bug:  781524 
> Change-Id: Ic00ddd4996e8a17775678193b6ffdb0b4d62d5fa
> Reviewed-on: https://chromium-review.googlesource.com/760936
> Reviewed-by: Raymes Khoury <raymes@chromium.org>
> Reviewed-by: Peter Beverloo <peter@chromium.org>
> Commit-Queue: Anita Woodruff <awdf@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#515538}

TBR=raymes@chromium.org,peter@chromium.org,awdf@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  781524 , 784823 
Change-Id: I53d19029118bbd0c907ad1317f744e3e6e0a80c3
Reviewed-on: https://chromium-review.googlesource.com/768787
Reviewed-by: Anita Woodruff <awdf@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516297}
[modify] https://crrev.com/2aac74d3581683516ed149948903875b741c125a/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/2aac74d3581683516ed149948903875b741c125a/chrome/browser/notifications/notification_channels_provider_android_unittest.cc
[modify] https://crrev.com/2aac74d3581683516ed149948903875b741c125a/components/content_settings/core/test/content_settings_mock_provider.cc

Revert landed in 64.0.3269.0. Tested fix using this version of Canary on Oreo and toggling the feature flag to trigger the migration logic:

0. Disable SiteNotificationChannels feature (to emulate Chrome versions pre-62):
  $ build/android/adb_chrome_public_command_line --disable-features=SiteNotificationChannels

1. Grant notification permission to tests.peter.sh/notification-generator/
  (Note its notifications are sent to the 'Sites' channel)

2. Force-kill Chrome and enable SiteNotificationChannels feature (to emulate updating to 62+):
  $ build/android/adb_chrome_public_command_line --enable-features=SiteNotificationChannels
  (Note notifications are now sent to a 'tests.peter.sh' channel)

3. Clear & Reset the notification permission

4. Go to 'Additional settings in the app' or Chrome > Settings > Site Settings 

The site is no longer listed, and triggers a permission prompt on navigating to the site.

-----------

Will proceed with the merge as per #26.

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

commit 5081f8d1b0ce2ae7193702731869697fd7c67cd2
Author: Anita Woodruff <awdf@chromium.org>
Date: Wed Nov 15 12:26:19 2017

[Android O] Clear notification permission from PrefProvider on Reset

- Previously when users on O pressed the 'Clear and Reset' button to
clear notification settings for a site, the permission could remain
granted if it was granted on an older version of the browser, due to
a bug in the migration of settings to notification channels.

- This was because settings were never deleted from the PrefProvider
either during the migration process or on clearing notification
permission for a site.

- Now we return false from NCPA::SetWebsiteSetting when clearing
permission.

- This means the HostContentSettingsMap continues to iterate through the
content setting providers and clears the permission from the PrefProvider
too if present.

Bug:  781524 
Change-Id: I883ee6ab0b70873c4157b545aeac3691bfb8a23f
Reviewed-on: https://chromium-review.googlesource.com/758687
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#515532}(cherry picked from commit 3ecbacc4b37d37a19b2f1ece0bd72e983f836d26)
Reviewed-on: https://chromium-review.googlesource.com/771750
Reviewed-by: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#503}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/5081f8d1b0ce2ae7193702731869697fd7c67cd2/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/5081f8d1b0ce2ae7193702731869697fd7c67cd2/chrome/browser/notifications/notification_channels_provider_android_unittest.cc

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

commit 138706e38fcd103ba462dbbc13785235a591c4f3
Author: Anita Woodruff <awdf@chromium.org>
Date: Thu Nov 16 13:25:52 2017

[Android O] Clean up notification settings on channel migration

- Previously when migrating to channels, the content settings were
simply copied over to the NotificationChannelsProviderAndroid from
the PrefProvider.

- Now they are deleted from the PrefProvider after they are copied over
to the NCPA.

- This avoids old content settings hanging around and getting
reinstated on channel deletion.

Bug:  781524 , 784823 
Change-Id: I01ec7974831f6a2be192c7e6e2a587148e736b9e
Reviewed-on: https://chromium-review.googlesource.com/768722
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517071}
[modify] https://crrev.com/138706e38fcd103ba462dbbc13785235a591c4f3/chrome/browser/notifications/notification_channels_provider_android.cc
[modify] https://crrev.com/138706e38fcd103ba462dbbc13785235a591c4f3/chrome/browser/notifications/notification_channels_provider_android_unittest.cc
[modify] https://crrev.com/138706e38fcd103ba462dbbc13785235a591c4f3/components/content_settings/core/test/content_settings_mock_provider.cc