Implement ServiceWorkerClient attributes [2/3]

content/browser/service_worker/service_worker_version.cc

Index: content/browser/service_worker/service_worker_version.cc
diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
index f1760a0741437c088db3e0d90cc8fa1a4dc623a2..4271f3d0bdfa884db938656575d8cdb192684daf 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -23,6 +23,38 @@ namespace content {
 typedef ServiceWorkerVersion::StatusCallback StatusCallback;
 typedef ServiceWorkerVersion::MessageCallback MessageCallback;
 
+class ServiceWorkerVersion::GetClientInfoCallback {
+ public:
+  GetClientInfoCallback(int request_id)
+    : request_id_(request_id) {}
+  void add(int client_request_id, int client_id) {

[nhiroki, 2014/12/08 05:45:09]

The first letter of function names should be capitalized.

[Kunihiko Sakamoto, 2014/12/08 06:43:18]

Done.

+    pending_requests_[client_request_id] = client_id;
+  }
+  void success(int client_request_id, const ServiceWorkerClientInfo& info) {
+    auto found = pending_requests_.find(client_request_id);
+    DCHECK(found != pending_requests_.end());
+    clients_.push_back(info);
+    clients_.back().client_id = found->second;
+    pending_requests_.erase(found);
+  }
+  void error(int client_request_id) { cancel(client_request_id); }
+  void cancel(int client_request_id) {
+    auto found = pending_requests_.find(client_request_id);
+    DCHECK(found != pending_requests_.end());
+    pending_requests_.erase(found);
+  }
+  int request_id() { return request_id_; }
+  bool has_pending_requests() { return !pending_requests_.empty(); }
+  const std::vector<ServiceWorkerClientInfo>& clients() { return clients_; }
+
+  static void ClearCallbacks(IDMap<GetClientInfoCallback>* callbacks);
+
+ private:
+  std::vector<ServiceWorkerClientInfo> clients_;
+  int request_id_;
+  std::map<int, int> pending_requests_;
+};
+
 namespace {
 
 // Default delay for scheduled stop.
@@ -577,6 +609,8 @@ void ServiceWorkerVersion::OnStopped(
   RunIDMapCallbacks(&geofencing_callbacks_,
                     SERVICE_WORKER_ERROR_FAILED);
 
+  GetClientInfoCallback::ClearCallbacks(&get_client_info_callbacks_);
+
   FOR_EACH_OBSERVER(Listener, listeners_, OnWorkerStopped(this));
 
   // There should be no more communication from/to a stopped worker. Deleting
@@ -644,6 +678,10 @@ bool ServiceWorkerVersion::OnMessageReceived(const IPC::Message& message) {
                         OnPostMessageToDocument)
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_FocusClient,
                         OnFocusClient)
+    IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_GetClientInfoSuccess,
+                        OnGetClientInfoSuccess)
+    IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_GetClientInfoError,
+                        OnGetClientInfoError)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -685,19 +723,71 @@ void ServiceWorkerVersion::DispatchActivateEventAfterStartWorker(
 }
 
 void ServiceWorkerVersion::OnGetClientDocuments(int request_id) {
-  std::vector<int> client_ids;
+  if (controllee_by_id_.IsEmpty()) {
+    if (running_status() == RUNNING) {
+      embedded_worker_->SendMessage(
+          ServiceWorkerMsg_DidGetClientDocuments(request_id,
+              std::vector<ServiceWorkerClientInfo>()));
+    }
+    return;
+  }
+  GetClientInfoCallback* callback = new GetClientInfoCallback(request_id);
   ControlleeByIDMap::iterator it(&controllee_by_id_);
   TRACE_EVENT0("ServiceWorker",
                "ServiceWorkerVersion::OnGetClientDocuments");
   while (!it.IsAtEnd()) {
-    client_ids.push_back(it.GetCurrentKey());
+    int client_request_id = get_client_info_callbacks_.Add(callback);
+    callback->add(client_request_id, it.GetCurrentKey());
+    it.GetCurrentValue()->GetClientInfo(embedded_worker_->embedded_worker_id(),
+                                        client_request_id);
     it.Advance();
   }
+}
+
+void ServiceWorkerVersion::OnGetClientInfoSuccess(
+    int request_id,
+    const ServiceWorkerClientInfo& info) {
+  GetClientInfoCallback* callback =
+      get_client_info_callbacks_.Lookup(request_id);
+  if (!callback) {
+    NOTREACHED() << "Got unexpected message: " << request_id;
+    return;
+  }
+  get_client_info_callbacks_.Remove(request_id);
+
+  callback->success(request_id, info);
+  if (callback->has_pending_requests())
+    return;
+
+  // Don't bother if it's no longer running.
+  if (running_status() == RUNNING) {
+    embedded_worker_->SendMessage(
+        ServiceWorkerMsg_DidGetClientDocuments(callback->request_id(),
+                                               callback->clients()));
+  }
+  delete callback;
+}
+
+void ServiceWorkerVersion::OnGetClientInfoError(int request_id) {
+  GetClientInfoCallback* callback =
+      get_client_info_callbacks_.Lookup(request_id);
+  if (!callback) {
+    NOTREACHED() << "Got unexpected message: " << request_id;
+    return;
+  }
+  get_client_info_callbacks_.Remove(request_id);
+
+  callback->error(request_id);
+  if (callback->has_pending_requests())
+    return;
+
   // Don't bother if it's no longer running.
   if (running_status() == RUNNING) {
     embedded_worker_->SendMessage(
-        ServiceWorkerMsg_DidGetClientDocuments(request_id, client_ids));
+        ServiceWorkerMsg_DidGetClientDocuments(callback->request_id(),
+                                               callback->clients()));
   }
