ServiceWorker: Make '.ready' return a promise to be resolved with ServiceWorkerRegistration (2/3)

content/browser/service_worker/service_worker_provider_host.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_provider_host.h"
 
 #include "base/stl_util.h"
 #include "content/browser/message_port_message_filter.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_context_request_handler.h"
@@ skipping 82 matching lines @@
    if (active_version != active_version_.get()) {
      scoped_refptr<ServiceWorkerVersion> previous_version = active_version_;
      if (previous_version.get())
        previous_version->RemovePotentialControllee(this);
      if (active_version)
        active_version->AddPotentialControllee(this);
      active_version_ = active_version;
    }
 }
 
 void ServiceWorkerProviderHost::SetControllerVersionAttribute(

[michaeln, 2014/09/09 02:22:02]

if this this method remains public, can you move it up so it's position in the
.cc file matches the .h file, and add a dcheck to assert that the version
provided is either NULL or the 'active' version of the associated registration?

     ServiceWorkerVersion* version) {
   if (version == controlling_version_.get())
     return;
 
   scoped_refptr<ServiceWorkerVersion> previous_version = controlling_version_;
   controlling_version_ = version;
   if (version)
     version->AddControllee(this);
   if (previous_version.get())
     previous_version->RemoveControllee(this);
@@ skipping 23 matching lines @@
     return false;
   }
 
   running_hosted_version_ = live_version;
   return true;
 }
 
 void ServiceWorkerProviderHost::AssociateRegistration(
     ServiceWorkerRegistration* registration) {
   DCHECK(CanAssociateRegistration(registration));
+
+  if (dispatcher_host_) {
+    ServiceWorkerRegistrationHandle* handle =
+        dispatcher_host_->GetOrCreateRegistrationHandle(
+            provider_id(), registration);
+
+    ServiceWorkerVersionAttributes attrs;
+    attrs.installing = handle->CreateServiceWorkerHandleAndPass(
+        registration->installing_version());
+    attrs.waiting = handle->CreateServiceWorkerHandleAndPass(
+        registration->waiting_version());
+    attrs.active = handle->CreateServiceWorkerHandleAndPass(
+        registration->active_version());
+
+    dispatcher_host_->Send(new ServiceWorkerMsg_AssociateRegistration(
+        kDocumentMainThreadId, provider_id(), handle->GetObjectInfo(), attrs));

[nasko, 2014/09/10 01:15:56]

nit: Is this expected to change in the immediate future? If not, it is hardcoded
in all callsites, so it should be removed.

[nhiroki, 2014/09/10 01:50:47]

Good point. I think we don't have any plan to change it in the near future
(horo@, is this correct?)

Removing the constant would need more work (changing all message formats,
SWMessageFilter and so on), so for now I'd like to keep this CL as is and fix
them in a separate CL.

[nasko, 2014/09/10 04:15:10]

Fixing in another CL is fine. Please file a bug so it doesn't get forgotten.

[nhiroki, 2014/09/10 05:28:47]

I took a deeper look and found that this is not always constant. Some messages
(eg. ServiceWorkerMsg_ServiceWorkerRegistered) are set to a thread id sent from
the requester (SWDispatcher).

+  }
+
   associated_registration_ = registration;
-  registration->AddListener(this);
+  associated_registration_->AddListener(this);
   UpdatePotentialControllees(registration->installing_version(),
                              registration->waiting_version(),
                              registration->active_version());
-  SetControllerVersionAttribute(registration->active_version());
 }
 
 void ServiceWorkerProviderHost::UnassociateRegistration() {
   if (!associated_registration_.get())
     return;
+  UpdatePotentialControllees(NULL, NULL, NULL);
+  SetControllerVersionAttribute(NULL);

[michaeln, 2014/09/09 02:22:02]

does the change in order of operations matter here, just curious?

[nhiroki, 2014/09/09 08:49:08]

Just to make consistent with AssociateRegistration() (reversed order)

   associated_registration_->RemoveListener(this);
   associated_registration_ = NULL;
-  UpdatePotentialControllees(NULL, NULL, NULL);
-  SetControllerVersionAttribute(NULL);
+
+  if (dispatcher_host_) {
+    dispatcher_host_->Send(new ServiceWorkerMsg_UnassociateRegistration(
+        kDocumentMainThreadId, provider_id()));
+  }
 }
 
 scoped_ptr<ServiceWorkerRequestHandler>
 ServiceWorkerProviderHost::CreateRequestHandler(
     ResourceType resource_type,
     base::WeakPtr<storage::BlobStorageContext> blob_storage_context,
     scoped_refptr<ResourceRequestBody> body) {
   if (IsHostToRunningServiceWorker()) {
     return scoped_ptr<ServiceWorkerRequestHandler>(
         new ServiceWorkerContextRequestHandler(
@@ skipping 52 matching lines @@
     dispatcher_host_->RegisterServiceWorkerHandle(handle.Pass());
   }
   return info;
 }
 
 bool ServiceWorkerProviderHost::IsContextAlive() {
   return context_ != NULL;
 }
 
 }  // namespace content