Implement WebPushProvider.unregister() in Chromium.

Implement WebPushProvider.unregister() in Chromium.

This is plumbing the call all the way to GCM in chrome/.
It is also updating the layout test push service to react properly if
unregister() is called multiple times or on a non-registered origin.

This is part of a multi-sided CL:
Part 1: https://codereview.chromium.org/793913002/
Part 2: <this>
Part 3: https://codereview.chromium.org/800503002/

BUG=440958
Committed: https://crrev.com/2447700c2e26741e9ca3d043c3693884d7b66cf5
Cr-Commit-Position: refs/heads/master@{#308819}

[mlamouri (slow - plz ping), 2014-12-11 16:53:08]

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

[Michael van Ouwerkerk, 2014-12-12 13:55:41]

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
File content/browser/push_messaging/push_messaging_message_filter.h (right):

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
content/browser/push_messaging/push_messaging_message_filter.h:81: int
request_id);
Please keep request_id as the first argument and service_worker_registration_id
as the second, like in OnUnregister.

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
File content/child/push_messaging/push_provider.cc (right):

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
content/child/push_messaging/push_provider.cc:102:
blink::WebPushUnregisterCallbacks* callback) {
Please rename callback to callbacks for consistency.

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
content/child/push_messaging/push_provider.cc:196:
scoped_ptr<blink::WebPushUnregisterCallbacks> callback(it->second);
s/callback/callbacks/

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
File content/child/push_messaging/push_provider.h (right):

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
content/child/push_messaging/push_provider.h:58: void OnUnregisterResponse(int
request_id, bool response);
nit: consider renaming |response| to something more meaningful such as
|success|.

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
content/child/push_messaging/push_provider.h:74: std::map<int,
blink::WebPushUnregisterCallbacks*> unregister_callbacks_;
Aha, leaking a whole map of raw pointers, just like me :-) If this class is
destroyed before the IPC responds, these leak. See ~PushProvider.

https://codereview.chromium.org/793403002/diff/20001/content/common/push_mess...
File content/common/push_messaging_messages.h (right):

https://codereview.chromium.org/793403002/diff/20001/content/common/push_mess...
content/common/push_messaging_messages.h:60: bool /* response */)
success?

https://codereview.chromium.org/793403002/diff/20001/content/shell/browser/la...
File content/shell/browser/layout_test/layout_test_push_messaging_service.cc
(right):

https://codereview.chromium.org/793403002/diff/20001/content/shell/browser/la...
content/shell/browser/layout_test/layout_test_push_messaging_service.cc:52:
registrations_.insert(requesting_origin);
Move this before running the callback.

[johnme, 2014-12-12 15:20:06]

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

