ServiceWorker: Simplify callback type of service_worker_client_utils::GetClients()

TL;DR: This CL makes service_worker_client_utils::GetClients() take
blink::mojom::ServiceWorkerHost::GetClientsCallback as a callback instead of
base::OnceCallback<void(std::unique_ptr<ServiceWorkerClientPtrs>)>.

> Details

- In the original code, GetClients() uses
    std::unique_ptr<ServiceWorkerClientPtrs>
  as a container to collect client information. ServiceWorkerClientsPtrs is an
  alias of
    std::vector<blink::mojom::ServiceWorkerClientInfoPtr>>
  so this is expanded to
    std::unique_ptr<std::vector<blink::mojom::ServiceWorkerClientInfoPtr>>>
  std::vector behaves like a smart pointer, so it's not necessary to wrap
  std::vector with std::unique_ptr, that is, GetClients() can directly uses
    std::vector<blink::mojom::ServiceWorkerClientInfoPtr>>
  as the container type.

- In the original code, GetClients() takes
    base::OnceCallback<void(std::unique_ptr<ServiceWorkerClientPtrs>)>
  as a callback type. As mentioned above, this can be simplified to
    base::OnceCallback<void(std::vector<blink::mojom::ServiceWorkerClientInfoPtr>>)>
  This is equal to
    blink::mojom::ServiceWorkerHost::GetClientsCallback

Change-Id: Ie50965a5d44919207d2829c1642449e300ecc9ea
Bug: n/a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1943717
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720144}

content/browser/service_worker/service_worker_client_utils.cc

@@ -331,9 +331,10 @@ void AddWindowClient(
       container_host->create_time(), container_host->client_uuid()));
 }
 
