[ServiceWorker] Implement ServiceWorkerRegistrationObject.SetVersionAttributes

BUG=769236

Change-Id: Ife84ee28396033eabe5d5b0793d895177fe1cf80
Reviewed-on: https://chromium-review.googlesource.com/732146
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512823}

content/browser/service_worker/service_worker_provider_host.cc

@@ -777,39 +777,6 @@ void ServiceWorkerProviderHost::SendUpdateFoundMessage(
       render_thread_id_, registration_handle_id));
 }
 
-void ServiceWorkerProviderHost::SendSetVersionAttributesMessage(
-    int registration_handle_id,
-    ChangedVersionAttributesMask changed_mask,
-    ServiceWorkerVersion* installing_version,
-    ServiceWorkerVersion* waiting_version,
-    ServiceWorkerVersion* active_version) {
-  if (!dispatcher_host_)
-    return;
-  if (!changed_mask.changed())
-    return;
-
-  if (!IsReadyToSendMessages()) {
-    queued_events_.push_back(base::Bind(
-        &ServiceWorkerProviderHost::SendSetVersionAttributesMessage,
-        AsWeakPtr(), registration_handle_id, changed_mask,
-        base::RetainedRef(installing_version),
-        base::RetainedRef(waiting_version), base::RetainedRef(active_version)));
-    return;
-  }
-
-  ServiceWorkerVersionAttributes attrs;
-  if (changed_mask.installing_changed())
-    attrs.installing = *GetOrCreateServiceWorkerHandle(installing_version);
-  if (changed_mask.waiting_changed())
-    attrs.waiting = *GetOrCreateServiceWorkerHandle(waiting_version);
-  if (changed_mask.active_changed())
-    attrs.active = *GetOrCreateServiceWorkerHandle(active_version);
-
-  Send(new ServiceWorkerMsg_SetVersionAttributes(
-      render_thread_id_, registration_handle_id, changed_mask.changed(),
-      attrs));
-}
-
 void ServiceWorkerProviderHost::SendServiceWorkerStateChangedMessage(
     int worker_handle_id,
     blink::mojom::ServiceWorkerState state) {

content/browser/service_worker/service_worker_provider_host.h

@@ -307,12 +307,6 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   // until the worker thread id is known via SetReadyToSendMessagesToWorker().
   void SendUpdateFoundMessage(
       int registration_handle_id);
-  void SendSetVersionAttributesMessage(
-      int registration_handle_id,
-      ChangedVersionAttributesMask changed_mask,
-      ServiceWorkerVersion* installing_version,
-      ServiceWorkerVersion* waiting_version,
-      ServiceWorkerVersion* active_version);
   void SendServiceWorkerStateChangedMessage(
       int worker_handle_id,
       blink::mojom::ServiceWorkerState state);

content/browser/service_worker/service_worker_registration_handle.cc

@@ -239,11 +239,24 @@ void ServiceWorkerRegistrationHandle::SetVersionAttributes(
     ServiceWorkerVersion* active_version) {
   if (!provider_host_)
     return;  // Could be nullptr in some tests.
-  provider_host_->SendSetVersionAttributesMessage(handle_id_,
-                                                  changed_mask,
-                                                  installing_version,
-                                                  waiting_version,
-                                                  active_version);
+  if (!changed_mask.changed())
+    return;
+
+  blink::mojom::ServiceWorkerObjectInfoPtr installing;
+  blink::mojom::ServiceWorkerObjectInfoPtr waiting;
+  blink::mojom::ServiceWorkerObjectInfoPtr active;
+  if (changed_mask.installing_changed()) {
+    installing =
+        provider_host_->GetOrCreateServiceWorkerHandle(installing_version);
+  }
+  if (changed_mask.waiting_changed())
+    waiting = provider_host_->GetOrCreateServiceWorkerHandle(waiting_version);
+  if (changed_mask.active_changed())
+    active = provider_host_->GetOrCreateServiceWorkerHandle(active_version);
+  DCHECK(remote_registration_);
+  remote_registration_->SetVersionAttributes(
+      changed_mask.changed(), std::move(installing), std::move(waiting),
+      std::move(active));
 }
 
 void ServiceWorkerRegistrationHandle::OnConnectionError() {

content/browser/service_worker/service_worker_test_utils.cc

@@ -102,6 +102,9 @@ void ServiceWorkerRemoteProviderEndpoint::BindWithProviderInfo(
   client_request_ = std::move(info->client_request);
   host_ptr_.Bind(std::move(info->host_ptr_info));
   registration_object_info_ = std::move(info->registration);
+  // To enable the caller end point to make calls safely with no need to pass
+  // |registration_object_info_->request| through a message pipe endpoint.
+  mojo::GetIsolatedInterface(registration_object_info_->request.PassHandle());
 }
 
 std::unique_ptr<ServiceWorkerProviderHost> CreateProviderHostForWindow(

content/common/service_worker/service_worker_messages.h

@@ -108,12 +108,6 @@ IPC_STRUCT_TRAITS_BEGIN(blink::mojom::ServiceWorkerObjectInfo)
   IPC_STRUCT_TRAITS_MEMBER(version_id)
 IPC_STRUCT_TRAITS_END()
 
-IPC_STRUCT_TRAITS_BEGIN(content::ServiceWorkerVersionAttributes)
-  IPC_STRUCT_TRAITS_MEMBER(installing)
-  IPC_STRUCT_TRAITS_MEMBER(waiting)
-  IPC_STRUCT_TRAITS_MEMBER(active)
-IPC_STRUCT_TRAITS_END()
-
 IPC_STRUCT_TRAITS_BEGIN(content::ServiceWorkerClientInfo)
   IPC_STRUCT_TRAITS_MEMBER(client_uuid)
   IPC_STRUCT_TRAITS_MEMBER(page_visibility_state)
@@ -244,13 +238,6 @@ IPC_MESSAGE_CONTROL3(ServiceWorkerMsg_ServiceWorkerStateChanged,
                      int /* handle_id */,
                      blink::mojom::ServiceWorkerState)
 
-// Tells the child process to set service workers for the given registration.
-IPC_MESSAGE_CONTROL4(ServiceWorkerMsg_SetVersionAttributes,
-                     int /* thread_id */,
-                     int /* registration_handle_id */,
-                     int /* changed_mask */,
-                     content::ServiceWorkerVersionAttributes)
-
 // Informs the child process that new ServiceWorker enters the installation
 // phase.
 IPC_MESSAGE_CONTROL2(ServiceWorkerMsg_UpdateFound,

content/common/service_worker/service_worker_types.h

@@ -177,12 +177,6 @@ struct CONTENT_EXPORT ServiceWorkerResponse {
   ServiceWorkerHeaderList cors_exposed_header_names;
 };
 
-struct CONTENT_EXPORT ServiceWorkerVersionAttributes {
-  blink::mojom::ServiceWorkerObjectInfo installing;
-  blink::mojom::ServiceWorkerObjectInfo waiting;
-  blink::mojom::ServiceWorkerObjectInfo active;
-};
-
 class ChangedVersionAttributesMask {
  public:
   enum {

content/common/service_worker/service_worker_types.mojom

@@ -18,9 +18,3 @@ enum ServiceWorkerProviderType {
   SERVICE_WORKER_PROVIDER_TYPE_LAST =
       SERVICE_WORKER_PROVIDER_FOR_CONTROLLER
 };
-
-// Struct for delivering the information about the installing, waiting and
-// active ServiceWorkers.
-// Defined in service_worker_types.h.
-[Native]
-struct ServiceWorkerVersionAttributes;

content/common/service_worker/service_worker_types.typemap

@@ -14,7 +14,4 @@ traits_headers = [
 sources = [
   "//content/common/service_worker/service_worker_types_struct_traits.cc",
 ]
-type_mappings = [
-  "content.mojom.ServiceWorkerProviderType=::content::ServiceWorkerProviderType",
-  "content.mojom.ServiceWorkerVersionAttributes=::content::ServiceWorkerVersionAttributes",
-]
+type_mappings = [ "content.mojom.ServiceWorkerProviderType=::content::ServiceWorkerProviderType" ]

content/renderer/service_worker/service_worker_dispatcher.cc

@@ -78,8 +78,6 @@ void ServiceWorkerDispatcher::OnMessageReceived(const IPC::Message& msg) {
                         OnSetNavigationPreloadHeaderError)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_ServiceWorkerStateChanged,
                         OnServiceWorkerStateChanged)
-    IPC_MESSAGE_HANDLER(ServiceWorkerMsg_SetVersionAttributes,
-                        OnSetVersionAttributes)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_UpdateFound,
                         OnUpdateFound)
     IPC_MESSAGE_HANDLER(ServiceWorkerMsg_CountFeature, OnCountFeature)
@@ -340,39 +338,6 @@ void ServiceWorkerDispatcher::OnServiceWorkerStateChanged(
     worker->second->OnStateChanged(state);
 }
 
-void ServiceWorkerDispatcher::OnSetVersionAttributes(
-    int thread_id,
-    int registration_handle_id,
-    int changed_mask,
-    const ServiceWorkerVersionAttributes& attrs) {
-  TRACE_EVENT1("ServiceWorker",
-               "ServiceWorkerDispatcher::OnSetVersionAttributes",
-               "Thread ID", thread_id);
-
-  // Adopt the references sent from the browser process and pass it to the
-  // registration if it exists.
-  std::unique_ptr<ServiceWorkerHandleReference> installing =
-      Adopt(attrs.installing.Clone());
-  std::unique_ptr<ServiceWorkerHandleReference> waiting =
-      Adopt(attrs.waiting.Clone());
-  std::unique_ptr<ServiceWorkerHandleReference> active =
-      Adopt(attrs.active.Clone());
-
-  RegistrationObjectMap::iterator found =
-      registrations_.find(registration_handle_id);
-  if (found != registrations_.end()) {
-    // Populate the version fields (eg. .installing) with worker objects.
-    ChangedVersionAttributesMask mask(changed_mask);
-    if (mask.installing_changed())
-      found->second->SetInstalling(
-          GetOrCreateServiceWorker(std::move(installing)));
-    if (mask.waiting_changed())
-      found->second->SetWaiting(GetOrCreateServiceWorker(std::move(waiting)));
-    if (mask.active_changed())
-      found->second->SetActive(GetOrCreateServiceWorker(std::move(active)));
-  }
-}
-
 void ServiceWorkerDispatcher::OnUpdateFound(
     int thread_id,
     int registration_handle_id) {

content/renderer/service_worker/service_worker_dispatcher.h

@@ -41,7 +41,6 @@ class ServiceWorkerProviderContext;
 class ThreadSafeSender;
 class WebServiceWorkerImpl;
 class WebServiceWorkerRegistrationImpl;
-struct ServiceWorkerVersionAttributes;
 
 // This class manages communication with the browser process about
 // registration of the service worker, exposed to renderer and worker
@@ -163,10 +162,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcher : public WorkerThread::Observer {
   void OnServiceWorkerStateChanged(int thread_id,
                                    int handle_id,
                                    blink::mojom::ServiceWorkerState state);
-  void OnSetVersionAttributes(int thread_id,
-                              int registration_handle_id,
-                              int changed_mask,
-                              const ServiceWorkerVersionAttributes& attributes);
   void OnUpdateFound(int thread_id,
                      int registration_handle_id);
   void OnCountFeature(int thread_id, int provider_id, uint32_t feature);

content/renderer/service_worker/service_worker_message_filter.cc

@@ -6,30 +6,12 @@
 
 #include <stddef.h>
 
-#include "content/child/thread_safe_sender.h"
 #include "content/common/service_worker/service_worker_messages.h"
-#include "content/common/service_worker/service_worker_types.h"
 #include "content/renderer/service_worker/service_worker_dispatcher.h"
 #include "ipc/ipc_message_macros.h"
-#include "third_party/WebKit/public/platform/modules/serviceworker/service_worker_object.mojom.h"
 
 namespace content {
 
-namespace {
-
-// Sends a ServiceWorkerObjectDestroyed message to the browser so it can delete
-// the ServiceWorker handle.
-void SendServiceWorkerObjectDestroyed(
-    ThreadSafeSender* sender,
-    int handle_id) {
-  if (handle_id == blink::mojom::kInvalidServiceWorkerHandleId)
-    return;
-  sender->Send(
-      new ServiceWorkerHostMsg_DecrementServiceWorkerRefCount(handle_id));
-}
-
-}  // namespace
-
 ServiceWorkerMessageFilter::ServiceWorkerMessageFilter(ThreadSafeSender* sender)
     : WorkerThreadMessageFilter(sender) {
 }
@@ -54,29 +36,4 @@ bool ServiceWorkerMessageFilter::GetWorkerThreadIdForMessage(
   return base::PickleIterator(msg).ReadInt(ipc_thread_id);
 }
 
-void ServiceWorkerMessageFilter::OnStaleMessageReceived(
-    const IPC::Message& msg) {
-  // Specifically handle some messages in case we failed to post task
-  // to the thread (meaning that the context on the thread is now gone).
-  IPC_BEGIN_MESSAGE_MAP(ServiceWorkerMessageFilter, msg)
-    IPC_MESSAGE_HANDLER(ServiceWorkerMsg_SetVersionAttributes,
-                        OnStaleSetVersionAttributes)
-  IPC_END_MESSAGE_MAP()
-}
-
-void ServiceWorkerMessageFilter::OnStaleSetVersionAttributes(
-    int thread_id,
-    int registration_handle_id,
-    int changed_mask,
-    const ServiceWorkerVersionAttributes& attrs) {
-  SendServiceWorkerObjectDestroyed(thread_safe_sender(),
-                                   attrs.installing.handle_id);
-  SendServiceWorkerObjectDestroyed(thread_safe_sender(),
-                                   attrs.waiting.handle_id);
-  SendServiceWorkerObjectDestroyed(thread_safe_sender(),
-                                   attrs.active.handle_id);
-  // Don't have to decrement registration refcount because the sender of the
-  // SetVersionAttributes message doesn't increment it.
-}
-
 }  // namespace content

content/renderer/service_worker/service_worker_message_filter.h

@@ -5,17 +5,12 @@
 #ifndef CONTENT_RENDERER_SERVICE_WORKER_SERVICE_WORKER_MESSAGE_FILTER_H_
 #define CONTENT_RENDERER_SERVICE_WORKER_SERVICE_WORKER_MESSAGE_FILTER_H_
 
-#include <set>
-#include <vector>
-
 #include "base/macros.h"
 #include "content/common/content_export.h"
 #include "content/renderer/worker_thread_message_filter.h"
 
 namespace content {
 
-struct ServiceWorkerVersionAttributes;
-
 class CONTENT_EXPORT ServiceWorkerMessageFilter
     : public WorkerThreadMessageFilter {
  public:
@@ -31,16 +26,6 @@ class CONTENT_EXPORT ServiceWorkerMessageFilter
   bool GetWorkerThreadIdForMessage(const IPC::Message& msg,
                                    int* ipc_thread_id) override;
 
-  // ChildMessageFilter:
-  void OnStaleMessageReceived(const IPC::Message& msg) override;
-
-  // Message handlers for stale messages.
-  void OnStaleSetVersionAttributes(
-      int thread_id,
-      int registration_handle_id,
-      int changed_mask,
-      const ServiceWorkerVersionAttributes& attrs);
-
   DISALLOW_COPY_AND_ASSIGN(ServiceWorkerMessageFilter);
 };
 

content/renderer/service_worker/web_service_worker_registration_impl.cc

@@ -12,6 +12,7 @@
 #include "content/child/child_process.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/renderer/service_worker/service_worker_dispatcher.h"
+#include "content/renderer/service_worker/service_worker_handle_reference.h"
 #include "content/renderer/service_worker/web_service_worker_impl.h"
 #include "content/renderer/service_worker/web_service_worker_provider_impl.h"
 #include "third_party/WebKit/public/platform/modules/serviceworker/WebNavigationPreloadState.h"
@@ -401,4 +402,41 @@ WebServiceWorkerRegistrationImpl::~WebServiceWorkerRegistrationImpl() {
     dispatcher->RemoveServiceWorkerRegistration(handle_id_);
 }
 
+void WebServiceWorkerRegistrationImpl::SetVersionAttributes(
+    int changed_mask,
+    blink::mojom::ServiceWorkerObjectInfoPtr installing,
+    blink::mojom::ServiceWorkerObjectInfoPtr waiting,
+    blink::mojom::ServiceWorkerObjectInfoPtr active) {
+  if (!creation_task_runner_->RunsTasksInCurrentSequence()) {
+    // As this posted task will definitely run before OnConnectionError() on the
+    // |creation_task_runner_|, using base::Unretained() here is safe.
+    creation_task_runner_->PostTask(
+        FROM_HERE,
+        base::BindOnce(&WebServiceWorkerRegistrationImpl::SetVersionAttributes,
+                       base::Unretained(this), changed_mask,
+                       std::move(installing), std::move(waiting),
+                       std::move(active)));
+    return;
+  }
+  ServiceWorkerDispatcher* dispatcher =
+      ServiceWorkerDispatcher::GetThreadSpecificInstance();
+  DCHECK(dispatcher);
+  ChangedVersionAttributesMask mask(changed_mask);
+  if (mask.installing_changed()) {
+    DCHECK(installing);
+    SetInstalling(dispatcher->GetOrCreateServiceWorker(
+        dispatcher->Adopt(std::move(installing))));
+  }
+  if (mask.waiting_changed()) {
+    DCHECK(waiting);
+    SetWaiting(dispatcher->GetOrCreateServiceWorker(
+        dispatcher->Adopt(std::move(waiting))));
+  }
+  if (mask.active_changed()) {
+    DCHECK(active);
+    SetActive(dispatcher->GetOrCreateServiceWorker(
+        dispatcher->Adopt(std::move(active))));
+  }
+}
+
 }  // namespace content

content/renderer/service_worker/web_service_worker_registration_impl.h

@@ -133,6 +133,13 @@ class CONTENT_EXPORT WebServiceWorkerRegistrationImpl
       blink::mojom::ServiceWorkerRegistrationObjectInfoPtr info);
   ~WebServiceWorkerRegistrationImpl() override;
 
+  // Implements blink::mojom::ServiceWorkerRegistrationObject.
+  void SetVersionAttributes(
+      int changed_mask,
+      blink::mojom::ServiceWorkerObjectInfoPtr installing,
+      blink::mojom::ServiceWorkerObjectInfoPtr waiting,
+      blink::mojom::ServiceWorkerObjectInfoPtr active) override;
+
   // RefCounted traits implementation, rather than delete |impl| directly, calls
   // |impl->DetachAndMaybeDestroy()| to notify that the last reference to it has
   // gone away.
@@ -160,7 +167,7 @@ class CONTENT_EXPORT WebServiceWorkerRegistrationImpl
   // -     |kDetached| --> |kAttachedAndBound|
   //   When |this| is in |kDetached| state, if an inflight
   //   ServiceWorkerRegistrationObjectInfo for the same JavaScript registration
-  //   object arrived, |this| is resued to be provided to Blink. In such a case
+  //   object arrived, |this| is reused to be provided to Blink. In such a case
   //   AttachForServiceWorkerGlobalScope() or AttachForServiceWorkerClient()
   //   sets |state_| to |kAttachedAndBound|.
   enum class LifecycleState {

third_party/WebKit/public/platform/modules/serviceworker/service_worker_registration.mojom

@@ -83,8 +83,20 @@ interface ServiceWorkerRegistrationObjectHost {
 // ServiceWorkerRegistration JavaScript object. The browser uses it to talk with
 // the corresponding impl of ServiceWorkerRegistration in the renderer.
 interface ServiceWorkerRegistrationObject {
+  // Sets changed service worker objects for this registration object.
+  // |changed_mask| indicates which objects of {|installing|,|waiting|,|active|}
+  // have changed. See ChangedVersionAttributesMask in service_worker_types.h
+  // for details.
+  // Changed objects always have non-null value, which may be invalid value
+  // describing that the object does not exist any more.
+  // Unchanged ones always have null value.
+  // TODO(leonhsl): Use mojofied ChangedVersionAttributesMask directly instead
+  // of an int32 for |changed_mask|.
+  SetVersionAttributes(int32 changed_mask,
+                       ServiceWorkerObjectInfo? installing,
+                       ServiceWorkerObjectInfo? waiting,
+                       ServiceWorkerObjectInfo? active);
+
   // TODO(leonhsl): Implement all methods.
-  // SetVersionAttributes(int32 changed_mask,
-  //                      ServiceWorkerVersionAttributes attrs);
   // UpdateFound();
 };