Issue 628058 link

Starred by 3 users

Issue metadata

Status:	Fixed
Owner:	█ raymes@chromium.org OOO until 4th Feb
Closed:	Nov 2016
Components:	Internals>Permissions Internals>Permissions>Model
EstimatedDays:	----
NextAction:	----
OS:	All
Pri:	3
Type:	Bug

Team-Security-UX

Remove the push messaging content setting

Project Member Reported by raymes@chromium.org, Jul 14 2016

Issue description

Roughly 2 years ago we coalesced these two settings, so that the user just answered one prompt for both notifications and push notifications. The PUSH_MESSAGING content setting is now only used in the following circumstance:
If it has previously been disabled for a given origin, that origin will not have access to push notifications.

However, the PUSH_MESSAGING content setting will never be set by any current code in Chrome.

There are several issues with the PUSH_MESSAGING content setting:
There is no way for a user to view or change existing settings to CONTENT_SETTINGS_TYPE_PUSH_MESSAGING. This has privacy consequences. 
It is very hard/impossible for a user to reason about how their PUSH_MESSAGING content setting has an impact on them, if it is set.
Keeping this content setting increases the complexity of the code and is a maintenance burden.

Project Member

Comment 1 by bugdroid1@chromium.org, Jul 19 2016

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

commit 4f82b403f397fbad2e44fe287c05956fcb7cf282
Author: raymes <raymes@chromium.org>
Date: Tue Jul 19 07:04:37 2016

Use the same codepath for NOTIFICATIONS and PUSH_MESSAGING permissions

We used to have a complicated dependency between the notifications and push
messaging permissions and content settings. The push messaging content setting
will no longer be used (the setting will be deleted in a follow up CL). Since
the permissions are essentially the same now, we reuse the same
PermissionContext class for both, with some very slight behavioral differences
for push messaging.

Behaviorally, this change will mean that if a user happened to have
CONTENT_SETTINGS_TYPE_PUSH_MESSAGING set to block for a particular origin,
they will fall back to their CONTENT_SETTINGS_TYPE_NOTIFICATIONS setting for
that origin. Note that it's very unlikely that any users do have
CONTENT_SETTINGS_TYPE_PUSH_MESSAGING set, as it seems like this setting
was never actually exposed to users on stable. More details can be found here:
https://docs.google.com/document/d/1-Vny4paBx6gFZuCeLX2Gq9ztkF-JhNYM_uqvrQDj-6M/edit#heading=h.h99sppup43dn

BUG= 628058 

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

