[ServiceWorker] Mojofy PushEvent of Service Worker.

content/browser/push_messaging/push_messaging_router.cc

Index: content/browser/push_messaging/push_messaging_router.cc
diff --git a/content/browser/push_messaging/push_messaging_router.cc b/content/browser/push_messaging/push_messaging_router.cc
index f6ff797ff2540b972d03d8e9a54fe25623969157..584dfa0f34cfc63a3d2028d831f7c4e9a613374c 100644
--- a/content/browser/push_messaging/push_messaging_router.cc
+++ b/content/browser/push_messaging/push_messaging_router.cc
@@ -123,8 +123,14 @@ void PushMessagingRouter::DeliverMessageToWorker(
                  deliver_message_callback, service_worker_registration),
       base::TimeDelta::FromSeconds(kPushMessageTimeoutSeconds),
       ServiceWorkerVersion::KILL_ON_TIMEOUT);
-  service_worker->DispatchSimpleEvent<ServiceWorkerHostMsg_PushEventFinished>(
-      request_id, ServiceWorkerMsg_PushEvent(request_id, payload));
+
+  // |event_dispatcher| is owned by |service_worker|, once |service_worker|
+  // got destroyed, the bound function will never be called, so it is safe to
+  // use

[shimazu, 2016/12/14 04:42:34]

nit: please fix indent

[xiaofengzhang, 2016/12/15 01:30:49]

Acknowledged.

+  // base::Unretained() here.
+  service_worker->event_dispatcher()->DispatchPushEvent(
+      payload, base::Bind(&ServiceWorkerVersion::OnSimpleEventFinished,
+                          base::Unretained(service_worker.get()), request_id));

[Peter Beverloo, 2016/12/13 13:31:32]

Instead of using base::Unretained(), just pass |service_worker| directly. It's a
scoped_refptr so that should work just fine, and that allows you to drop the
comment.

Ideally this entire class would stop using const& for the scoped_refptr<>
arguments. That would allow us to use std::move() instead of making copies. But
that's a clean-up for another time :).

[leonhsl(Using Gerrit), 2016/12/14 03:29:18]

I have one question which I'm not so sure: This way the bound callback would
hold an additional reference on |service_worker|, then only after this callback
got fired or destroyed, |service_worker| would got chance to be destroyed, this
would be a little different with previous scenario of |service_worker|
lifecycle?

[xiaofengzhang, 2016/12/15 01:30:48]

I re-submit and pass |service_worker| directly, but please also see leon's
question.
For "stop using const& for the scoped_refptr", yes, I will do it in another next
cl. Thanks :)

[xiaofengzhang, 2016/12/16 01:34:48]

Any comments on Leon's question:
"I have one question which I'm not so sure: This way the bound callback would
hold an additional reference on |service_worker|, then only after this callback
got fired or destroyed, |service_worker| would got chance to be destroyed, this
would be a little different with previous scenario of |service_worker|
lifecycle?"

 }
 
 // static