Push API unsubscribe() promise shouldn't hang offline |
||
Issue descriptionOur current behavior for pushSubscription.unsubscribe() is to delete local state then hang until we manage to tell GCM about the unsubscription (with exponential backoff retries on desktop at least). So unsubscribing offline may never resolve the Promise, but will have succeeded to the extent that subscribe()/getSubscription() will appear unsubscribed, and messages will no longer be delivered. It seems it would be nicer to resolve successfully to JS as soon as we clear local state. Note that calling subscribe() after we clear local state will work fine (no risk of getting back the old subscription from GCM) since we generate a new subtype (that's part of the local state we cleared). [In fact when testing on Linux 54.0.2840.90, I found that the promise hangs - is never resolved or rejected - even if we unsubscribe successfully from GCM! So there may be an additional bug here]
,
Feb 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ef71bd0bc3e889890de79958ae65052029d429f1 commit ef71bd0bc3e889890de79958ae65052029d429f1 Author: johnme <johnme@chromium.org> Date: Thu Feb 09 17:45:56 2017 Push API: Don't wait for network when unsubscribing pushSubscription.unsubscribe() used to delete local state then hang for up to 10 minutes until we managed to tell GCM servers about the unsubscription (the 10 minutes is due to exponential backoff retries on desktop, if the device is offline or on a flaky connection). So unsubscribing offline would never resolve the Promise if the tab/Service Worker was closed before 10 minutes, as is common, however the unsubscription would in fact have succeeded to the extent that subscribe()/getSubscription() correctly appear unsubscribed, and messages are no longer be delivered. This patch resolves the unsubscribe() promise as soon as the local state has been deleted, without waiting for network requests to GCM servers. Since we no longer use the results of those network requests directly, UMA logging is added to track them. To allow testing offline behavior, FakeGCMProfileService can now provide a coarse simulation of being offline, and I refactored it a little to simplify things. BUG= 669095 , 448500 Review-Url: https://codereview.chromium.org/2675293003 Cr-Commit-Position: refs/heads/master@{#449343} [modify] https://crrev.com/ef71bd0bc3e889890de79958ae65052029d429f1/chrome/browser/gcm/fake_gcm_profile_service.cc [modify] https://crrev.com/ef71bd0bc3e889890de79958ae65052029d429f1/chrome/browser/gcm/fake_gcm_profile_service.h [modify] https://crrev.com/ef71bd0bc3e889890de79958ae65052029d429f1/chrome/browser/push_messaging/push_messaging_browsertest.cc [modify] https://crrev.com/ef71bd0bc3e889890de79958ae65052029d429f1/chrome/browser/push_messaging/push_messaging_service_impl.cc [modify] https://crrev.com/ef71bd0bc3e889890de79958ae65052029d429f1/chrome/browser/push_messaging/push_messaging_service_impl.h [modify] https://crrev.com/ef71bd0bc3e889890de79958ae65052029d429f1/components/gcm_driver/gcm_client.h [modify] https://crrev.com/ef71bd0bc3e889890de79958ae65052029d429f1/components/gcm_driver/gcm_client_impl.cc [modify] https://crrev.com/ef71bd0bc3e889890de79958ae65052029d429f1/components/gcm_driver/instance_id/instance_id.h [modify] https://crrev.com/ef71bd0bc3e889890de79958ae65052029d429f1/tools/metrics/histograms/histograms.xml
,
Feb 10 2017
,
Feb 20 2017
See also https://github.com/w3c/push-api/issues/122 for updating the spec to allow this :) |
||
►
Sign in to add a comment |
||
Comment 1 by joh...@chromium.org
, Nov 28 2016