+  delete callback;
 }
 
 void ServiceWorkerVersion::OnActivateEventFinished(
@@ -900,4 +990,17 @@ void ServiceWorkerVersion::DoomInternal() {
   context_->storage()->PurgeResources(resources);
 }
 
+void ServiceWorkerVersion::GetClientInfoCallback::ClearCallbacks(
+    IDMap<GetClientInfoCallback>* callbacks) {
+  IDMap<GetClientInfoCallback>::iterator iter(callbacks);
+  while (!iter.IsAtEnd()) {
+    GetClientInfoCallback* callback = iter.GetCurrentValue();
+    callback->cancel(iter.GetCurrentKey());
+    if (!callback->has_pending_requests())

[nhiroki, 2014/12/08 05:45:08]

This condition seems never fulfilled because only callbacks whose
|has_pending_requests()| is greater than 0 should be in
|get_client_info_callbacks|.

Did you mean "if (callback->has_pending_requests()) delete callback;"?

[nhiroki, 2014/12/08 05:47:05]

CORRECTION: |has_pending_requests()| is **true**
 

[Kunihiko Sakamoto, 2014/12/08 06:43:18]

callback->Cancel() above decrements pending_requests_.size() by 1 and makes
HasPendingRequests() false.
Since |callbacks| may have multiple references to a GetClientInfoCallback, this
condition makes sure that we delete callback exactly once.

[nhiroki, 2014/12/08 07:09:33]

Ah sorry, I missed the cancel above. That makes sense.

+      delete callback;
+    iter.Advance();
+  }
+  callbacks->Clear();

[nhiroki, 2014/12/08 05:45:08]

After clearing |callbacks|, GetClientInfo{Success,Error} messages from documents
could reach NOTREACHED at 753 and 775... right?

[Kunihiko Sakamoto, 2014/12/08 06:43:18]

I see. Removed NOTREACHEDs.
I wonder if other callbacks that gets cleared in OnStopped() don't have the same
problem.

[nhiroki, 2014/12/08 07:09:33]

I think other event completion messages (eg. ActivateEventFinished) are sent
from a worker and they shouldn't be sent after the worker stopped.

+}
+
 }  // namespace content