[johnme, 2014-12-12 15:20:07]

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
File content/browser/push_messaging/push_messaging_message_filter.h (right):

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
content/browser/push_messaging/push_messaging_message_filter.h:73: void
OnUnregister(int request_id,
Please use the following order when adding methods to this class (and everywhere
else):

  Register
  Unregister
  GetRegistration
  HasPermission
  Delivery (not applicable in this class)

Once you rebase your patch, you'll see that registration is now an 8-step
process (darn thread hops!), and it's vastly clearer to keep all the
registration methods together and in flow order, then all the unregistration
methods in order, etc, rather than interspersing these flows of different
length.

The PermissionStatus methods predate Michael and I agreeing to this order; I'll
fix that after m41, when it won't cause unnecessary merge conflicts for
everyone.

[johnme, 2014-12-12 15:22:02]

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
File content/browser/push_messaging/push_messaging_message_filter.cc (right):

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
content/browser/push_messaging/push_messaging_message_filter.cc:308: Send(new
PushMessagingMsg_UnregisterResponse(request_id, result));
Please remove the registration from SW storage now.

[mlamouri (slow - plz ping), 2014-12-15 11:29:37]

Comments applied. PTAL.

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
File content/browser/push_messaging/push_messaging_message_filter.cc (right):

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
content/browser/push_messaging/push_messaging_message_filter.cc:308: Send(new
PushMessagingMsg_UnregisterResponse(request_id, result));
On 2014/12/12 15:22:02, johnme wrote:
> Please remove the registration from SW storage now.

I will do that in a follow-up CL. No need to have a big patch if we can cut
things in bite sized chunks.

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
File content/browser/push_messaging/push_messaging_message_filter.h (right):

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
content/browser/push_messaging/push_messaging_message_filter.h:73: void
OnUnregister(int request_id,
On 2014/12/12 15:20:07, johnme wrote:
> Please use the following order when adding methods to this class (and
everywhere
> else):
> 
>   Register
>   Unregister
>   GetRegistration
>   HasPermission
>   Delivery (not applicable in this class)
> 
> Once you rebase your patch, you'll see that registration is now an 8-step
> process (darn thread hops!), and it's vastly clearer to keep all the
> registration methods together and in flow order, then all the unregistration
> methods in order, etc, rather than interspersing these flows of different
> length.
> 
> The PermissionStatus methods predate Michael and I agreeing to this order;
I'll
> fix that after m41, when it won't cause unnecessary merge conflicts for
> everyone.

Not sure what you wanted here. I've added an "unregister methods" section and
put all the unregister-related methods there. Hopefully, that's what you wanted.

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
content/browser/push_messaging/push_messaging_message_filter.h:81: int
request_id);
On 2014/12/12 13:55:41, Michael van Ouwerkerk wrote:
> Please keep request_id as the first argument and
service_worker_registration_id
> as the second, like in OnUnregister.

Done.

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
File content/child/push_messaging/push_provider.cc (right):

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
content/child/push_messaging/push_provider.cc:102:
blink::WebPushUnregisterCallbacks* callback) {
On 2014/12/12 13:55:41, Michael van Ouwerkerk wrote:
> Please rename callback to callbacks for consistency.

I prefer not. See below.

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
content/child/push_messaging/push_provider.cc:196:
scoped_ptr<blink::WebPushUnregisterCallbacks> callback(it->second);
On 2014/12/12 13:55:41, Michael van Ouwerkerk wrote:
> s/callback/callbacks/

As much as we need to keep the silly class name because of consistency, using
|callbacks| as a variable name is simply confusing because it's actually *one*
callback, not a list of callbacks.

I would gladly s/callbacks/callback/ on the rest of the file if you want.

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
File content/child/push_messaging/push_provider.h (right):

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
content/child/push_messaging/push_provider.h:58: void OnUnregisterResponse(int
request_id, bool response);
On 2014/12/12 13:55:41, Michael van Ouwerkerk wrote:
> nit: consider renaming |response| to something more meaningful such as
> |success|.

Done.

https://codereview.chromium.org/793403002/diff/20001/content/child/push_messa...
content/child/push_messaging/push_provider.h:74: std::map<int,
blink::WebPushUnregisterCallbacks*> unregister_callbacks_;
On 2014/12/12 13:55:41, Michael van Ouwerkerk wrote:
> Aha, leaking a whole map of raw pointers, just like me :-) If this class is
> destroyed before the IPC responds, these leak. See ~PushProvider.

Yeah. Right. I usually use IDMap<> and have it own the callbacks. Why don't we
do the same here?

(Fixed, but I would love to change that in a follow-up.)

https://codereview.chromium.org/793403002/diff/20001/content/common/push_mess...
File content/common/push_messaging_messages.h (right):

https://codereview.chromium.org/793403002/diff/20001/content/common/push_mess...
content/common/push_messaging_messages.h:60: bool /* response */)
On 2014/12/12 13:55:41, Michael van Ouwerkerk wrote:
> success?

Done.

https://codereview.chromium.org/793403002/diff/20001/content/shell/browser/la...
File content/shell/browser/layout_test/layout_test_push_messaging_service.cc
(right):

https://codereview.chromium.org/793403002/diff/20001/content/shell/browser/la...
content/shell/browser/layout_test/layout_test_push_messaging_service.cc:52:
registrations_.insert(requesting_origin);
On 2014/12/12 13:55:41, Michael van Ouwerkerk wrote:
> Move this before running the callback.

Done.

[johnme, 2014-12-15 12:07:16]

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
File content/browser/push_messaging/push_messaging_message_filter.h (right):

