[OnionSoup] Remove WebServiceWorkerClientCallbacks - part 2

After https://chromium-review.googlesource.com/c/chromium/src/+/1214709,
now we send {FocusClient, NavigateClient, OpenPaymentHandlerWindow,
OpenWindowForClients} Mojo messages from within Blink, no longer need
blink::NavigateClientCallback (subclass of
WebServiceWorkerClientCallbacks) to help cross the boundary of Content
and Blink.

This CL removes blink::NavigateClientCallback then removes
the parent class WebServiceWorkerClientCallbacks completely.

BUG=879019

Change-Id: I0643b7439d1a07ca2a3296f3327440d306cf0c2f
Reviewed-on: https://chromium-review.googlesource.com/1227619Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Leon Han <leon.han@intel.com>
Cr-Commit-Position: refs/heads/master@{#593054}

third_party/blink/public/platform/modules/service_worker/web_service_worker_clients_info.h

@@ -8,16 +8,16 @@
 #include "services/network/public/mojom/request_context_frame_type.mojom-shared.h"
 #include "third_party/blink/public/mojom/page/page_visibility_state.mojom-shared.h"
 #include "third_party/blink/public/mojom/service_worker/service_worker_client.mojom-shared.h"
-#include "third_party/blink/public/platform/web_callbacks.h"
+#include "third_party/blink/public/platform/web_string.h"
 #include "third_party/blink/public/platform/web_url.h"
-#include "third_party/blink/public/platform/web_url_request.h"
-
-#include <memory>
 
 namespace blink {
 
-struct WebServiceWorkerError;
-
+// The only usage of this class is to carry the source of an extendable message
+// dispatched via content.mojom.ServiceWorker across the boundary of Content and
+// Blink.
+// TODO(crbug.com/879019): Remove this class once we move
+// content.mojom.ServiceWorker impl into Blink.
 struct WebServiceWorkerClientInfo {
   WebServiceWorkerClientInfo()
       : page_visibility_state(mojom::PageVisibilityState::kMaxValue),
@@ -34,10 +34,6 @@ struct WebServiceWorkerClientInfo {
   mojom::ServiceWorkerClientType client_type;
 };
 
-using WebServiceWorkerClientCallbacks =
-    WebCallbacks<std::unique_ptr<WebServiceWorkerClientInfo>,
-                 const WebServiceWorkerError&>;
-
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_PUBLIC_PLATFORM_MODULES_SERVICE_WORKER_WEB_SERVICE_WORKER_CLIENTS_INFO_H_

third_party/blink/renderer/modules/payments/can_make_payment_event.cc

@@ -10,7 +10,6 @@
 #include "third_party/blink/renderer/core/workers/worker_location.h"
 #include "third_party/blink/renderer/modules/service_worker/respond_with_observer.h"
 #include "third_party/blink/renderer/modules/service_worker/service_worker_global_scope_client.h"
-#include "third_party/blink/renderer/modules/service_worker/service_worker_window_client_callback.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"
 

third_party/blink/renderer/modules/payments/payment_request_event.cc

@@ -13,7 +13,6 @@
 #include "third_party/blink/renderer/core/workers/worker_location.h"
 #include "third_party/blink/renderer/modules/service_worker/respond_with_observer.h"
 #include "third_party/blink/renderer/modules/service_worker/service_worker_global_scope_client.h"
-#include "third_party/blink/renderer/modules/service_worker/service_worker_window_client_callback.h"
 #include "third_party/blink/renderer/platform/bindings/script_state.h"
 #include "third_party/blink/renderer/platform/wtf/text/atomic_string.h"
 
@@ -104,7 +103,7 @@ ScriptPromise PaymentRequestEvent::openWindow(ScriptState* script_state,
   context->ConsumeWindowInteraction();
 
   ServiceWorkerGlobalScopeClient::From(context)->OpenWindowForPaymentHandler(
-      parsed_url_to_open, std::make_unique<NavigateClientCallback>(resolver));
+      parsed_url_to_open, resolver);
   return promise;
 }
 

third_party/blink/renderer/modules/service_worker/BUILD.gn

@@ -54,8 +54,6 @@ blink_modules_sources("service_worker") {
     "service_worker_thread.h",
     "service_worker_window_client.cc",
     "service_worker_window_client.h",
-    "service_worker_window_client_callback.cc",
-    "service_worker_window_client_callback.h",
     "thread_safe_script_container.cc",
     "thread_safe_script_container.h",
     "wait_until_observer.cc",

third_party/blink/renderer/modules/service_worker/service_worker_clients.cc

@@ -21,7 +21,6 @@
 #include "third_party/blink/renderer/modules/service_worker/service_worker_error.h"
 #include "third_party/blink/renderer/modules/service_worker/service_worker_global_scope_client.h"
 #include "third_party/blink/renderer/modules/service_worker/service_worker_window_client.h"
-#include "third_party/blink/renderer/modules/service_worker/service_worker_window_client_callback.h"
 #include "third_party/blink/renderer/platform/bindings/v8_throw_exception.h"
 #include "third_party/blink/renderer/platform/wtf/vector.h"
 
@@ -52,7 +51,7 @@ void DidGetClient(ScriptPromiseResolver* resolver,
     resolver->Resolve();
     return;
   }
-  ServiceWorkerClient* client;
+  ServiceWorkerClient* client = nullptr;
   switch (info->client_type) {
     case mojom::ServiceWorkerClientType::kWindow:
       client = ServiceWorkerWindowClient::Create(*info);
@@ -182,7 +181,7 @@ ScriptPromise ServiceWorkerClients::openWindow(ScriptState* script_state,
   context->ConsumeWindowInteraction();
 
   ServiceWorkerGlobalScopeClient::From(context)->OpenWindowForClients(
-      parsed_url, std::make_unique<NavigateClientCallback>(resolver));
+      parsed_url, resolver);
   return promise;
 }
 

third_party/blink/renderer/modules/service_worker/service_worker_global_scope_client.cc

@@ -36,87 +36,44 @@
 #include "third_party/blink/public/platform/modules/service_worker/web_service_worker_error.h"
 #include "third_party/blink/public/platform/modules/service_worker/web_service_worker_response.h"
 #include "third_party/blink/public/web/modules/service_worker/web_service_worker_context_client.h"
+#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
 #include "third_party/blink/renderer/core/fetch/response.h"
 #include "third_party/blink/renderer/core/workers/worker_global_scope.h"
+#include "third_party/blink/renderer/modules/service_worker/service_worker_window_client.h"
+#include "third_party/blink/renderer/platform/bindings/v8_throw_exception.h"
+#include "third_party/blink/renderer/platform/heap/persistent.h"
 #include "third_party/blink/renderer/platform/wtf/vector.h"
 
 namespace blink {
 
 namespace {
 
-// TODO(leonhsl): Remove this function, pass ServiceWorkerClientInfoPtr through
-// to replace WebServiceWorkerClientInfo.
-blink::WebServiceWorkerClientInfo ToWebServiceWorkerClientInfo(
-    mojom::blink::ServiceWorkerClientInfoPtr client_info) {
-  DCHECK(!client_info->client_uuid.IsEmpty());
-
-  blink::WebServiceWorkerClientInfo web_client_info;
-
-  web_client_info.uuid = client_info->client_uuid;
-  web_client_info.page_visibility_state = client_info->page_visibility_state;
-  web_client_info.is_focused = client_info->is_focused;
-  web_client_info.url = client_info->url;
-  web_client_info.frame_type = client_info->frame_type;
-  web_client_info.client_type = client_info->client_type;
-
-  return web_client_info;
-}
-
-void DidOpenWindow(
-    std::unique_ptr<blink::WebServiceWorkerClientCallbacks> callbacks,
-    bool success,
-    mojom::blink::ServiceWorkerClientInfoPtr client,
-    const String& error_msg) {
-  if (!success) {
-    DCHECK(!client);
-    DCHECK(!error_msg.IsNull());
-    callbacks->OnError(blink::WebServiceWorkerError(
-        mojom::blink::ServiceWorkerErrorType::kNavigation, error_msg));
+void DidNavigateOrOpenWindow(ScriptPromiseResolver* resolver,
+                             bool success,
+                             mojom::blink::ServiceWorkerClientInfoPtr info,
+                             const String& error_msg) {
+  if (!resolver->GetExecutionContext() ||
+      resolver->GetExecutionContext()->IsContextDestroyed()) {
     return;
   }
 
-  std::unique_ptr<blink::WebServiceWorkerClientInfo> web_client;
-  if (client) {
-    web_client = std::make_unique<blink::WebServiceWorkerClientInfo>(
-        ToWebServiceWorkerClientInfo(std::move(client)));
-  }
-  callbacks->OnSuccess(std::move(web_client));
-}
-
-void DidFocusClient(
-    std::unique_ptr<blink::WebServiceWorkerClientCallbacks> callbacks,
-    mojom::blink::ServiceWorkerClientInfoPtr client) {
-  if (!client) {
-    callbacks->OnError(blink::WebServiceWorkerError(
-        mojom::blink::ServiceWorkerErrorType::kNotFound,
-        "The client was not found."));
-    return;
-  }
-  auto web_client = std::make_unique<blink::WebServiceWorkerClientInfo>(
-      ToWebServiceWorkerClientInfo(std::move(client)));
-  callbacks->OnSuccess(std::move(web_client));
-}
-
-void DidNavigateClient(
-    std::unique_ptr<blink::WebServiceWorkerClientCallbacks> callbacks,
-    bool success,
-    mojom::blink::ServiceWorkerClientInfoPtr client,
-    const String& error_msg) {
   if (!success) {
-    DCHECK(!client);
+    DCHECK(!info);
     DCHECK(!error_msg.IsNull());
-    callbacks->OnError(blink::WebServiceWorkerError(
-        mojom::blink::ServiceWorkerErrorType::kNavigation, error_msg));
+    ScriptState::Scope scope(resolver->GetScriptState());
+    resolver->Reject(V8ThrowException::CreateTypeError(
+        resolver->GetScriptState()->GetIsolate(), error_msg));
     return;
   }
-
-  std::unique_ptr<blink::WebServiceWorkerClientInfo> web_client;
-  if (client) {
-    web_client = std::make_unique<blink::WebServiceWorkerClientInfo>(
-        ToWebServiceWorkerClientInfo(std::move(client)));
-  }
-  callbacks->OnSuccess(std::move(web_client));
+  ServiceWorkerWindowClient* window_client = nullptr;
+  // Even if the open/navigation succeeded, |info| may be null if information of
+  // the opened/navigated window could not be obtained (this can happen for a
+  // cross-origin window, or if the browser process could not get the
+  // information in time before the window was closed).
+  if (info)
+    window_client = ServiceWorkerWindowClient::Create(*info);
+  resolver->Resolve(window_client);
 }
 
 }  // namespace
@@ -138,18 +95,16 @@ void ServiceWorkerGlobalScopeClient::GetClients(
 
 void ServiceWorkerGlobalScopeClient::OpenWindowForClients(
     const KURL& url,
-    std::unique_ptr<WebServiceWorkerClientCallbacks> callbacks) {
-  DCHECK(callbacks);
+    ScriptPromiseResolver* resolver) {
   service_worker_host_->OpenNewTab(
-      url, WTF::Bind(&DidOpenWindow, std::move(callbacks)));
+      url, WTF::Bind(&DidNavigateOrOpenWindow, WrapPersistent(resolver)));
 }
 
 void ServiceWorkerGlobalScopeClient::OpenWindowForPaymentHandler(
     const KURL& url,
-    std::unique_ptr<WebServiceWorkerClientCallbacks> callbacks) {
-  DCHECK(callbacks);
+    ScriptPromiseResolver* resolver) {
   service_worker_host_->OpenPaymentHandlerWindow(
-      url, WTF::Bind(&DidOpenWindow, std::move(callbacks)));
+      url, WTF::Bind(&DidNavigateOrOpenWindow, WrapPersistent(resolver)));
 }
 
 void ServiceWorkerGlobalScopeClient::SetCachedMetadata(const KURL& url,
@@ -178,21 +133,17 @@ void ServiceWorkerGlobalScopeClient::Claim(ClaimCallback callback) {
   service_worker_host_->ClaimClients(std::move(callback));
 }
 
-void ServiceWorkerGlobalScopeClient::Focus(
-    const String& client_uuid,
-    std::unique_ptr<WebServiceWorkerClientCallbacks> callbacks) {
-  DCHECK(callbacks);
-  service_worker_host_->FocusClient(
-      client_uuid, WTF::Bind(&DidFocusClient, std::move(callbacks)));
+void ServiceWorkerGlobalScopeClient::Focus(const String& client_uuid,
+                                           FocusCallback callback) {
+  service_worker_host_->FocusClient(client_uuid, std::move(callback));
 }
 
-void ServiceWorkerGlobalScopeClient::Navigate(
-    const String& client_uuid,
-    const KURL& url,
-    std::unique_ptr<WebServiceWorkerClientCallbacks> callbacks) {
-  DCHECK(callbacks);
+void ServiceWorkerGlobalScopeClient::Navigate(const String& client_uuid,
+                                              const KURL& url,
+                                              ScriptPromiseResolver* resolver) {
   service_worker_host_->NavigateClient(
-      client_uuid, url, WTF::Bind(&DidNavigateClient, std::move(callbacks)));
+      client_uuid, url,
+      WTF::Bind(&DidNavigateOrOpenWindow, WrapPersistent(resolver)));
 }
 
 void ServiceWorkerGlobalScopeClient::DidHandleActivateEvent(

third_party/blink/renderer/modules/service_worker/service_worker_global_scope_client.h

@@ -37,7 +37,6 @@
 #include "third_party/blink/public/common/messaging/transferable_message.h"
 #include "third_party/blink/public/mojom/service_worker/service_worker.mojom-blink.h"
 #include "third_party/blink/public/mojom/service_worker/service_worker_event_status.mojom-blink.h"
-#include "third_party/blink/public/platform/modules/service_worker/web_service_worker_clients_info.h"
 #include "third_party/blink/public/platform/modules/service_worker/web_service_worker_stream_handle.h"
 #include "third_party/blink/renderer/core/messaging/message_port.h"
 #include "third_party/blink/renderer/core/workers/worker_clients.h"
@@ -48,9 +47,10 @@ namespace blink {
 
 struct WebPaymentHandlerResponse;
 class ExecutionContext;
+class KURL;
+class ScriptPromiseResolver;
 class WebServiceWorkerContextClient;
 class WebServiceWorkerResponse;
-class KURL;
 class WorkerClients;
 
 // See WebServiceWorkerContextClient for documentation for the methods in this
@@ -67,6 +67,7 @@ class MODULES_EXPORT ServiceWorkerGlobalScopeClient final
   using GetClientCallback = mojom::blink::ServiceWorkerHost::GetClientCallback;
   using GetClientsCallback =
       mojom::blink::ServiceWorkerHost::GetClientsCallback;
+  using FocusCallback = mojom::blink::ServiceWorkerHost::FocusClientCallback;
 
   static const char kSupplementName[];
 
@@ -76,21 +77,15 @@ class MODULES_EXPORT ServiceWorkerGlobalScopeClient final
   void GetClient(const String&, GetClientCallback);
   void GetClients(mojom::blink::ServiceWorkerClientQueryOptionsPtr,
                   GetClientsCallback);
-  void OpenWindowForClients(const KURL&,
-                            std::unique_ptr<WebServiceWorkerClientCallbacks>);
-  void OpenWindowForPaymentHandler(
-      const KURL&,
-      std::unique_ptr<WebServiceWorkerClientCallbacks>);
+  void OpenWindowForClients(const KURL&, ScriptPromiseResolver*);
+  void OpenWindowForPaymentHandler(const KURL&, ScriptPromiseResolver*);
   void SetCachedMetadata(const KURL&, const char*, size_t);
   void ClearCachedMetadata(const KURL&);
   void PostMessageToClient(const String& client_uuid, BlinkTransferableMessage);
   void SkipWaiting(SkipWaitingCallback);
   void Claim(ClaimCallback);
-  void Focus(const String& client_uuid,
-             std::unique_ptr<WebServiceWorkerClientCallbacks>);
-  void Navigate(const String& client_uuid,
-                const KURL&,
-                std::unique_ptr<WebServiceWorkerClientCallbacks>);
+  void Focus(const String& client_uuid, FocusCallback);
+  void Navigate(const String& client_uuid, const KURL&, ScriptPromiseResolver*);
 
   void DidHandleActivateEvent(int event_id,
                               mojom::ServiceWorkerEventStatus,

third_party/blink/renderer/modules/service_worker/service_worker_window_client.cc

@@ -16,24 +16,39 @@
 #include "third_party/blink/renderer/core/workers/worker_location.h"
 #include "third_party/blink/renderer/modules/service_worker/service_worker_error.h"
 #include "third_party/blink/renderer/modules/service_worker/service_worker_global_scope_client.h"
-#include "third_party/blink/renderer/modules/service_worker/service_worker_window_client_callback.h"
 #include "third_party/blink/renderer/platform/bindings/v8_throw_exception.h"
 
 namespace blink {
 
-ServiceWorkerWindowClient* ServiceWorkerWindowClient::Take(
-    ScriptPromiseResolver*,
-    std::unique_ptr<WebServiceWorkerClientInfo> web_client) {
-  return web_client ? ServiceWorkerWindowClient::Create(*web_client) : nullptr;
+namespace {
+
+void DidFocus(ScriptPromiseResolver* resolver,
+              mojom::blink::ServiceWorkerClientInfoPtr client) {
+  if (!resolver->GetExecutionContext() ||
+      resolver->GetExecutionContext()->IsContextDestroyed()) {
+    return;
+  }
+
+  if (!client) {
+    resolver->Reject(ServiceWorkerError::GetException(
+        resolver, mojom::blink::ServiceWorkerErrorType::kNotFound,
+        "The client was not found."));
+    return;
+  }
+  resolver->Resolve(ServiceWorkerWindowClient::Create(*client));
 }
 
+}  // namespace
+
 ServiceWorkerWindowClient* ServiceWorkerWindowClient::Create(
     const WebServiceWorkerClientInfo& info) {
+  DCHECK_EQ(mojom::blink::ServiceWorkerClientType::kWindow, info.client_type);
   return new ServiceWorkerWindowClient(info);
 }
 
 ServiceWorkerWindowClient* ServiceWorkerWindowClient::Create(
     const mojom::blink::ServiceWorkerClientInfo& info) {
+  DCHECK_EQ(mojom::blink::ServiceWorkerClientType::kWindow, info.client_type);
   return new ServiceWorkerWindowClient(info);
 }
 
@@ -67,10 +82,7 @@ ScriptPromise ServiceWorkerWindowClient::focus(ScriptState* script_state) {
   ExecutionContext::From(script_state)->ConsumeWindowInteraction();
 
   ServiceWorkerGlobalScopeClient::From(ExecutionContext::From(script_state))
-      ->Focus(Uuid(),
-              std::make_unique<CallbackPromiseAdapter<ServiceWorkerWindowClient,
-                                                      ServiceWorkerError>>(
-                  resolver));
+      ->Focus(Uuid(), WTF::Bind(&DidFocus, WrapPersistent(resolver)));
   return promise;
 }
 
@@ -93,8 +105,8 @@ ScriptPromise ServiceWorkerWindowClient::navigate(ScriptState* script_state,
     return promise;
   }
 
-  ServiceWorkerGlobalScopeClient::From(context)->Navigate(
-      Uuid(), parsed_url, std::make_unique<NavigateClientCallback>(resolver));
+  ServiceWorkerGlobalScopeClient::From(context)->Navigate(Uuid(), parsed_url,
+                                                          resolver);
   return promise;
 }
 

third_party/blink/renderer/modules/service_worker/service_worker_window_client.h

@@ -14,7 +14,6 @@
 
 namespace blink {
 
-class ScriptPromiseResolver;
 class ScriptState;
 
 class MODULES_EXPORT ServiceWorkerWindowClient final
@@ -22,13 +21,6 @@ class MODULES_EXPORT ServiceWorkerWindowClient final
   DEFINE_WRAPPERTYPEINFO();
 
  public:
-  // To be used by CallbackPromiseAdapter.
-  using WebType = std::unique_ptr<WebServiceWorkerClientInfo>;
-
-  static ServiceWorkerWindowClient* Take(
-      ScriptPromiseResolver*,
-      std::unique_ptr<WebServiceWorkerClientInfo>);
-
   static ServiceWorkerWindowClient* Create(const WebServiceWorkerClientInfo&);
   static ServiceWorkerWindowClient* Create(
       const mojom::blink::ServiceWorkerClientInfo&);

third_party/blink/renderer/modules/service_worker/service_worker_window_client_callback.cc

-// Copyright 2015 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 "third_party/blink/renderer/modules/service_worker/service_worker_window_client_callback.h"
-
-#include <memory>
-
-#include "base/memory/ptr_util.h"
-#include "third_party/blink/public/mojom/service_worker/service_worker_error_type.mojom-blink.h"
-#include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
-#include "third_party/blink/renderer/core/dom/dom_exception.h"
-#include "third_party/blink/renderer/modules/service_worker/service_worker_error.h"
-#include "third_party/blink/renderer/modules/service_worker/service_worker_window_client.h"
-#include "third_party/blink/renderer/platform/bindings/v8_throw_exception.h"
-
-namespace blink {
-
-void NavigateClientCallback::OnSuccess(
-    std::unique_ptr<WebServiceWorkerClientInfo> client_info) {
-  if (!resolver_->GetExecutionContext() ||
-      resolver_->GetExecutionContext()->IsContextDestroyed())
-    return;
-  resolver_->Resolve(ServiceWorkerWindowClient::Take(
-      resolver_.Get(), base::WrapUnique(client_info.release())));
-}
-
-void NavigateClientCallback::OnError(const WebServiceWorkerError& error) {
-  if (!resolver_->GetExecutionContext() ||
-      resolver_->GetExecutionContext()->IsContextDestroyed())
-    return;
-
-  if (error.error_type == mojom::blink::ServiceWorkerErrorType::kNavigation) {
-    ScriptState::Scope scope(resolver_->GetScriptState());
-    resolver_->Reject(V8ThrowException::CreateTypeError(
-        resolver_->GetScriptState()->GetIsolate(), error.message));
-    return;
-  }
-
-  resolver_->Reject(ServiceWorkerError::Take(resolver_.Get(), error));
-}
-
-}  // namespace blink

third_party/blink/renderer/modules/service_worker/service_worker_window_client_callback.h

-// Copyright 2015 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.
-
-#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_SERVICE_WORKER_SERVICE_WORKER_WINDOW_CLIENT_CALLBACK_H_
-#define THIRD_PARTY_BLINK_RENDERER_MODULES_SERVICE_WORKER_SERVICE_WORKER_WINDOW_CLIENT_CALLBACK_H_
-
-#include "base/macros.h"
-#include "third_party/blink/public/platform/modules/service_worker/web_service_worker_clients_info.h"
-#include "third_party/blink/renderer/platform/heap/persistent.h"
-
-namespace blink {
-
-class ScriptPromiseResolver;
-
-class NavigateClientCallback : public WebServiceWorkerClientCallbacks {
- public:
-  explicit NavigateClientCallback(ScriptPromiseResolver* resolver)
-      : resolver_(resolver) {}
-
-  void OnSuccess(std::unique_ptr<WebServiceWorkerClientInfo>) override;
-  void OnError(const WebServiceWorkerError&) override;
-
- private:
-  Persistent<ScriptPromiseResolver> resolver_;
-  DISALLOW_COPY_AND_ASSIGN(NavigateClientCallback);
-};
-
-}  // namespace blink
-
-#endif  // THIRD_PARTY_BLINK_RENDERER_MODULES_SERVICE_WORKER_SERVICE_WORKER_WINDOW_CLIENT_CALLBACK_H_