ServiceWorker: coalesce all SWClientInfo request into one task.

GetClientDocuments is posting on task to the UI thread for every
clients it is currently controlling. This is coalescing the requests
into one task in order to reduce resource usage.

This also gives a better snapshot of the clients state given that
all the information are gathered during the same event loop iteration.

BUG=None

Review URL: https://codereview.chromium.org/985113002

Cr-Commit-Position: refs/heads/master@{#319572}

4 files changed, 92 insertions, 88 deletions

diff --git a/content/browser/service_worker/service_worker_provider_host.cc b/content/browser/service_worker/service_worker_provider_host.cc
index 6dd627c..e2d26bd 100644
--- a/content/browser/service_worker/service_worker_provider_host.cc
+++ b/content/browser/service_worker/service_worker_provider_host.cc
@@ -30,25 +30,6 @@ namespace content {
 
 namespace {
 
-ServiceWorkerClientInfo GetClientInfoOnUIThread(
-    int render_process_id,
-    int render_frame_id) {
-  RenderFrameHostImpl* render_frame_host =
-      RenderFrameHostImpl::FromID(render_process_id, render_frame_id);
-  if (!render_frame_host)
-    return ServiceWorkerClientInfo();
-
-  // TODO(mlamouri,michaeln): it is possible to end up collecting information
-  // for a frame that is actually being navigated and isn't exactly what we are
-  // expecting.
-  return ServiceWorkerClientInfo(
-      render_frame_host->GetVisibilityState(),
-      render_frame_host->IsFocused(),
-      render_frame_host->GetLastCommittedURL(),
-      render_frame_host->GetParent() ? REQUEST_CONTEXT_FRAME_TYPE_NESTED
-                                     : REQUEST_CONTEXT_FRAME_TYPE_TOP_LEVEL);
-}
-
 ServiceWorkerClientInfo FocusOnUIThread(
     int render_process_id,
     int render_frame_id) {
@@ -71,7 +52,8 @@ ServiceWorkerClientInfo FocusOnUIThread(
   // Move the web contents to the foreground.
   web_contents->Activate();
 
-  return GetClientInfoOnUIThread(render_process_id, render_frame_id);
+  return ServiceWorkerProviderHost::GetClientInfoOnUI(
+      render_process_id, render_frame_id);
 }
 
 }  // anonymous namespace
@@ -305,12 +287,31 @@ void ServiceWorkerProviderHost::GetClientInfo(
     const GetClientInfoCallback& callback) const {
   BrowserThread::PostTaskAndReplyWithResult(
       BrowserThread::UI, FROM_HERE,
-      base::Bind(&GetClientInfoOnUIThread,
-                render_process_id_,
-                render_frame_id_),
+      base::Bind(&ServiceWorkerProviderHost::GetClientInfoOnUI,
+                 render_process_id_,
+                 render_frame_id_),
       callback);
 }
 
+// static
+ServiceWorkerClientInfo ServiceWorkerProviderHost::GetClientInfoOnUI(
+    int render_process_id, int render_frame_id) {
+  RenderFrameHostImpl* render_frame_host =
+      RenderFrameHostImpl::FromID(render_process_id, render_frame_id);
+  if (!render_frame_host)
+    return ServiceWorkerClientInfo();
+
+  // TODO(mlamouri,michaeln): it is possible to end up collecting information
+  // for a frame that is actually being navigated and isn't exactly what we are
+  // expecting.
+  return ServiceWorkerClientInfo(
+      render_frame_host->GetVisibilityState(),
+      render_frame_host->IsFocused(),
+      render_frame_host->GetLastCommittedURL(),
+      render_frame_host->GetParent() ? REQUEST_CONTEXT_FRAME_TYPE_NESTED
+                                     : REQUEST_CONTEXT_FRAME_TYPE_TOP_LEVEL);
+}
+
 void ServiceWorkerProviderHost::AddScopedProcessReferenceToPattern(
     const GURL& pattern) {
   associated_patterns_.push_back(pattern);
diff --git a/content/browser/service_worker/service_worker_provider_host.h b/content/browser/service_worker/service_worker_provider_host.h
index a4f3704..0943e2f 100644
--- a/content/browser/service_worker/service_worker_provider_host.h
+++ b/content/browser/service_worker/service_worker_provider_host.h
@@ -155,6 +155,12 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   // Asks the renderer to send back the document information.
   void GetClientInfo(const GetClientInfoCallback& callback) const;
 
+  // Same as above but has to be called from the UI thread.
+  // It is taking the process and frame ids in parameter because |this| is meant
+  // to live on the IO thread.
+  static ServiceWorkerClientInfo GetClientInfoOnUI(int render_process_id,
+                                                   int render_frame_id);
+
   // Adds reference of this host's process to the |pattern|, the reference will
   // be removed in destructor.
   void AddScopedProcessReferenceToPattern(const GURL& pattern);
diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
index 22c6c8e..34e6773 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -36,41 +36,9 @@
 
 namespace content {
 
-typedef ServiceWorkerVersion::StatusCallback StatusCallback;
-
-class ServiceWorkerVersion::GetClientDocumentsCallback
-    : public base::RefCounted<GetClientDocumentsCallback> {
- public:
-  GetClientDocumentsCallback(int request_id,
-                             ServiceWorkerVersion* version)
-      : request_id_(request_id),
-        version_(version) {
-    DCHECK(version_);
-  }
-
-  void AddClientInfo(int client_id, const ServiceWorkerClientInfo& info) {
-    clients_.push_back(info);
-    clients_.back().client_id = client_id;
-  }
-
- private:
-  friend class base::RefCounted<GetClientDocumentsCallback>;
-
-  virtual ~GetClientDocumentsCallback() {
-    DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
-    if (version_->running_status() == RUNNING) {
-      version_->embedded_worker_->SendMessage(
-          ServiceWorkerMsg_DidGetClientDocuments(request_id_, clients_));
-    }
-  }
-
-  std::vector<ServiceWorkerClientInfo> clients_;
-  int request_id_;
-  scoped_refptr<ServiceWorkerVersion> version_;
-
-  DISALLOW_COPY_AND_ASSIGN(GetClientDocumentsCallback);
-};
+using StatusCallback = ServiceWorkerVersion::StatusCallback;
+using GetClientDocumentsCallback =
+    base::Callback<void(const std::vector<ServiceWorkerClientInfo>&)>;
 
 namespace {
 
@@ -294,6 +262,38 @@ base::TimeDelta GetTickDuration(const base::TimeTicks& time) {
   return base::TimeTicks().Now() - time;
 }
 
+void OnGetClientDocumentsFromUI(
+    // The tuple contains process_id, frame_id, client_id.
+    const std::vector<Tuple<int,int,int>>& clients_info,
+    const GURL& script_url,
+    const GetClientDocumentsCallback& callback) {
+  std::vector<ServiceWorkerClientInfo> clients;
+
+  for (const auto& it : clients_info) {
+    ServiceWorkerClientInfo info =
+        ServiceWorkerProviderHost::GetClientInfoOnUI(get<0>(it), get<1>(it));
+
+    // If the request to the provider_host returned an empty
+    // ServiceWorkerClientInfo, that means that it wasn't possible to associate
+    // it with a valid RenderFrameHost. It might be because the frame was killed
+    // or navigated in between.
+    if (info.IsEmpty())
+      continue;
+
+    // We can get info for a frame that was navigating end ended up with a
+    // different URL than expected. In such case, we should make sure to not
+    // expose cross-origin WindowClient.
+    if (info.url.GetOrigin() != script_url.GetOrigin())
+      return;
+
+    info.client_id = get<2>(it);
+    clients.push_back(info);
+  }
+
+  BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                          base::Bind(callback, clients));
+}
+
 }  // namespace
 
 ServiceWorkerVersion::ServiceWorkerVersion(
@@ -1001,18 +1001,27 @@ void ServiceWorkerVersion::OnGetClientDocuments(int request_id) {
     }
     return;
   }
-  scoped_refptr<GetClientDocumentsCallback> callback(
-      new GetClientDocumentsCallback(request_id, this));
-  ControlleeByIDMap::iterator it(&controllee_by_id_);
+
   TRACE_EVENT0("ServiceWorker",
                "ServiceWorkerVersion::OnGetClientDocuments");
-  while (!it.IsAtEnd()) {
-    // TODO(mlamouri): we could coalesce those requests into one.
-    it.GetCurrentValue()->GetClientInfo(
-        base::Bind(&ServiceWorkerVersion::DidGetClientInfo,
-                   weak_factory_.GetWeakPtr(), it.GetCurrentKey(), callback));
-    it.Advance();
+
+  std::vector<Tuple<int,int,int>> clients_info;
+  for (ControlleeByIDMap::iterator it(&controllee_by_id_); !it.IsAtEnd();
+       it.Advance()) {
+    int process_id = it.GetCurrentValue()->process_id();
+    int frame_id = it.GetCurrentValue()->frame_id();
+    int client_id = it.GetCurrentKey();
+
+    clients_info.push_back(MakeTuple(process_id, frame_id, client_id));
   }
+
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&OnGetClientDocumentsFromUI, clients_info, script_url_,
+                 base::Bind(&ServiceWorkerVersion::DidGetClientDocuments,
+                            weak_factory_.GetWeakPtr(),
+                            request_id)));
+
 }
 
 void ServiceWorkerVersion::OnActivateEventFinished(
@@ -1401,24 +1410,15 @@ void ServiceWorkerVersion::DidClaimClients(
   embedded_worker_->SendMessage(ServiceWorkerMsg_DidClaimClients(request_id));
 }
 
-void ServiceWorkerVersion::DidGetClientInfo(
-    int client_id,
-    scoped_refptr<GetClientDocumentsCallback> callback,
-    const ServiceWorkerClientInfo& info) {
-  // If the request to the provider_host returned an empty
-  // ServiceWorkerClientInfo, that means that it wasn't possible to associate
-  // it with a valid RenderFrameHost. It might be because the frame was killed
-  // or navigated in between.
-  if (info.IsEmpty())
-    return;
-
-  // We can get info for a frame that was navigating end ended up with a
-  // different URL than expected. In such case, we should make sure to not
-  // expose cross-origin WindowClient.
-  if (info.url.GetOrigin() != script_url_.GetOrigin())
+void ServiceWorkerVersion::DidGetClientDocuments(
+    int request_id,
+    const std::vector<ServiceWorkerClientInfo>& clients) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (running_status() != RUNNING)
     return;
 
-  callback->AddClientInfo(client_id, info);
+  embedded_worker_->SendMessage(
+      ServiceWorkerMsg_DidGetClientDocuments(request_id, clients));
 }
 
 void ServiceWorkerVersion::StartTimeoutTimer() {
diff --git a/content/browser/service_worker/service_worker_version.h b/content/browser/service_worker/service_worker_version.h
index bb6efbf..5911541 100644
--- a/content/browser/service_worker/service_worker_version.h
+++ b/content/browser/service_worker/service_worker_version.h
@@ -298,8 +298,6 @@ class CONTENT_EXPORT ServiceWorkerVersion
   const net::HttpResponseInfo* GetMainScriptHttpResponseInfo();
 
  private:
-  class GetClientDocumentsCallback;
-
   friend class base::RefCounted<ServiceWorkerVersion>;
   friend class ServiceWorkerURLRequestJobTest;
   FRIEND_TEST_ALL_PREFIXES(ServiceWorkerControlleeRequestHandlerTest,
@@ -389,9 +387,8 @@ class CONTENT_EXPORT ServiceWorkerVersion
 
   void DidSkipWaiting(int request_id);
   void DidClaimClients(int request_id, ServiceWorkerStatusCode status);
-  void DidGetClientInfo(int client_id,
-                        scoped_refptr<GetClientDocumentsCallback> callback,
-                        const ServiceWorkerClientInfo& info);
+  void DidGetClientDocuments(
+      int request_id, const std::vector<ServiceWorkerClientInfo>& clients);
 
   // The timeout timer periodically calls OnTimeoutTimer, which stops the worker
   // if it is excessively idle or unresponsive to ping.