-void AddNonWindowClient(ServiceWorkerContainerHost* container_host,
+void AddNonWindowClient(
+    ServiceWorkerContainerHost* container_host,
     blink::mojom::ServiceWorkerClientType client_type,
-                        ServiceWorkerClientPtrs* out_clients) {
+    std::vector<blink::mojom::ServiceWorkerClientInfoPtr>* out_clients) {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
   blink::mojom::ServiceWorkerClientType host_client_type =
       container_host->client_type();
@@ -362,8 +363,8 @@ void OnGetWindowClientsOnUI(
     const std::vector<std::tuple<int, int, base::TimeTicks, std::string>>&
         clients_info,
     const GURL& script_url,
-    ClientsCallback callback,
-    std::unique_ptr<ServiceWorkerClientPtrs> out_clients) {
+    blink::mojom::ServiceWorkerHost::GetClientsCallback callback,
+    std::vector<blink::mojom::ServiceWorkerClientInfoPtr> out_clients) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   for (const auto& it : clients_info) {
@@ -384,7 +385,7 @@ void OnGetWindowClientsOnUI(
     if (info->url.GetOrigin() != script_url.GetOrigin())
       continue;
 
-    out_clients->push_back(std::move(info));
+    out_clients.push_back(std::move(info));
   }
 
   RunOrPostTaskOnThread(
@@ -414,11 +415,12 @@ struct ServiceWorkerClientInfoSort {
   }
 };
 
-void DidGetClients(ClientsCallback callback,
-                   std::unique_ptr<ServiceWorkerClientPtrs> clients) {
+void DidGetClients(
+    blink::mojom::ServiceWorkerHost::GetClientsCallback callback,
+    std::vector<blink::mojom::ServiceWorkerClientInfoPtr> clients) {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
 
-  std::sort(clients->begin(), clients->end(), ServiceWorkerClientInfoSort());
+  std::sort(clients.begin(), clients.end(), ServiceWorkerClientInfoSort());
 
   std::move(callback).Run(std::move(clients));
 }
@@ -426,20 +428,19 @@ void DidGetClients(ClientsCallback callback,
 void GetNonWindowClients(
     const base::WeakPtr<ServiceWorkerVersion>& controller,
     blink::mojom::ServiceWorkerClientQueryOptionsPtr options,
-    ClientsCallback callback,
-    std::unique_ptr<ServiceWorkerClientPtrs> clients) {
+    blink::mojom::ServiceWorkerHost::GetClientsCallback callback,
+    std::vector<blink::mojom::ServiceWorkerClientInfoPtr> clients) {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
   if (!options->include_uncontrolled) {
     for (auto& controllee : controller->controllee_map())
-      AddNonWindowClient(controllee.second, options->client_type,
-                         clients.get());
+      AddNonWindowClient(controllee.second, options->client_type, &clients);
   } else if (controller->context()) {
     GURL origin = controller->script_url().GetOrigin();
     for (auto it = controller->context()->GetClientProviderHostIterator(
              origin, false /* include_reserved_clients */);
          !it->IsAtEnd(); it->Advance()) {
       AddNonWindowClient(it->GetProviderHost()->container_host(),
-                         options->client_type, clients.get());
+                         options->client_type, &clients);
     }
   }
   DidGetClients(std::move(callback), std::move(clients));
@@ -448,8 +449,8 @@ void GetNonWindowClients(
 void DidGetWindowClients(
     const base::WeakPtr<ServiceWorkerVersion>& controller,
     blink::mojom::ServiceWorkerClientQueryOptionsPtr options,
-    ClientsCallback callback,
-    std::unique_ptr<ServiceWorkerClientPtrs> clients) {
+    blink::mojom::ServiceWorkerHost::GetClientsCallback callback,
+    std::vector<blink::mojom::ServiceWorkerClientInfoPtr> clients) {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
   if (options->client_type == blink::mojom::ServiceWorkerClientType::kAll) {
     GetNonWindowClients(controller, std::move(options), std::move(callback),
@@ -459,10 +460,11 @@ void DidGetWindowClients(
   DidGetClients(std::move(callback), std::move(clients));
 }
 
-void GetWindowClients(const base::WeakPtr<ServiceWorkerVersion>& controller,
+void GetWindowClients(
+    const base::WeakPtr<ServiceWorkerVersion>& controller,
     blink::mojom::ServiceWorkerClientQueryOptionsPtr options,
-                      ClientsCallback callback,
-                      std::unique_ptr<ServiceWorkerClientPtrs> clients) {
+    blink::mojom::ServiceWorkerHost::GetClientsCallback callback,
+    std::vector<blink::mojom::ServiceWorkerClientInfoPtr> clients) {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
   DCHECK(options->client_type ==
              blink::mojom::ServiceWorkerClientType::kWindow ||
@@ -638,10 +640,10 @@ void GetClient(ServiceWorkerContainerHost* container_host,
 
 void GetClients(const base::WeakPtr<ServiceWorkerVersion>& controller,
                 blink::mojom::ServiceWorkerClientQueryOptionsPtr options,
-                ClientsCallback callback) {
+                blink::mojom::ServiceWorkerHost::GetClientsCallback callback) {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
 
-  auto clients = std::make_unique<ServiceWorkerClientPtrs>();
+  auto clients = std::vector<blink::mojom::ServiceWorkerClientInfoPtr>();
   if (!controller->HasControllee() && !options->include_uncontrolled) {
     DidGetClients(std::move(callback), std::move(clients));
     return;

content/browser/service_worker/service_worker_client_utils.h

@@ -11,6 +11,7 @@
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
 #include "third_party/blink/public/common/service_worker/service_worker_status_code.h"
+#include "third_party/blink/public/mojom/service_worker/service_worker.mojom.h"
 #include "third_party/blink/public/mojom/service_worker/service_worker_client.mojom.h"
 
 class GURL;
@@ -32,10 +33,6 @@ using NavigationCallback = base::OnceCallback<void(
     blink::mojom::ServiceWorkerClientInfoPtr client_info)>;
 using ClientCallback = base::OnceCallback<void(
     blink::mojom::ServiceWorkerClientInfoPtr client_info)>;
-using ServiceWorkerClientPtrs =
-    std::vector<blink::mojom::ServiceWorkerClientInfoPtr>;
-using ClientsCallback =
-    base::OnceCallback<void(std::unique_ptr<ServiceWorkerClientPtrs> clients)>;
 
 // The type of an opened window.
 enum class WindowType {
@@ -80,7 +77,7 @@ void GetClient(ServiceWorkerContainerHost* container_host,
 // information sorted in MRU order (most recently focused order) on completion.
 void GetClients(const base::WeakPtr<ServiceWorkerVersion>& controller,
                 blink::mojom::ServiceWorkerClientQueryOptionsPtr options,
-                ClientsCallback callback);
+                blink::mojom::ServiceWorkerHost::GetClientsCallback callback);
 
 // Finds the provider host for |origin| in |context| then uses
 // |render_process_id| and |render_process_host| to create a relevant

content/browser/service_worker/service_worker_version.cc

@@ -173,14 +173,6 @@ void DidShowPaymentHandlerWindow(
   }
 }
 
-void DidGetClients(
-    blink::mojom::ServiceWorkerHost::GetClientsCallback callback,
-    std::unique_ptr<service_worker_client_utils::ServiceWorkerClientPtrs>
-        clients) {
-  DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
-  std::move(callback).Run(std::move(*clients));
-}
-
 void DidNavigateClient(
     blink::mojom::ServiceWorkerHost::NavigateClientCallback callback,
     const GURL& url,
@@ -1193,8 +1185,7 @@ void ServiceWorkerVersion::GetClients(
     blink::mojom::ServiceWorkerClientQueryOptionsPtr options,
     GetClientsCallback callback) {
   service_worker_client_utils::GetClients(
-      weak_factory_.GetWeakPtr(), std::move(options),
-      base::BindOnce(&DidGetClients, std::move(callback)));
+      weak_factory_.GetWeakPtr(), std::move(options), std::move(callback));
 }
 
 void ServiceWorkerVersion::GetClient(const std::string& client_uuid,