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

Issue 477401 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Merge the subscriptionId and endpoint properties in a PushSubscription object

Project Member Reported by peter@chromium.org, Apr 15 2015

Issue description

In the following PR, Martin proposes merging subscriptionId and endpoint.

https://github.com/w3c/push-api/pull/129

Let's do it. Developers who currently rely on both properties to exist independently of each other should move towards concatenating them if "subscriptionId" is still supported, for example:

serviceWorkerRegistration.pushManager.subscribe({
    userVisible: true
}).then(function(subscription) {
  var endpoint = subscription.endpoint;
  if ('subscriptionId' in subscription)
    endpoint += subscription.subscriptionId;

  // Transmit the data to the application server.
});
 

Comment 1 by peter@chromium.org, Apr 17 2015

To clarify - we plan to continue shipping *subscriptionId* with the current value until Chrome 45, while *endpoint* will contain the concatenated value.
Project Member

Comment 2 by bugdroid1@chromium.org, May 1 2015

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

commit bd88943c4c1605bbbf5e54e008f811aba3dd6539
Author: peter <peter@chromium.org>
Date: Fri May 01 16:24:30 2015

Prepare Push Messaging browser tests for the endpoint value change.

The subscriptionId property is being deprecated and its value will
be appended to the actual push endpoint. This CL prepares the browser
tests for this change, to make sure they don't break.

This CL is part of a three-sided patch:
  [1] This CL.
  [2] https://codereview.chromium.org/1084683003/
  [3] https://codereview.chromium.org/1078873004/

BUG= 477401 

Review URL: https://codereview.chromium.org/1099673002

