[ServiceWorker] Mojofy InstallEvent of Service Worker

content/browser/service_worker/service_worker_register_job.cc

 // Copyright 2013 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 "content/browser/service_worker/service_worker_register_job.h"
 
 #include <stdint.h>
 
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "content/browser/service_worker/embedded_worker_status.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_job_coordinator.h"
 #include "content/browser/service_worker/service_worker_metrics.h"
 #include "content/browser/service_worker/service_worker_registration.h"
 #include "content/browser/service_worker/service_worker_storage.h"
 #include "content/browser/service_worker/service_worker_write_to_cache_job.h"
+#include "content/common/service_worker/service_worker_event_dispatcher.mojom.h"
 #include "content/common/service_worker/service_worker_messages.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/common/service_worker/service_worker_utils.h"
 #include "content/public/browser/browser_thread.h"
+#include "mojo/public/cpp/bindings/strong_associated_binding.h"
 #include "net/base/net_errors.h"
 
 namespace content {
 
 namespace {
 
 void RunSoon(const base::Closure& closure) {
   base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, closure);
 }
 
@@ skipping 36 matching lines @@
       weak_factory_(this) {
   internal_.registration = registration;
 }
 
 ServiceWorkerRegisterJob::~ServiceWorkerRegisterJob() {
   DCHECK(!context_ ||
          phase_ == INITIAL || phase_ == COMPLETE || phase_ == ABORT)
       << "Jobs should only be interrupted during shutdown.";
 }
 
+class ServiceWorkerRegisterJob::InstallEventMethodsReceiver

[shimazu, 2017/04/24 05:00:13]

This is not necessary to be an inner class of ServcieWorkerRegisterJob. I think
you can implement this in unnamed namespace with passing a raw pointer to the
SWVersion in the constructor.

[xiaofengzhang, 2017/04/25 06:01:29]

Done.

+    : public mojom::ServiceWorkerInstallEventMethods {
+ public:
+  static mojom::ServiceWorkerInstallEventMethodsAssociatedPtrInfo CreateAndBind(
+      const base::WeakPtr<ServiceWorkerRegisterJob>& job) {
+    mojom::ServiceWorkerInstallEventMethodsAssociatedPtrInfo ptr_info;
+    auto request = mojo::MakeRequest(&ptr_info);
+    mojo::MakeStrongAssociatedBinding(

[shimazu, 2017/04/24 05:00:13]

I feel managing its lifecycle in browser process is better to understand the
behavior, so I prefer normal AssociatedBinding for this receiver.
WDYT?

[leonhsl(Using Gerrit), 2017/04/24 05:34:26]

In some cases the callback(OnInstallFinished()) of install event may never be
called because of unexpected disconnection of EventDispatcher mojo interface.
For example, after dispatched Install event, the browser side timer out
happened, ServiceWorkerVersion::OnTimeoutTimer() will close EventDispatcher mojo
connection, or sometimes renderer process crashed unexpectedly. 
With StrongAssociatedBinding, the InstallEventMethodsReceiver instance still can
be destroyed correctly even in above cases.

[shimazu, 2017/04/24 06:22:18]

Thanks for explanation! It makes sense.

If we have the instance as an unique_ptr in the callback of EventDispatcher, it
will be also discarded correctly. 
It's like the following in short:
event_dispatcher->DispatchEvent(base::Bind(&Callback,
base::MakeUnique<Receiver>()));


I prefer having a receiver instance in the callback because this pattern ensures
the lifecycle matches to the event_dispatcher comparing to waiting for a message
of "pipe has been closed" asynchronously. 

[leonhsl(Using Gerrit), 2017/04/24 06:37:38]

Oh I see.. I think you're right, even if EventDispatcher mojo interface
disconnected unexpectedly, although the install callback can not be called, but
mojo will destroy it, so that the base::Passed() unique_ptr could also be
destroyed as well. Thanks!

[xiaofengzhang, 2017/04/25 06:01:29]

Done.

+        base::WrapUnique(new InstallEventMethodsReceiver(job)),

[leonhsl(Using Gerrit), 2017/04/23 03:33:10]

Use base::MakeUnique instead.

[xiaofengzhang, 2017/04/24 05:02:40]

Done.

+        std::move(request));
+    return ptr_info;
+  }
+
+  InstallEventMethodsReceiver(
+      const base::WeakPtr<ServiceWorkerRegisterJob>& job)
+      : job_(job) {}
+  ~InstallEventMethodsReceiver() override {}
+
+  // mojom::ServiceWorkerInstallEventMethod implementation.
+  void RegisterForeignFetchScopes(
+      const std::vector<GURL>& sub_scopes,
+      const std::vector<url::Origin>& origins) override {
+    if (!job_)
+      return;
+    job_->new_version()->RegisterForeignFetchScopes(sub_scopes, origins);
+  }
+
+ private:
+  base::WeakPtr<ServiceWorkerRegisterJob> job_;
+};
+
 void ServiceWorkerRegisterJob::AddCallback(
     const RegistrationCallback& callback,
     ServiceWorkerProviderHost* provider_host) {
   if (!is_promise_resolved_) {
     callbacks_.push_back(callback);
     if (provider_host)
       provider_host->AddScopedProcessReferenceToPattern(pattern_);
     return;
   }
   RunSoon(base::Bind(callback, promise_resolved_status_,
@@ skipping 334 matching lines @@
 
 void ServiceWorkerRegisterJob::DispatchInstallEvent() {
   DCHECK_EQ(ServiceWorkerVersion::INSTALLING, new_version()->status())
       << new_version()->status();
   DCHECK_EQ(EmbeddedWorkerStatus::RUNNING, new_version()->running_status())
       << "Worker stopped too soon after it was started.";
   int request_id = new_version()->StartRequest(
       ServiceWorkerMetrics::EventType::INSTALL,
       base::Bind(&ServiceWorkerRegisterJob::OnInstallFailed,
                  weak_factory_.GetWeakPtr()));
-  new_version()
-      ->RegisterRequestCallback<ServiceWorkerHostMsg_InstallEventFinished>(
-          request_id, base::Bind(&ServiceWorkerRegisterJob::OnInstallFinished,
-                                 weak_factory_.GetWeakPtr()));
-  new_version()->DispatchEvent({request_id},
-                               ServiceWorkerMsg_InstallEvent(request_id));
+
+  new_version()->event_dispatcher()->DispatchInstallEvent(
+      InstallEventMethodsReceiver::CreateAndBind(weak_factory_.GetWeakPtr()),
+      base::Bind(&ServiceWorkerRegisterJob::OnInstallFinished,

[shimazu, 2017/04/24 05:00:13]

To clarify InstallEventMethodsReceiver is valid only between
DispatchInstallEvent and OnInstallFinished, how about passing the receiver
instance to OnInstallFinished and remove it in OnInstallFinished?

ex)

  ...
  auto install_event_methods =
base::MakeUnique<InstallEventMethodsReceiver>(...);
  new_version()->event_dispatcher()->DispatchInstallEvent(
    install_event_methods.CreateAndBindInterface(),
    base::Bind(&ServiceWorkerRegisterJob::OnInstallFinished,
               weak_factory_.GetWeakPtr(), request_id, 
               base::Passed(&install_event_methods)));
}

void ServiceWorkerRegisterJob::OnInstallFinished(
    int request_id,
    std::unique_ptr<InstallEventMethodsReceiver> instal_event_methods,
    ...) { 
  install_event_methods.reset();
  ...
}

[xiaofengzhang, 2017/04/25 06:01:28]

Done.

+                 weak_factory_.GetWeakPtr(), request_id));
 }
 
 void ServiceWorkerRegisterJob::OnInstallFinished(
     int request_id,
-    blink::WebServiceWorkerEventResult result,
+    ServiceWorkerStatusCode status,
     bool has_fetch_handler,
     base::Time dispatch_event_time) {
-  new_version()->FinishRequest(
-      request_id, result == blink::kWebServiceWorkerEventResultCompleted,
-      dispatch_event_time);
-
-  ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
-  switch (result) {
-    case blink::kWebServiceWorkerEventResultCompleted:
-      status = SERVICE_WORKER_OK;
-      break;
-    case blink::kWebServiceWorkerEventResultRejected:
-      status = SERVICE_WORKER_ERROR_EVENT_WAITUNTIL_REJECTED;
-      break;
-    default:
-      NOTREACHED();
-  }
+  new_version()->FinishRequest(request_id, status == SERVICE_WORKER_OK,
+                               dispatch_event_time);
 
   if (status != SERVICE_WORKER_OK) {
     OnInstallFailed(status);
     return;
   }
 
   ServiceWorkerMetrics::RecordInstallEventStatus(status);
   ServiceWorkerMetrics::RecordForeignFetchRegistrationCount(
       new_version()->foreign_fetch_scopes().size(),
       new_version()->foreign_fetch_origins().size());
@@ skipping 176 matching lines @@
       new_version()->force_bypass_cache_for_scripts() ||
       registration()->last_update_check().is_null()) {
     registration()->set_last_update_check(base::Time::Now());
 
     if (registration()->has_installed_version())
       context_->storage()->UpdateLastUpdateCheckTime(registration());
   }
 }
 
 }  // namespace content