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

Issue 661660 link

Starred by 2 users
Status: Fixed
Owner:
█ joh...@chromium.org
no longer working on chrome
Closed: May 2017
Cc:
peter@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

When the GCM Store is reset, cached push subscriptions should be reset

Project Member Reported by joh...@chromium.org, Nov 2 2016 
What steps will reproduce the problem?
1. Go to https://rawgit.com/johnmellor/460bdac0c5b30832df898e67b4b7cedf/raw/560baaf598896aae1eed3309702ffd7c5e16369f/
2. Accept notification permission.
3. Write down the endpoint URL.
4. Confirm that pressing Send push results in a visible notification.
5. Close Chrome fully.
6. Delete your GCM Store directory (to simulate a store reset due to corruption etc).
7. Reopen Chrome, and go back to that page.

What is the expected output?
A different endpoint URL should be shown, and clicking Send push should successfully show a notification.

What do you see instead?
The same endpoint URL is shown, and clicking send gets a successful 200 response from the GCM server, but the push message never arrives and no notification is ever shown.
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 17 2016 

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

commit 93ec793ac69a542b2213297737178a55d069fd0d
Author: johnme <johnme@chromium.org>
Date: Thu Nov 17 14:26:58 2016

Notify GCMAppHandlers when the store is reset, so they clear cached IDs

The GCM Store can be reset due to corruption or GCM rejecting the
checkin (with a HTTP_UNAUTHORIZED or HTTP_BAD_REQUEST response status).

When this happens, the device ID changes and so all registrations are
invalidated, hence app handlers should clear any registration IDs they
store (locally and ideally remotely too). This patch does that for
locally cached registration IDs; it does not yet attempt to clear
remotely cached registration IDs.

It does so by adding an OnStoreReset method to the GCMAppHandler
interface, which suffers the known race condition that app handlers
that are not yet registered will not find out about the store being
reset (e.g. during startup); the comment on OnStoreReset warns about
this.

BUG= 661660 
TBR=rdevlin.cronin@chromium.org (based on jianli's review)

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

[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/chrome/browser/chromeos/policy/heartbeat_scheduler.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/chrome/browser/chromeos/policy/heartbeat_scheduler.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/chrome/browser/chromeos/policy/heartbeat_scheduler_unittest.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/chrome/browser/extensions/extension_gcm_app_handler.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/chrome/browser/extensions/extension_gcm_app_handler.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/chrome/browser/push_messaging/push_messaging_app_identifier.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/chrome/browser/push_messaging/push_messaging_app_identifier.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/chrome/browser/push_messaging/push_messaging_service_impl.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/chrome/browser/push_messaging/push_messaging_service_impl.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/default_gcm_app_handler.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/default_gcm_app_handler.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/fake_gcm_app_handler.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/fake_gcm_app_handler.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/gcm_account_mapper.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/gcm_account_mapper.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/gcm_app_handler.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/gcm_client.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/gcm_client_impl.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/gcm_client_impl_unittest.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/gcm_driver_desktop.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/gcm_driver/gcm_driver_desktop.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/invalidation/impl/gcm_invalidation_bridge.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/invalidation/impl/gcm_invalidation_bridge.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/invalidation/impl/gcm_invalidation_bridge_unittest.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/invalidation/impl/gcm_network_channel.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/invalidation/impl/gcm_network_channel.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/invalidation/impl/gcm_network_channel_delegate.h
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/invalidation/impl/gcm_network_channel_unittest.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl.cc
[modify] https://crrev.com/93ec793ac69a542b2213297737178a55d069fd0d/components/proximity_auth/cryptauth/cryptauth_gcm_manager_impl.h

Comment 2 by joh...@chromium.org, Feb 7 2017 

For web push messaging, we still need to make subscribe() and getSubscription() on desktop check with the GCM Store that the subscription is still valid, to fix things for subscriptions that predate 93ec793ac69a542b2213297737178a55d069fd0d or where a shutdown race condition prevented us from properly clearing the subscriptions after a GCM Store reset.
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 3 2017 

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

commit 6576ecf83ab56518daa1274dfa354aee84c8696b
Author: johnme <johnme@chromium.org>
Date: Mon Apr 03 19:26:28 2017

Push API: Validate storage before returning cached subscriptions

For desktop users whose GCM Store was reset before
93ec793ac69a542b2213297737178a55d069fd0d
(https://codereview.chromium.org/2473813002), or whose GCM Store was
reset after that patch but who encountered a race condition (e.g. around
browser shutdown) that prevented PushMessagingServiceImpl::OnStoreReset
from deleting affected subscriptions, it was possible to get into a
nasty state where PushManager.getSubscription() and
PushManager.subscribe() would both keep returning the old push
subscriptions cached by the Service Worker database, even though they
were no longer valid (resetting the GCM Store changed Chrome desktop's
device ID, so it's impossible for any messages to be delivered to the
old subscriptions).

It was also possible for the PushMessagingAppIdentifier map to get out
of sync with the Service Worker database (on both Android and desktop).

This patch makes PushManager.getSubscription() & PushManager.subscribe()
both validate that subscriptions are properly stored across all four
databases that could get out of sync:
- the Service Worker database
- the PushMessagingAppIdentifier map
- the GCM Store (on desktop only)
- the GCMKeyStore
(they don't check against the GCM server, since they must work offline)
and if an inconsistency is detected, it will be automatically resolved
by unsubscribing the invalid subscription (then re-subscribing, in the
case of PushManager.subscribe()).

BUG= 661660 

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

[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/chrome/browser/push_messaging/push_messaging_app_identifier.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/chrome/browser/push_messaging/push_messaging_app_identifier.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/chrome/browser/push_messaging/push_messaging_browsertest.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/chrome/browser/push_messaging/push_messaging_service_impl.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/chrome/browser/push_messaging/push_messaging_service_impl.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/chrome/test/data/push_messaging/push_test.js
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/crypto/gcm_encryption_provider.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/fake_gcm_client.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/fake_gcm_client.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/fake_gcm_driver.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/fake_gcm_driver.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/gcm_client.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/gcm_client_impl.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/gcm_client_impl.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/gcm_driver.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/gcm_driver.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/gcm_driver_android.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/gcm_driver_android.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/gcm_driver_desktop.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/gcm_driver_desktop.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/instance_id/fake_gcm_driver_for_instance_id.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/instance_id/fake_gcm_driver_for_instance_id.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/instance_id/instance_id.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/instance_id/instance_id_android.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/instance_id/instance_id_android.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/instance_id/instance_id_impl.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/components/gcm_driver/instance_id/instance_id_impl.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/content/browser/push_messaging/push_messaging_manager.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/content/browser/push_messaging/push_messaging_manager.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/content/child/push_messaging/push_provider.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/content/common/push_messaging.mojom
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/content/common/push_messaging_param_traits.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/content/public/browser/push_messaging_service.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/content/public/common/push_messaging_status.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/content/public/common/push_messaging_status.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/content/shell/browser/layout_test/layout_test_push_messaging_service.cc
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/content/shell/browser/layout_test/layout_test_push_messaging_service.h
[modify] https://crrev.com/6576ecf83ab56518daa1274dfa354aee84c8696b/tools/metrics/histograms/histograms.xml

Comment 4 by joh...@chromium.org, May 15 2017

Labels: M-59
Status: Fixed (was: Started)
►

Sign in to add a comment