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

Issue 608831 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
no longer working on chrome
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocking:
issue 589461



Sign in to add a comment

Push messaging sender ID can get out of sync with registration ID

Project Member Reported by joh...@chromium.org, May 3 2016

Issue description

We store the sender ID into the Service Worker database whenever an attempt was made at registering. This means that a successful subscribe followed by an unsuccessful subscribe with a different sender ID leaves the SW database in an inconsistent state: we'd be storing the old successful registration ID, but the new unsuccessful sender ID!

This is mainly a problem when unsubscribing from GCM, which requires the correct sender ID on Android.
 
Blocking: 589461
Project Member

Comment 2 by bugdroid1@chromium.org, May 6 2016

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

commit fa6acac911373fd2e0f56a8a5f9652a7cc21b53e
Author: johnme <johnme@chromium.org>
Date: Fri May 06 13:20:47 2016

Make Service Worker DB UserData methods accept multiple keys at once

Primary benefit: it's now possible to read/write multiple keys at once
transactionally. This will be used in
https://codereview.chromium.org/1944863002 to fix the way
PushMessagingMessageFilter stores the sender ID and subscription ID.

Secondary benefit: this occasionally reduces the number of async hops,
for example in PushMessagingMessageFilter::UnsubscribeHavingGottenIds.

Since Chrome now supports initializer lists, it doesn't add much
code complexity when it isn't necessary (and hence it didn't seem
worth duplicating all these methods).

BUG= 608831 

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

[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/background_sync/background_sync_manager.cc
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/push_messaging/push_messaging_message_filter.cc
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/push_messaging/push_messaging_message_filter.h
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/service_worker/service_worker_context_wrapper.cc
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/service_worker/service_worker_context_wrapper.h
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/service_worker/service_worker_database.cc
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/service_worker/service_worker_database.h
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/service_worker/service_worker_database_unittest.cc
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/service_worker/service_worker_registration.h
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/service_worker/service_worker_storage.cc
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/service_worker/service_worker_storage.h
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/browser/service_worker/service_worker_storage_unittest.cc
[modify] https://crrev.com/fa6acac911373fd2e0f56a8a5f9652a7cc21b53e/content/public/browser/push_messaging_service.cc

Project Member

Comment 3 by bugdroid1@chromium.org, May 11 2016

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

commit 8112686d4a56ef5200014a9ffa294529d071debc
Author: johnme <johnme@chromium.org>
Date: Wed May 11 20:15:53 2016

Push API: Fix stored sender ID being out of sync with registration ID

Until now we stored the sender ID into the Service Worker database
whenever an attempt was made at registering. This meant that a
successful subscribe followed by an unsuccessful subscribe with a
different sender ID would leave the SW database in an inconsistent
state: we'd be storing the old successful registration ID, but the new
unsuccessful sender ID!

This is mainly a problem when unsubscribing from GCM, which requires the
correct sender ID on Android.

This patch fixes that, so that henceforth whenever a registration ID is
stored, the sender ID is guaranteed to be the one used when subscribing.

Depends on https://codereview.chromium.org/1945753002

BUG= 608831 

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

[modify] https://crrev.com/8112686d4a56ef5200014a9ffa294529d071debc/content/browser/push_messaging/push_messaging_message_filter.cc
[modify] https://crrev.com/8112686d4a56ef5200014a9ffa294529d071debc/content/browser/push_messaging/push_messaging_message_filter.h
[modify] https://crrev.com/8112686d4a56ef5200014a9ffa294529d071debc/content/child/push_messaging/push_provider.cc
[modify] https://crrev.com/8112686d4a56ef5200014a9ffa294529d071debc/content/common/push_messaging_messages.h
[modify] https://crrev.com/8112686d4a56ef5200014a9ffa294529d071debc/content/renderer/push_messaging/push_messaging_dispatcher.cc

Comment 4 by joh...@chromium.org, May 12 2016

Status: Fixed (was: Started)
Fix will be in Chrome 52

Sign in to add a comment