Reduce the impact of the push/notification kill switches

Reduce the impact of the push/notification kill switches

Enabling the kill switch today will not just block permission requests
and incoming push messages, but it will also cause (potentially mass)
unregistrations for websites that receive push messages while it's
enabled. This means that there's no clean recovery after disabling it.

This CL instead changes Push Messaging to silently drop received
messages on the floor whilst a permission kill switch has been enabled.
UMA will still be logged in that case.

BUG=

[Peter Beverloo, 2017-05-25 17:54:11]

The CQ bit was checked by peter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-25 17:54:41]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Peter Beverloo, 2017-05-25 17:55:00]

peter@chromium.org changed reviewers:
+ johnme@chromium.org

[Peter Beverloo, 2017-05-25 17:55:01]

+johnme, PTAL

[commit-bot: I haz the power, 2017-05-25 18:37:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-25 18:37:48]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Peter Beverloo, 2017-05-25 18:47:54]

The CQ bit was checked by peter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-25 18:48:29]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-25 20:09:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-25 20:09:12]

Dry run: This issue passed the CQ dry run.

[johnme, 2017-05-31 13:56:06]

tl;dr: lgtm

Ideally the suspended subscriptions would appear offline to GCM rather than
silently consuming messages whilst the kill switch is active, but unfortunately
it's not possible for Chrome to be only partially offline (without having
multiple connections to GCM, which would be very inefficient without major
refactorings of both GCM and Chrome), so this approach lgtm.

https://codereview.chromium.org/2907613002/diff/20001/chrome/browser/push_mes...
File chrome/browser/push_messaging/push_messaging_service_impl.cc (left):

https://codereview.chromium.org/2907613002/diff/20001/chrome/browser/push_mes...
chrome/browser/push_messaging/push_messaging_service_impl.cc:281: if
(!IsPermissionSet(app_identifier.origin())) {
Optional: since you're removing the second last usage of this, you could
consider inlining IsPermissionSet into OnContentSettingChanged.

[Peter Beverloo, 2017-06-02 13:42:36]

The CQ bit was checked by peter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-02 13:42:59]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Peter Beverloo, 2017-06-02 13:43:36]

peter@chromium.org changed reviewers:
+ clamy@chromium.org

[Peter Beverloo, 2017-06-02 13:43:37]

+clamy for trivial //content/public enum addition

Thanks John! In general I think of this kill switch as a (very) last resort, so
let's hope this code never gets exercised in practice ):.

https://codereview.chromium.org/2907613002/diff/20001/chrome/browser/push_mes...
File chrome/browser/push_messaging/push_messaging_service_impl.cc (left):

https://codereview.chromium.org/2907613002/diff/20001/chrome/browser/push_mes...
chrome/browser/push_messaging/push_messaging_service_impl.cc:281: if
(!IsPermissionSet(app_identifier.origin())) {
On 2017/05/31 13:56:06, johnme wrote:
> Optional: since you're removing the second last usage of this, you could
> consider inlining IsPermissionSet into OnContentSettingChanged.

Done.

[Peter Beverloo, 2017-06-02 13:43:50]

Description was changed from

==========
Reduce the impact of the push/notification kill switches

Enabling the kill switch today will not just block permission requests
and incoming push messages, but it will also cause (potentially mass)
unregistrations for websites that receive push messages while it's
enabled. This means that there's no clean recovery after disabling it.

This CL instead changes Push Messaging to silently drop received
messages on the floor whilst a permission kill switch has been enabled.
UMA will still be logged in that case.

BUG=
==========

to

==========
Reduce the impact of the push/notification kill switches

Enabling the kill switch today will not just block permission requests
and incoming push messages, but it will also cause (potentially mass)
unregistrations for websites that receive push messages while it's
enabled. This means that there's no clean recovery after disabling it.

This CL instead changes Push Messaging to silently drop received
messages on the floor whilst a permission kill switch has been enabled.
UMA will still be logged in that case.

BUG=
==========

[commit-bot: I haz the power, 2017-06-02 15:58:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-02 15:58:32]

Dry run: This issue passed the CQ dry run.

[clamy, 2017-06-09 14:48:29]

Thanks! content/ lgtm.