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

content/child/service_worker/web_service_worker_provider_impl.cc

Index: content/child/service_worker/web_service_worker_provider_impl.cc
diff --git a/content/child/service_worker/web_service_worker_provider_impl.cc b/content/child/service_worker/web_service_worker_provider_impl.cc
index ca4cfc7ba37eb461362657261e8ee6872ef7753a..4f8d005677ae7cea7c054948b7db017d22011667 100644
--- a/content/child/service_worker/web_service_worker_provider_impl.cc
+++ b/content/child/service_worker/web_service_worker_provider_impl.cc
@@ -10,7 +10,9 @@
 #include "content/child/service_worker/service_worker_dispatcher.h"
 #include "content/child/service_worker/service_worker_handle_reference.h"
 #include "content/child/service_worker/service_worker_provider_context.h"
+#include "content/child/service_worker/service_worker_registration_handle_reference.h"
 #include "content/child/service_worker/web_service_worker_impl.h"
+#include "content/child/service_worker/web_service_worker_registration_impl.h"
 #include "content/child/thread_safe_sender.h"
 #include "content/common/service_worker/service_worker_messages.h"
 #include "third_party/WebKit/public/platform/WebServiceWorkerProviderClient.h"
@@ -47,19 +49,24 @@ void WebServiceWorkerProviderImpl::setClient(
   // for more context)
   GetDispatcher()->AddScriptClient(provider_id_, client);
 
-  if (context_->installing_handle_id() != kInvalidServiceWorkerHandleId) {
-    client->setInstalling(GetDispatcher()->GetServiceWorker(
-        context_->installing()->info(), false));
-  }
-
-  if (context_->waiting_handle_id() != kInvalidServiceWorkerHandleId) {
-    client->setWaiting(GetDispatcher()->GetServiceWorker(
-        context_->waiting()->info(), false));
-  }
-
   if (context_->active_handle_id() != kInvalidServiceWorkerHandleId) {
-    client->setActive(GetDispatcher()->GetServiceWorker(
+    WebServiceWorkerRegistrationImpl* registration =
+        GetDispatcher()->GetServiceWorkerRegistration(
+            context_->registration()->info(), false);

[michaeln, 2014/09/04 00:27:23]

I know this is because things are in an intermediary state, but this is kinda
bizarre reading code. Here its assuming that if active_handle_id then there must
be a reg->info. Ok, if you say so?

[nhiroki, 2014/09/08 15:55:52]

Straightened the logic in the latest patchset. How does it look to you?

+
+    if (context_->installing_handle_id() != kInvalidServiceWorkerHandleId) {
+      registration->SetInstalling(GetDispatcher()->GetServiceWorker(

[michaeln, 2014/09/04 00:27:23]

Registration may be a preexising object accoding to the impl, yet here we'll
overwrite whatever it may contain but only if we're over writing with non-null
values.

This is all so arbitrary over here in the renderer-side of things.

[nhiroki, 2014/09/08 15:55:52]

Updated. The latest patchset just reuses the existing registration without any
overwrites if possible.

+          context_->installing()->info(), false));
+    }
+    if (context_->waiting_handle_id() != kInvalidServiceWorkerHandleId) {
+      registration->SetWaiting(GetDispatcher()->GetServiceWorker(
+          context_->waiting()->info(), false));
+    }
+    registration->SetActive(GetDispatcher()->GetServiceWorker(
         context_->active()->info(), false));
+
+    client->setReady(GetDispatcher()->GetServiceWorkerRegistration(
+        context_->registration()->info(), false));

[michaeln, 2014/09/04 00:27:23]

Is there a reason to call GetServiceWorkerRegistration() again or could you use
the |registration| local variable? Sounds like that may be a rhetorical
question, but its not, the client side of this makes no sense to me.

[nhiroki, 2014/09/08 15:55:52]

Oops... that's just my mistake. The local variable must be ok. Sorry for making
you confused.

   }
 
   if (context_->controller_handle_id() != kInvalidServiceWorkerHandleId) {