[ServiceWorker] Mojofy ActivateEvent of Service Worker

content/browser/service_worker/service_worker_registration.cc

Index: content/browser/service_worker/service_worker_registration.cc
diff --git a/content/browser/service_worker/service_worker_registration.cc b/content/browser/service_worker/service_worker_registration.cc
index de9f481de5c81e77c1c9c4bee30e6a7eed0ab293..9ecdbfc2e85eedec2ea2a3eacbcdfc55ebea8ea9 100644
--- a/content/browser/service_worker/service_worker_registration.cc
+++ b/content/browser/service_worker/service_worker_registration.cc
@@ -427,9 +427,9 @@ void ServiceWorkerRegistration::DispatchActivateEvent(
       ServiceWorkerMetrics::EventType::ACTIVATE,
       base::Bind(&ServiceWorkerRegistration::OnActivateEventFinished, this,
                  activating_version));
-  activating_version
-      ->DispatchSimpleEvent<ServiceWorkerHostMsg_ActivateEventFinished>(
-          request_id, ServiceWorkerMsg_ActivateEvent(request_id));
+  activating_version->event_dispatcher()->DispatchActivateEvent(
+      base::Bind(&ServiceWorkerVersion::OnSimpleEventFinished,
+                 activating_version, request_id));

[shimazu, 2017/02/07 04:43:48]

Could you use base::Unretained(activating_version.get()) instead of passing
scoped_refptr<SWVersion> to avoid reference loop (version -> dispatcher ->
callback -> version)

[xiaofengzhang, 2017/02/07 06:15:36]

Sure. But I am a little confused that other places in this file always use
activating_version directly, such as line 426 and 342.., and in my previous
patch(https://codereview.chromium.org/2569993002/), Peter comments that I should
pass it directly for it is the scoped_refptr.

[shimazu, 2017/02/07 07:19:31]

Oops, I didn't realize that. I think base::Unretained(activating_version.get())
is better as I mentioned. (that might be confusing a little, though.)
peter@: WDYT?

[Peter Beverloo, 2017/02/08 15:13:16]

That's great context - thanks shimazu!

It would be very helpful if the Dispatch*Event methods could be annotated with a
comment that says it's safe to use callbacks pointing to an (active) version w/
base::Unretained(), and why.

Generally it's a bit scary seeing base::Unretained() in the code.

[shimazu, 2017/02/10 05:42:27]

Yeah, I also feel it's scary. 
How about having a helper method to create a callback to call
OnSimpleEventFinished with base::Unretained instead of using base::Unretained
explicitly?

==
// Creates a callback passed to event_dispatcher().
StatusCallback
ServiceWorkerVersion::CreateSimpleEventCallback(int request_id) {
  // base::Unretained(this) should be used here otherwise
  // version->event_dispatcher()->DispatchSomeEvent(callback) would have a
  // reference loop.
  return base::Bind(&ServiceWorkerVersion::OnSimpleEventFinished,
                    base::Unretained(this), request_id);
}
==

 }
 
 void ServiceWorkerRegistration::OnActivateEventFinished(