Cr-Commit-Position: refs/heads/master@{#327918}

[modify] http://crrev.com/bd88943c4c1605bbbf5e54e008f811aba3dd6539/chrome/browser/push_messaging/push_messaging_browsertest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, May 5 2015

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=194922

------------------------------------------------------------------
r194922 | peter@chromium.org | 2015-05-05T13:40:13.269648Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/push_messaging/subscribe-success-in-document.html?r1=194922&r2=194921&pathrev=194922
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/push_messaging/PushSubscription.cpp?r1=194922&r2=194921&pathrev=194922
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/push_messaging/PushSubscription.h?r1=194922&r2=194921&pathrev=194922
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/UseCounter.cpp?r1=194922&r2=194921&pathrev=194922
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/push_messaging/PushSubscription.idl?r1=194922&r2=194921&pathrev=194922
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/UseCounter.h?r1=194922&r2=194921&pathrev=194922

Merge the value of the push subscription id into the endpoint.

This change merges the subscription id belonging to a push subscription
into the endpoint, and deprecates the subscriptionId attribute.

The change is part of the following Intent to Deprecate thread:
https://groups.google.com/a/chromium.org/d/topic/blink-dev/CK13omVO5ds/discussion

When developers access the subscriptionId after all, they will get a
deprecation warning shown in their console:

> 'PushSubscription.subscriptionId' is deprecated and is now included in
> 'PushSubscription.endpoint'. It will be removed in Chrome 45, around
> August 2015.

This CL is part of a three-sided patch:
  [1] https://codereview.chromium.org/1099673002/
  [2] This CL.
  [3] https://codereview.chromium.org/1078873004/

BUG= 477401 

Review URL: https://codereview.chromium.org/1084683003
-----------------------------------------------------------------
Project Member

Comment 4 by bugdroid1@chromium.org, May 6 2015

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

commit 7015c4d2419cd07f4643dd33321991ee81dcb6a2
Author: peter <peter@chromium.org>
Date: Wed May 06 13:41:27 2015

Address the TODOs in the PushMessagingBrowserTest for the endpoint merge.

The endpoint value checks have been put back in place, now following
the new format and only checking the |endpoint| value. Inclusion of the
subscription Id in the endpoint is covered by Blink's layout tests.

This CL is part of a three-sided patch:
  [1] https://codereview.chromium.org/1099673002/
  [2] https://codereview.chromium.org/1084683003/
  [3] This CL.

BUG= 477401 

Review URL: https://codereview.chromium.org/1078873004

Cr-Commit-Position: refs/heads/master@{#328523}

[modify] http://crrev.com/7015c4d2419cd07f4643dd33321991ee81dcb6a2/chrome/browser/push_messaging/push_messaging_browsertest.cc
[modify] http://crrev.com/7015c4d2419cd07f4643dd33321991ee81dcb6a2/chrome/test/data/push_messaging/push_test.js

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 2 2015

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=196315

------------------------------------------------------------------
r196315 | peter@chromium.org | 2015-06-02T14:04:30.947847Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/push_messaging/get-subscription-in-document.html?r1=196315&r2=196314&pathrev=196315
   A http://src.chromium.org/viewvc/blink/trunk/Source/platform/exported/WebPushSubscription.cpp?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/webexposed/global-interface-listing-expected.txt?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/push_messaging/PushSubscription.cpp?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/push_messaging/resources/instrumentation-service-worker.js?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/push_messaging/push-subscription-stringification.html?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/push_messaging/unsubscribe-in-service-worker.html?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/public/platform/modules/push_messaging/WebPushSubscription.h?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/push_messaging/PushSubscription.h?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/blink_platform.gypi?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/UseCounter.cpp?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/push_messaging/subscribe-success-in-service-worker.html?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/UseCounter.h?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/push_messaging/subscribe-success-in-document.html?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/http/tests/push_messaging/get-subscription-in-service-worker.html?r1=196315&r2=196314&pathrev=196315
   M http://src.chromium.org/viewvc/blink/trunk/Source/modules/push_messaging/PushSubscription.idl?r1=196315&r2=196314&pathrev=196315

Remove support for PushSubscription.subscriptionId.

The subscriptionId property has been included in endpoint for Chrome 44,
and is to be removed in Chrome 45, per the following Intent:

https://groups.google.com/a/chromium.org/d/topic/blink-dev/6OK5qm491Eg/discussion

This CL is part of a three-sided patch:
  [1] This CL.
  [2] https://codereview.chromium.org/1154303003/
  [3] https://codereview.chromium.org/1154233005/

BUG= 477401 

Review URL: https://codereview.chromium.org/1148763005
-----------------------------------------------------------------
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 3 2015

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

commit 109e5107e8db6ed12f0b9a903c87cdc485d29365
Author: peter <peter@chromium.org>
Date: Wed Jun 03 12:25:37 2015

Move push endpoint concatenation up to the browser process.

The renderer no longer needs to know about the subscription id independent
of the endpoint, so we can concatenate the endpoint and the subscription
id before sending it to the renderer.

This CL is part of a three-sided patch:
  [1] https://codereview.chromium.org/1148763005/
  [2] This CL.
  [3] https://codereview.chromium.org/1154233005/

BUG= 477401 

Review URL: https://codereview.chromium.org/1154303003

Cr-Commit-Position: refs/heads/master@{#332602}

[modify] http://crrev.com/109e5107e8db6ed12f0b9a903c87cdc485d29365/content/browser/push_messaging/push_messaging_message_filter.cc
[modify] http://crrev.com/109e5107e8db6ed12f0b9a903c87cdc485d29365/content/browser/push_messaging/push_messaging_message_filter.h
[modify] http://crrev.com/109e5107e8db6ed12f0b9a903c87cdc485d29365/content/child/push_messaging/push_provider.cc
[modify] http://crrev.com/109e5107e8db6ed12f0b9a903c87cdc485d29365/content/child/push_messaging/push_provider.h
[modify] http://crrev.com/109e5107e8db6ed12f0b9a903c87cdc485d29365/content/common/push_messaging_messages.h
[modify] http://crrev.com/109e5107e8db6ed12f0b9a903c87cdc485d29365/content/renderer/push_messaging/push_messaging_dispatcher.cc
[modify] http://crrev.com/109e5107e8db6ed12f0b9a903c87cdc485d29365/content/renderer/push_messaging/push_messaging_dispatcher.h

Comment 7 by peter@chromium.org, Jun 5 2015

Owner: peter@chromium.org
Status: Fixed

Sign in to add a comment