[ServiceWorker] Mojofy ActivateEvent of Service Worker

content/browser/service_worker/service_worker_job_unittest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <stdint.h>
 #include <tuple>
 
 #include "base/files/scoped_temp_dir.h"
 #include "base/logging.h"
 #include "base/macros.h"
@@ skipping 1442 matching lines @@
         activate_event_result_(blink::WebServiceWorkerEventResultCompleted) {}
 
   void OnInstallEvent(int embedded_worker_id,
                       int request_id) override {
     if (!install_callback_.is_null())
       install_callback_.Run();
     SimulateSend(new ServiceWorkerHostMsg_InstallEventFinished(
         embedded_worker_id, request_id, install_event_result_,
         has_fetch_handler_, base::Time::Now()));
   }
-  void OnActivateEvent(int embedded_worker_id, int request_id) override {
-    SimulateSend(new ServiceWorkerHostMsg_ActivateEventFinished(
-        embedded_worker_id, request_id, activate_event_result_,
-        base::Time::Now()));
+
+  void OnActivateEvent(
+      const mojom::ServiceWorkerEventDispatcher::DispatchActivateEventCallback&
+          callback) override {
+    callback.Run(
+        activate_event_result_ == blink::WebServiceWorkerEventResultCompleted
+            ? SERVICE_WORKER_OK
+            : SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED,

[Peter Beverloo, 2017/02/14 15:48:13]

shimazu@ - it may make sense to generalize EventResultToStatus() somewhere.
There's lots of code doing exactly the same conversions, often without checking
for invalid values.

[shimazu, 2017/02/15 01:29:02]

I agree that we have too many duplicated codes for the conversion.. I feel great
if we can remove the usage of SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED and
SERVICE_WORKER_OK and use blink::WebServiceWorkerEventResult instead. Now we can
use the public blink type in content/browser/. 
It should be in the separated patch anyway. I think this patch is good as is.

Thanks for pointing it out! :)

[xiaofengzhang, 2017/02/15 01:59:46]

thanks a lot for both comments. :-)

[falken, 2017/02/15 07:32:03]

Are you sure this is possible? Don't callbacks to install and activate expect to
get more status codes like ERROR_NETWORK or ERROR_TIMEOUT?

I think we should write cleanly now with what we have and not expect future code
cleanup to necessarily happen.

[shimazu, 2017/02/15 08:25:34]

I see. I simply assumed that it's possible because all of "SimpleEvent" take
only OK/REJECTED, but that seems a bit complex (and I've recalled now that I
changed it).
EventResultToStatus is basically used only in SWContextCore except for this unit
test and callbacks for the legacy IPC, so I think it's okay to have
EventResultToStatus in this file too.

[xiaofengzhang, 2017/02/16 06:17:50]

Done.

+        base::Time::Now());
   }
 
   void set_install_callback(const base::Closure& callback) {
     install_callback_ = callback;
   }
   void set_install_event_result(blink::WebServiceWorkerEventResult result) {
     install_event_result_ = result;
   }
   void set_activate_event_result(blink::WebServiceWorkerEventResult result) {
     activate_event_result_ = result;
@@ skipping 249 matching lines @@
   // should not be promoted to ACTIVATED because failure occur
   // during shutdown.
   runner->RunUntilIdle();
   base::RunLoop().RunUntilIdle();
   EXPECT_EQ(new_version.get(), registration->active_version());
   EXPECT_EQ(ServiceWorkerVersion::ACTIVATING, new_version->status());
   registration->RemoveListener(update_helper);
 }
 
 }  // namespace content