Push API: rename registration to subscription.

Push API: rename registration to subscription.

This is in response to this spec issue:
https://github.com/w3c/push-api/issues/98

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

BUG=446883
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=188173

[Peter Beverloo, 2015-01-09 16:33:08]

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

[Peter Beverloo, 2015-01-09 16:33:09]

https://codereview.chromium.org/841333002/diff/20001/Source/modules/push_mess...
File Source/modules/push_messaging/PushManager.cpp (right):

https://codereview.chromium.org/841333002/diff/20001/Source/modules/push_mess...
Source/modules/push_messaging/PushManager.cpp:63:
pushProvider()->registerPushMessaging(m_registration->webRegistration(), new
PushSubscriptionCallbacks(resolver, m_registration));
I presume that you're going to do the API method renames in another 3-sided
patch, rather than including them in the API definitions now (albeit as aliases
on the Chromium side)?

https://codereview.chromium.org/841333002/diff/20001/Source/modules/push_mess...
File Source/modules/push_messaging/PushSubscription.h (right):

https://codereview.chromium.org/841333002/diff/20001/Source/modules/push_mess...
Source/modules/push_messaging/PushSubscription.h:1: // Copyright 2015 The
Chromium Authors. All rights reserved.
nit: we usually don't change copyright years for plain renames.

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushC...
File public/platform/WebPushClient.h (right):

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushC...
public/platform/WebPushClient.h:16: using WebPushSubscriptionCallbacks =
WebCallbacks<WebPushSubscription, WebPushError>;
+FIXME

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushP...
File public/platform/WebPushProvider.h (right):

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushP...
public/platform/WebPushProvider.h:18: using WebPushSubscriptionCallbacks =
WebCallbacks<WebPushSubscription, WebPushError>;
+FIXMEs

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushR...
File public/platform/WebPushRegistration.h (right):

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushR...
public/platform/WebPushRegistration.h:12: using WebPushRegistration =
WebPushSubscription;
+FIXME

[Michael van Ouwerkerk, 2015-01-09 17:11:39]

Thanks Peter, please take another look.

https://codereview.chromium.org/841333002/diff/20001/Source/modules/push_mess...
File Source/modules/push_messaging/PushManager.cpp (right):

https://codereview.chromium.org/841333002/diff/20001/Source/modules/push_mess...
Source/modules/push_messaging/PushManager.cpp:63:
pushProvider()->registerPushMessaging(m_registration->webRegistration(), new
PushSubscriptionCallbacks(resolver, m_registration));
On 2015/01/09 16:33:09, Peter Beverloo wrote:
> I presume that you're going to do the API method renames in another 3-sided
> patch, rather than including them in the API definitions now (albeit as
aliases
> on the Chromium side)?

Yea let's just get the web exposed stuff in asap and follow up with the rest
later.

https://codereview.chromium.org/841333002/diff/20001/Source/modules/push_mess...
File Source/modules/push_messaging/PushSubscription.h (right):

https://codereview.chromium.org/841333002/diff/20001/Source/modules/push_mess...
Source/modules/push_messaging/PushSubscription.h:1: // Copyright 2015 The
Chromium Authors. All rights reserved.
On 2015/01/09 16:33:09, Peter Beverloo wrote:
> nit: we usually don't change copyright years for plain renames.

Done.

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushC...
File public/platform/WebPushClient.h (right):

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushC...
public/platform/WebPushClient.h:16: using WebPushSubscriptionCallbacks =
WebCallbacks<WebPushSubscription, WebPushError>;
On 2015/01/09 16:33:09, Peter Beverloo wrote:
> +FIXME

Done.

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushP...
File public/platform/WebPushProvider.h (right):

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushP...
public/platform/WebPushProvider.h:18: using WebPushSubscriptionCallbacks =
WebCallbacks<WebPushSubscription, WebPushError>;
On 2015/01/09 16:33:09, Peter Beverloo wrote:
> +FIXMEs

Done.

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushR...
File public/platform/WebPushRegistration.h (right):

https://codereview.chromium.org/841333002/diff/20001/public/platform/WebPushR...
public/platform/WebPushRegistration.h:12: using WebPushRegistration =
WebPushSubscription;
On 2015/01/09 16:33:09, Peter Beverloo wrote:
> +FIXME

Done.

[Michael van Ouwerkerk, 2015-01-09 17:12:21]

mvanouwerkerk@chromium.org changed reviewers:
+ miguelg@chromium.org

[Michael van Ouwerkerk, 2015-01-09 17:12:22]

Adding Miguel as he seemed interested in this rename.

[Michael van Ouwerkerk, 2015-01-09 17:12:43]

mvanouwerkerk@chromium.org changed reviewers:
+ mkwst@chromium.org