https://codereview.chromium.org/793403002/diff/20001/content/browser/push_mes...
content/browser/push_messaging/push_messaging_message_filter.h:73: void
OnUnregister(int request_id,
On 2014/12/15 11:29:36, Mounir Lamouri wrote:
> On 2014/12/12 15:20:07, johnme wrote:
> > Please use the following order when adding methods to this class (and
> everywhere
> > else):
> > 
> >   Register
> >   Unregister
> >   GetRegistration
> >   HasPermission
> >   Delivery (not applicable in this class)
> > 
> > Once you rebase your patch, you'll see that registration is now an 8-step
> > process (darn thread hops!), and it's vastly clearer to keep all the
> > registration methods together and in flow order, then all the unregistration
> > methods in order, etc, rather than interspersing these flows of different
> > length.
> > 
> > The PermissionStatus methods predate Michael and I agreeing to this order;
> I'll
> > fix that after m41, when it won't cause unnecessary merge conflicts for
> > everyone.
> 
> Not sure what you wanted here. I've added an "unregister methods" section and
> put all the unregister-related methods there. Hopefully, that's what you
wanted.

Sorry, I meant to put all the unregister methods under all the register methods
please, i.e. as follows:

OnRegisterFromDocumentOld
OnRegisterFromDocument
OnRegisterFromWorker
OnPermissionStatusRequest
OnGetPermissionStatus
CheckForExistingRegistration
DidCheckForExistingRegistration
RegisterOnUI
DoPermissionStatusRequest
GetPermissionStatusOnUI
DidRegister
PersistRegistrationOnIO
DidPersistRegistrationOnIO
SendRegisterError
SendRegisterSuccess
SendRegisterSuccessOnUI
+OnUnregister
+UnregisterOnUI
+DidUnregister

(and I'll add the .*GetRegistration.* methods underneath your Unregister
methods, and eventually move the .*Permission.* methods to the bottom)

https://codereview.chromium.org/793403002/diff/60001/chrome/browser/services/...
File chrome/browser/services/gcm/push_messaging_service_impl.h (right):

https://codereview.chromium.org/793403002/diff/60001/chrome/browser/services/...
chrome/browser/services/gcm/push_messaging_service_impl.h:76: void Unregister(
Similarly here, please move this above the GetPermissionStatus methods (keeping
to the same Register,Unregister,GetRegistration,HasPermission,Delivery order).

https://codereview.chromium.org/793403002/diff/60001/content/child/push_messa...
File content/child/push_messaging/push_provider.h (right):

https://codereview.chromium.org/793403002/diff/60001/content/child/push_messa...
content/child/push_messaging/push_provider.h:40: virtual void
unregister(blink::WebServiceWorkerRegistration*,
Ditto method order. And below.

[Michael van Ouwerkerk, 2014-12-15 12:29:14]

https://codereview.chromium.org/793403002/diff/60001/content/child/push_messa...
File content/child/push_messaging/push_provider.h (right):

https://codereview.chromium.org/793403002/diff/60001/content/child/push_messa...
content/child/push_messaging/push_provider.h:58: void OnUnregisterResponse(int
request_id, bool unregistered);
I'd like more clarity on the meaning of this boolean before committing. Is it
"operation success" or "registration state is unregistered"? When unregistering
the same registration twice, the first meaning could be false and the second
true, for the same end result.

[mlamouri (slow - plz ping), 2014-12-15 20:06:50]

mlamouri@chromium.org changed reviewers:
+ jochen@chromium.org, tsepez@chromium.org

[mlamouri (slow - plz ping), 2014-12-15 20:06:50]

tsepez@chromium.org: Please review changes in 
- content/commen/push_messaging_messages.h

jochen@chromium.org: Please review changes in 
- content/ (OWNERS review, Michael will have a look too)

[Tom Sepez, 2014-12-15 20:27:54]

Messages themselves LGTM.

[Michael van Ouwerkerk, 2014-12-16 11:36:51]

https://codereview.chromium.org/793403002/diff/60001/content/child/push_messa...
File content/child/push_messaging/push_provider.h (right):

https://codereview.chromium.org/793403002/diff/60001/content/child/push_messa...
content/child/push_messaging/push_provider.h:58: void OnUnregisterResponse(int
request_id, bool unregistered);
On 2014/12/15 12:29:14, Michael van Ouwerkerk wrote:
> I'd like more clarity on the meaning of this boolean before committing. Is it
> "operation success" or "registration state is unregistered"? When
unregistering
> the same registration twice, the first meaning could be false and the second
> true, for the same end result.

From [1] it seems your intent is to signal whether the operation succeeded, not
whether the current state is unregistered. Please rename |unregistered| to
|success|.

[1] https://github.com/w3c/push-api/pull/97#issuecomment-67039200

[mlamouri (slow - plz ping), 2014-12-16 11:39:12]

On 2014/12/16 11:36:51, Michael van Ouwerkerk wrote:
>
https://codereview.chromium.org/793403002/diff/60001/content/child/push_messa...
> File content/child/push_messaging/push_provider.h (right):
> 
>
https://codereview.chromium.org/793403002/diff/60001/content/child/push_messa...
> content/child/push_messaging/push_provider.h:58: void OnUnregisterResponse(int
> request_id, bool unregistered);
> On 2014/12/15 12:29:14, Michael van Ouwerkerk wrote:
> > I'd like more clarity on the meaning of this boolean before committing. Is
it
> > "operation success" or "registration state is unregistered"? When
> unregistering
> > the same registration twice, the first meaning could be false and the second
> > true, for the same end result.
> 
> From [1] it seems your intent is to signal whether the operation succeeded,
not
> whether the current state is unregistered. Please rename |unregistered| to
> |success|.
> 
> [1] https://github.com/w3c/push-api/pull/97#issuecomment-67039200

If the promise is fulfilled, that means that the current state is unregistered
and the operation succeeded. If the value is false, it means that it was not
required to do any unregistration.

[Michael van Ouwerkerk, 2014-12-16 11:49:13]

On 2014/12/16 11:39:12, Mounir Lamouri wrote:
> On 2014/12/16 11:36:51, Michael van Ouwerkerk wrote:
> >
>
https://codereview.chromium.org/793403002/diff/60001/content/child/push_messa...
> > File content/child/push_messaging/push_provider.h (right):
> > 
> >
>
https://codereview.chromium.org/793403002/diff/60001/content/child/push_messa...
> > content/child/push_messaging/push_provider.h:58: void
OnUnregisterResponse(int
> > request_id, bool unregistered);
> > On 2014/12/15 12:29:14, Michael van Ouwerkerk wrote:
> > > I'd like more clarity on the meaning of this boolean before committing. Is
> it
> > > "operation success" or "registration state is unregistered"? When
> > unregistering
> > > the same registration twice, the first meaning could be false and the
second
> > > true, for the same end result.
> > 
> > From [1] it seems your intent is to signal whether the operation succeeded,
> not
> > whether the current state is unregistered. Please rename |unregistered| to
> > |success|.
> > 
> > [1] https://github.com/w3c/push-api/pull/97#issuecomment-67039200
> 
> If the promise is fulfilled, that means that the current state is unregistered
> and the operation succeeded. If the value is false, it means that it was not
> required to do any unregistration.

Yes, so |unregistered| is an ambiguous name. It could mean "the unregistration
call succeeded" or "the current state is unregistered". Please disambiguate it.

[Michael van Ouwerkerk, 2014-12-16 12:58:41]

lgtm but please address these comments

https://codereview.chromium.org/793403002/diff/120001/chrome/browser/services...
File chrome/browser/services/gcm/push_messaging_browsertest.cc (right):

https://codereview.chromium.org/793403002/diff/120001/chrome/browser/services...
chrome/browser/services/gcm/push_messaging_browsertest.cc:488:
ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
Sorry for the late notice, I had missed this patch set. You can now replace
lines 488-496 with:
TryToRegisterSuccessfully("1-0" /* expected_push_registration_id */);

https://codereview.chromium.org/793403002/diff/120001/chrome/browser/services...
chrome/browser/services/gcm/push_messaging_browsertest.cc:507:
ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
Same here.

https://codereview.chromium.org/793403002/diff/120001/chrome/browser/services...
chrome/browser/services/gcm/push_messaging_browsertest.cc:528:
ASSERT_TRUE(RunScript("registerServiceWorker()", &script_result));
Same here.

https://codereview.chromium.org/793403002/diff/120001/chrome/test/data/push_m...
File chrome/test/data/push_messaging/push_test.js (right):

https://codereview.chromium.org/793403002/diff/120001/chrome/test/data/push_m...
chrome/test/data/push_messaging/push_test.js:123: sendErrorToTest('no
registration');
Use sendResultToTest, there is no error object here.

[mlamouri (slow - plz ping), 2014-12-16 15:06:18]

Review comments applied.

jochen@, PTAL.

[johnme, 2014-12-16 16:31:59]

lgtm with nits (please apply the ordering comments globally, rather than just to
the specific places I noticed)

https://codereview.chromium.org/793403002/diff/160001/chrome/browser/services...
File chrome/browser/services/gcm/push_messaging_service_impl.cc (right):

https://codereview.chromium.org/793403002/diff/160001/chrome/browser/services...
chrome/browser/services/gcm/push_messaging_service_impl.cc:448: case
GCMClient::SERVER_ERROR:
I don't know if SERVER_ERROR ("Problem at the server." according to [1]) should
be considered a network error. Perhaps UNKNOWN_ERROR is more suitable?

[1]:
https://code.google.com/p/chromium/codesearch#chromium/src/components/gcm_dri...

https://codereview.chromium.org/793403002/diff/160001/chrome/browser/services...
File chrome/browser/services/gcm/push_messaging_service_impl.h (right):

https://codereview.chromium.org/793403002/diff/160001/chrome/browser/services...
chrome/browser/services/gcm/push_messaging_service_impl.h:76: void Unregister(
Please move this below RegisterFromWorker. Remember
[Register,Unregister,GetRegistration,HasPermission,Delivery].

https://codereview.chromium.org/793403002/diff/160001/content/child/push_mess...
File content/child/push_messaging/push_provider.cc (right):

https://codereview.chromium.org/793403002/diff/160001/content/child/push_mess...
content/child/push_messaging/push_provider.cc:93: void PushProvider::unregister(
Please move this below registerPushMessaging. Remember
[Register,Unregister,GetRegistration,HasPermission,Delivery].

https://codereview.chromium.org/793403002/diff/160001/content/child/push_mess...
content/child/push_messaging/push_provider.cc:119:
IPC_MESSAGE_HANDLER(PushMessagingMsg_UnregisterSuccess,
Please move this above GetPermissionStatusSuccess. Remember
[Register,Unregister,GetRegistration,HasPermission,Delivery].

https://codereview.chromium.org/793403002/diff/160001/content/public/browser/...
File content/public/browser/push_messaging_service.h (right):

https://codereview.chromium.org/793403002/diff/160001/content/public/browser/...
content/public/browser/push_messaging_service.h:71: virtual void
Unregister(const GURL& requesting_origin,
Please move this below RegisterFromWorker. Remember
[Register,Unregister,GetRegistration,HasPermission,Delivery].

https://codereview.chromium.org/793403002/diff/160001/content/public/common/p...
File content/public/common/push_messaging_status.h (right):

https://codereview.chromium.org/793403002/diff/160001/content/public/common/p...
content/public/common/push_messaging_status.h:94: enum PushUnregistrationStatus
{
Please move this above PushGetRegistrationStatus. Remember
[Register,Unregister,GetRegistration,HasPermission,Delivery].

https://codereview.chromium.org/793403002/diff/160001/content/public/common/p...
content/public/common/push_messaging_status.h:96:
PUSH_UNREGISTRATION_SUCCESS_UNREGISTER,
PUSH_UNREGISTRATION_STATUS_SUCCESS_UNREGISTERED? Similarly below.

[mlamouri (slow - plz ping), 2014-12-16 18:26:16]

Comments applied. I've also fixed the failing tests because of running a null
callback.

https://codereview.chromium.org/793403002/diff/160001/chrome/browser/services...
File chrome/browser/services/gcm/push_messaging_service_impl.cc (right):

https://codereview.chromium.org/793403002/diff/160001/chrome/browser/services...
chrome/browser/services/gcm/push_messaging_service_impl.cc:448: case
GCMClient::SERVER_ERROR:
On 2014/12/16 16:31:59, johnme wrote:
> I don't know if SERVER_ERROR ("Problem at the server." according to [1])
should
> be considered a network error. Perhaps UNKNOWN_ERROR is more suitable?
> 
> [1]:
>
https://code.google.com/p/chromium/codesearch#chromium/src/components/gcm_dri...

Done.

https://codereview.chromium.org/793403002/diff/160001/chrome/browser/services...
File chrome/browser/services/gcm/push_messaging_service_impl.h (right):

https://codereview.chromium.org/793403002/diff/160001/chrome/browser/services...
chrome/browser/services/gcm/push_messaging_service_impl.h:76: void Unregister(
On 2014/12/16 16:31:59, johnme wrote:
> Please move this below RegisterFromWorker. Remember
> [Register,Unregister,GetRegistration,HasPermission,Delivery].

Done.

https://codereview.chromium.org/793403002/diff/160001/content/child/push_mess...
File content/child/push_messaging/push_provider.cc (right):

https://codereview.chromium.org/793403002/diff/160001/content/child/push_mess...
content/child/push_messaging/push_provider.cc:93: void PushProvider::unregister(
On 2014/12/16 16:31:59, johnme wrote:
> Please move this below registerPushMessaging. Remember
> [Register,Unregister,GetRegistration,HasPermission,Delivery].

Coding style says that we have to follow declaration order in implementation.

https://codereview.chromium.org/793403002/diff/160001/content/child/push_mess...
content/child/push_messaging/push_provider.cc:119:
IPC_MESSAGE_HANDLER(PushMessagingMsg_UnregisterSuccess,
On 2014/12/16 16:31:59, johnme wrote:
> Please move this above GetPermissionStatusSuccess. Remember
> [Register,Unregister,GetRegistration,HasPermission,Delivery].

Done.

https://codereview.chromium.org/793403002/diff/160001/content/public/browser/...
File content/public/browser/push_messaging_service.h (right):

https://codereview.chromium.org/793403002/diff/160001/content/public/browser/...
content/public/browser/push_messaging_service.h:71: virtual void
Unregister(const GURL& requesting_origin,
On 2014/12/16 16:31:59, johnme wrote:
> Please move this below RegisterFromWorker. Remember
> [Register,Unregister,GetRegistration,HasPermission,Delivery].

Done.

https://codereview.chromium.org/793403002/diff/160001/content/public/common/p...
File content/public/common/push_messaging_status.h (right):

https://codereview.chromium.org/793403002/diff/160001/content/public/common/p...
content/public/common/push_messaging_status.h:94: enum PushUnregistrationStatus
{
On 2014/12/16 16:31:59, johnme wrote:
> Please move this above PushGetRegistrationStatus. Remember
> [Register,Unregister,GetRegistration,HasPermission,Delivery].

Done.

https://codereview.chromium.org/793403002/diff/160001/content/public/common/p...
content/public/common/push_messaging_status.h:96:
PUSH_UNREGISTRATION_SUCCESS_UNREGISTER,
On 2014/12/16 16:31:59, johnme wrote:
> PUSH_UNREGISTRATION_STATUS_SUCCESS_UNREGISTERED? Similarly below.

Done.

[jochen (gone - plz use gerrit), 2014-12-17 08:15:16]

lgtm

https://codereview.chromium.org/793403002/diff/200001/content/public/common/p...
File content/public/common/push_messaging_status.h (right):

https://codereview.chromium.org/793403002/diff/200001/content/public/common/p...
content/public/common/push_messaging_status.h:60:
PUSH_UNREGISTRATION_STATUS_SUCCESS_WAS_UNREGISTERED,
WAS_NOT_REGISTERED maybe?

[mlamouri (slow - plz ping), 2014-12-17 16:04:00]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-12-17 16:04:47]

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

[commit-bot: I haz the power, 2014-12-17 17:48:06]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2014-12-17 17:48:55]

Patchset 12 (id:??) landed as
https://crrev.com/2447700c2e26741e9ca3d043c3693884d7b66cf5
Cr-Commit-Position: refs/heads/master@{#308819}