Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1506613003:
Mark CONTENT_SETTINGS_TYPE_PUSH_MESSAGING as UNSYNCABLE (Closed)
|
Created:
5 years ago by johnme Modified:
5 years ago CC:
chromium-reviews, markusheintz_, msramek+watch_chromium.org, raymes+watch_chromium.org, Peter Beverloo Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMark CONTENT_SETTINGS_TYPE_PUSH_MESSAGING as UNSYNCABLE
The privacy considerations around push messaging are device-specific, so
the permission should not be synced. Also, this aligns push messaging
with CONTENT_SETTINGS_TYPE_NOTIFICATIONS, which is already UNSYNCABLE.
BUG=567153
Committed: https://crrev.com/0456c9bedd6a8a93362e6d67e8943b511811c06a
Cr-Commit-Position: refs/heads/master@{#363520}
Patch Set 1 #
Messages
Total messages: 17 (8 generated)
johnme@chromium.org changed reviewers: + msramek@chromium.org
msramek: the earliest reference I can find to push messaging being syncable is https://codereview.chromium.org/1005303003, where you return true from IsContentSettingsTypeSyncable. Can you remember why that was?
Description was changed from ========== Mark CONTENT_SETTINGS_TYPE_PUSH_MESSAGING as UNSYNCABLE The privacy considerations around push messaging are device-specific, so the permission should not be synced. Also, this aligns push messaging with CONTENT_SETTINGS_TYPE_NOTIFICATIONS, which is already UNSYNCABLE. BUG=567153 ========== to ========== Mark CONTENT_SETTINGS_TYPE_PUSH_MESSAGING as UNSYNCABLE The privacy considerations around push messaging are device-specific, so the permission should not be synced. Also, this aligns push messaging with CONTENT_SETTINGS_TYPE_NOTIFICATIONS, which is already UNSYNCABLE. BUG=567153 ==========
The CQ bit was checked by johnme@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1506613003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1506613003/1
Wow! So we did have PUSH_MESSAGING at that time already? That's a big oversight then. I remember that we went over settings that we want to stop syncing, and arrived at a list here: https://code.google.com/p/chromium/issues/detail?id=452388#c42 Apparently just nobody noticed :-(
msramek@chromium.org changed reviewers: + pavely@chromium.org
LGTM. I think switching a pref from syncable to unsyncable in place has no adverse effects, but adding +pavely@ from Sync to confirm.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
rs lgtm
The CQ bit was checked by johnme@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1506613003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1506613003/1
Message was sent while issue was closed.
Description was changed from ========== Mark CONTENT_SETTINGS_TYPE_PUSH_MESSAGING as UNSYNCABLE The privacy considerations around push messaging are device-specific, so the permission should not be synced. Also, this aligns push messaging with CONTENT_SETTINGS_TYPE_NOTIFICATIONS, which is already UNSYNCABLE. BUG=567153 ========== to ========== Mark CONTENT_SETTINGS_TYPE_PUSH_MESSAGING as UNSYNCABLE The privacy considerations around push messaging are device-specific, so the permission should not be synced. Also, this aligns push messaging with CONTENT_SETTINGS_TYPE_NOTIFICATIONS, which is already UNSYNCABLE. BUG=567153 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Mark CONTENT_SETTINGS_TYPE_PUSH_MESSAGING as UNSYNCABLE The privacy considerations around push messaging are device-specific, so the permission should not be synced. Also, this aligns push messaging with CONTENT_SETTINGS_TYPE_NOTIFICATIONS, which is already UNSYNCABLE. BUG=567153 ========== to ========== Mark CONTENT_SETTINGS_TYPE_PUSH_MESSAGING as UNSYNCABLE The privacy considerations around push messaging are device-specific, so the permission should not be synced. Also, this aligns push messaging with CONTENT_SETTINGS_TYPE_NOTIFICATIONS, which is already UNSYNCABLE. BUG=567153 Committed: https://crrev.com/0456c9bedd6a8a93362e6d67e8943b511811c06a Cr-Commit-Position: refs/heads/master@{#363520} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/0456c9bedd6a8a93362e6d67e8943b511811c06a Cr-Commit-Position: refs/heads/master@{#363520} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