[Michael van Ouwerkerk, 2015-01-09 17:12:44]

Mike, could you take a look please as platform owner? Thanks!

[Miguel Garcia, 2015-01-09 17:15:58]

miguelg@chromium.org changed reviewers:
- miguelg@chromium.org

[Peter Beverloo, 2015-01-09 17:23:23]

lgtm

For future renaming patches of similar magnitude, please try to isolate the
changes to *only* the rename.

In this case the assert_equals -> assert_regexp_match is minor, but it's very
easy to miss such differences in an otherwise mechanical change.

https://codereview.chromium.org/841333002/diff/40001/LayoutTests/http/tests/p...
File LayoutTests/http/tests/push_messaging/subscribe_permission_denied.html
(right):

https://codereview.chromium.org/841333002/diff/40001/LayoutTests/http/tests/p...
LayoutTests/http/tests/push_messaging/subscribe_permission_denied.html:31:
assert_regexp_match(e.message, /permission denied/);
What's the benefit of doing this? Do you expect the name to change again? :-)

https://codereview.chromium.org/841333002/diff/40001/Source/modules/push_mess...
File Source/modules/push_messaging/PushManager.idl (right):

https://codereview.chromium.org/841333002/diff/40001/Source/modules/push_mess...
Source/modules/push_messaging/PushManager.idl:5: //
http://w3c.github.io/push-api/#pushmanager-interface
micro nit: https://

[Michael van Ouwerkerk, 2015-01-09 17:34:39]

Then maybe don't ask me for changes like http to https :-P

Thanks for the review!

https://codereview.chromium.org/841333002/diff/40001/LayoutTests/http/tests/p...
File LayoutTests/http/tests/push_messaging/subscribe_permission_denied.html
(right):

https://codereview.chromium.org/841333002/diff/40001/LayoutTests/http/tests/p...
LayoutTests/http/tests/push_messaging/subscribe_permission_denied.html:31:
assert_regexp_match(e.message, /permission denied/);
On 2015/01/09 17:23:22, Peter Beverloo wrote:
> What's the benefit of doing this? Do you expect the name to change again? :-)

This error message comes from the embedder, we have to handle both the
'register' and the 'subscribe' message here while transitioning.

https://codereview.chromium.org/841333002/diff/40001/Source/modules/push_mess...
File Source/modules/push_messaging/PushManager.idl (right):

https://codereview.chromium.org/841333002/diff/40001/Source/modules/push_mess...
Source/modules/push_messaging/PushManager.idl:5: //
http://w3c.github.io/push-api/#pushmanager-interface
On 2015/01/09 17:23:22, Peter Beverloo wrote:
> micro nit: https://

Done.

[Michael van Ouwerkerk, 2015-01-09 19:41:54]

mvanouwerkerk@chromium.org changed reviewers:
+ dglazkov@chromium.org

[Michael van Ouwerkerk, 2015-01-09 19:41:55]

Dimitri, could you please take a look as owner of public/platform? Mike seems to
have started his weekend :-)

[dglazkov, 2015-01-09 20:19:15]

LGTM.

[Michael van Ouwerkerk, 2015-01-09 20:21:03]

Thanks!

[Michael van Ouwerkerk, 2015-01-09 20:21:21]

The CQ bit was checked by mvanouwerkerk@chromium.org

[commit-bot: I haz the power, 2015-01-09 20:22:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/841333002/60001

[Mike West, 2015-01-09 22:22:57]

LGTM too, sorry for the belated review. :/

[commit-bot: I haz the power, 2015-01-09 22:27:33]

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

[commit-bot: I haz the power, 2015-01-09 22:27:35]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/45565)

[Michael van Ouwerkerk, 2015-01-09 22:33:14]

The CQ bit was checked by mvanouwerkerk@chromium.org

[commit-bot: I haz the power, 2015-01-09 22:33:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/841333002/60001

[commit-bot: I haz the power, 2015-01-09 23:26:07]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=188173

[kenneth.christiansen, 2015-01-12 14:44:08]

Shouldn't you update the spec draft as well?

[Peter Beverloo, 2015-01-12 14:45:04]

On 2015/01/12 14:44:08, kenneth.christiansen wrote:
> Shouldn't you update the spec draft as well?

https://w3c.github.io/push-api/ is up to date as far as I can see? Did we miss
anything?

[Michael van Ouwerkerk, 2015-01-12 14:47:31]

On 2015/01/12 14:45:04, Peter Beverloo wrote:
> On 2015/01/12 14:44:08, kenneth.christiansen wrote:
> > Shouldn't you update the spec draft as well?
> 
> https://w3c.github.io/push-api/ is up to date as far as I can see? Did we miss
> anything?

Yes, I think I addressed it in the spec with this pull request:
https://github.com/w3c/push-api/pull/102