[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/app/generated_resources.grd
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/notifications/notification_permission_context.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/notifications/notification_permission_context.h
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/notifications/notification_permission_context_unittest.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/permissions/permission_bubble_request_impl.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/permissions/permission_manager.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/push_messaging/push_messaging_browsertest.cc
[delete] https://crrev.com/4e4d53c69188102853c17db8c1d6041f3559a9f2/chrome/browser/push_messaging/push_messaging_permission_context.cc
[delete] https://crrev.com/4e4d53c69188102853c17db8c1d6041f3559a9f2/chrome/browser/push_messaging/push_messaging_permission_context.h
[delete] https://crrev.com/4e4d53c69188102853c17db8c1d6041f3559a9f2/chrome/browser/push_messaging/push_messaging_permission_context_unittest.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/push_messaging/push_messaging_service_unittest.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/chrome_browser.gypi
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/chrome_tests_unit.gypi

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 19 2016

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

commit 4f82b403f397fbad2e44fe287c05956fcb7cf282
Author: raymes <raymes@chromium.org>
Date: Tue Jul 19 07:04:37 2016

Use the same codepath for NOTIFICATIONS and PUSH_MESSAGING permissions

We used to have a complicated dependency between the notifications and push
messaging permissions and content settings. The push messaging content setting
will no longer be used (the setting will be deleted in a follow up CL). Since
the permissions are essentially the same now, we reuse the same
PermissionContext class for both, with some very slight behavioral differences
for push messaging.

Behaviorally, this change will mean that if a user happened to have
CONTENT_SETTINGS_TYPE_PUSH_MESSAGING set to block for a particular origin,
they will fall back to their CONTENT_SETTINGS_TYPE_NOTIFICATIONS setting for
that origin. Note that it's very unlikely that any users do have
CONTENT_SETTINGS_TYPE_PUSH_MESSAGING set, as it seems like this setting
was never actually exposed to users on stable. More details can be found here:
https://docs.google.com/document/d/1-Vny4paBx6gFZuCeLX2Gq9ztkF-JhNYM_uqvrQDj-6M/edit#heading=h.h99sppup43dn

BUG= 628058 

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

[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/app/generated_resources.grd
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/notifications/notification_permission_context.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/notifications/notification_permission_context.h
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/notifications/notification_permission_context_unittest.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/permissions/permission_bubble_request_impl.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/permissions/permission_manager.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/push_messaging/push_messaging_browsertest.cc
[delete] https://crrev.com/4e4d53c69188102853c17db8c1d6041f3559a9f2/chrome/browser/push_messaging/push_messaging_permission_context.cc
[delete] https://crrev.com/4e4d53c69188102853c17db8c1d6041f3559a9f2/chrome/browser/push_messaging/push_messaging_permission_context.h
[delete] https://crrev.com/4e4d53c69188102853c17db8c1d6041f3559a9f2/chrome/browser/push_messaging/push_messaging_permission_context_unittest.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/browser/push_messaging/push_messaging_service_unittest.cc
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/chrome_browser.gypi
[modify] https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282/chrome/chrome_tests_unit.gypi

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 21 2016

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

commit d54345e651e054756b3c00fc80db05170f6cc129
Author: raymes <raymes@chromium.org>
Date: Thu Jul 21 04:58:15 2016

Use the same codepath for NOTIFICATIONS and PUSH_MESSAGING permissions

We used to have a complicated dependency between the notifications and push
messaging permissions and content settings. The push messaging content setting
will no longer be used (the setting will be deleted in a follow up CL). Since
the permissions are essentially the same now, we reuse the same
PermissionContext class for both, with some very slight behavioral differences
for push messaging.

Behaviorally, this change will mean that if a user happened to have
CONTENT_SETTINGS_TYPE_PUSH_MESSAGING set to block for a particular origin,
they will fall back to their CONTENT_SETTINGS_TYPE_NOTIFICATIONS setting for
that origin. Note that it's very unlikely that any users do have
CONTENT_SETTINGS_TYPE_PUSH_MESSAGING set, as it seems like this setting
was never actually exposed to users on stable. More details can be found here:
https://docs.google.com/document/d/1-Vny4paBx6gFZuCeLX2Gq9ztkF-JhNYM_uqvrQDj-6M/edit#heading=h.h99sppup43dn

BUG= 628058 

Committed: https://crrev.com/4f82b403f397fbad2e44fe287c05956fcb7cf282
Review-Url: https://codereview.chromium.org/2149883002
Cr-Original-Commit-Position: refs/heads/master@{#406225}
Cr-Commit-Position: refs/heads/master@{#406768}

[modify] https://crrev.com/d54345e651e054756b3c00fc80db05170f6cc129/chrome/app/generated_resources.grd
[modify] https://crrev.com/d54345e651e054756b3c00fc80db05170f6cc129/chrome/browser/notifications/notification_permission_context.cc
[modify] https://crrev.com/d54345e651e054756b3c00fc80db05170f6cc129/chrome/browser/notifications/notification_permission_context.h
[modify] https://crrev.com/d54345e651e054756b3c00fc80db05170f6cc129/chrome/browser/notifications/notification_permission_context_unittest.cc
[modify] https://crrev.com/d54345e651e054756b3c00fc80db05170f6cc129/chrome/browser/permissions/permission_manager.cc
[modify] https://crrev.com/d54345e651e054756b3c00fc80db05170f6cc129/chrome/browser/permissions/permission_request_impl.cc
[modify] https://crrev.com/d54345e651e054756b3c00fc80db05170f6cc129/chrome/browser/push_messaging/push_messaging_browsertest.cc
[delete] https://crrev.com/3239d5a2859023f95bb94774f77ddf903d6280cc/chrome/browser/push_messaging/push_messaging_permission_context.cc
[delete] https://crrev.com/3239d5a2859023f95bb94774f77ddf903d6280cc/chrome/browser/push_messaging/push_messaging_permission_context.h
[delete] https://crrev.com/3239d5a2859023f95bb94774f77ddf903d6280cc/chrome/browser/push_messaging/push_messaging_permission_context_unittest.cc
[modify] https://crrev.com/d54345e651e054756b3c00fc80db05170f6cc129/chrome/browser/push_messaging/push_messaging_service_unittest.cc
[modify] https://crrev.com/d54345e651e054756b3c00fc80db05170f6cc129/chrome/chrome_browser.gypi
[modify] https://crrev.com/d54345e651e054756b3c00fc80db05170f6cc129/chrome/chrome_tests_unit.gypi

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 2 2016

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

commit 516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6
Author: raymes <raymes@chromium.org>
Date: Tue Aug 02 06:43:27 2016

Remove remaining occurences of CONTENT_SETTINGS_TYPE_PUSH_MESSAGING

Occurences can either by safely changed to CONTENT_SETTINGS_TYPE_NOTIFICATIONS
or are removed (as often they had no purpose to begin with).

BUG= 628058 

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

[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/chrome/browser/permissions/permission_context_base_unittest.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/chrome/browser/permissions/permission_manager.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/chrome/browser/permissions/permission_manager_unittest.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/chrome/browser/permissions/permission_util.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/chrome/browser/push_messaging/push_messaging_service_impl.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/chrome/browser/push_messaging/push_messaging_service_unittest.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/chrome/browser/ui/website_settings/website_settings.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/chrome/browser/ui/website_settings/website_settings_ui.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/chrome/browser/ui/webui/options/content_settings_handler.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/chrome/browser/ui/webui/site_settings_helper.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/components/content_settings/core/browser/content_settings_default_provider.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/components/content_settings/core/browser/content_settings_registry.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/components/content_settings/core/common/content_settings.cc
[modify] https://crrev.com/516a7db6e16ec0eb7bb0c505c738e1a8c2627fe6/components/content_settings/core/common/content_settings_types.h

Comment 5 by raymes@chromium.org, Nov 17 2016

Status: Fixed (was: Assigned)

Comment 6 by lgar...@chromium.org, Nov 22 2016

Components: Internals>Permissions>Model

►

Issue 628058 link

Issue metadata

Remove the push messaging content setting

Issue description

Comment 1 by bugdroid1@chromium.org, Jul 19 2016

Comment 1 Deleted

Comment 2 by bugdroid1@chromium.org, Jul 19 2016

Comment 2 Deleted

Comment 3 by bugdroid1@chromium.org, Jul 21 2016

Comment 3 Deleted

Comment 4 by bugdroid1@chromium.org, Aug 2 2016

Comment 4 Deleted

Comment 5 by raymes@chromium.org, Nov 17 2016

Comment 5 Deleted

Comment 6 by lgar...@chromium.org, Nov 22 2016

Comment 6 Deleted

Sign in to add a comment