ServiceWorker: Replace RegistrationCallback and GetRegistrationsCallback with...

ServiceWorker: Replace RegistrationCallback and GetRegistrationsCallback with CallbackPromiseAdapter

This is a cleanup CL and does not change behavior.

BUG=n/a

Review-Url: https://codereview.chromium.org/2564363002
Cr-Commit-Position: refs/heads/master@{#438061}

content/child/service_worker/service_worker_dispatcher.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/lazy_instance.h"
+#include "base/memory/ptr_util.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/threading/thread_local.h"
@@ -523,21 +524,15 @@ void ServiceWorkerDispatcher::OnDidGetRegistrations(
   if (!callbacks)
     return;
 
-  typedef blink::WebVector<blink::WebServiceWorkerRegistration::Handle*>
-      WebServiceWorkerRegistrationArray;
-  std::unique_ptr<WebServiceWorkerRegistrationArray> registrations(
-      new WebServiceWorkerRegistrationArray(infos.size()));
+  using WebServiceWorkerRegistrationHandles =
+      WebServiceWorkerProvider::WebServiceWorkerRegistrationHandles;
+  std::unique_ptr<WebServiceWorkerRegistrationHandles> registrations =
+      base::MakeUnique<WebServiceWorkerRegistrationHandles>(infos.size());
   for (size_t i = 0; i < infos.size(); ++i) {
-    if (infos[i].handle_id != kInvalidServiceWorkerHandleId) {
-      ServiceWorkerRegistrationObjectInfo info(infos[i]);
-      ServiceWorkerVersionAttributes attr(attrs[i]);
-
-      // WebServiceWorkerGetRegistrationsCallbacks cannot receive an array of
-      // std::unique_ptr<WebServiceWorkerRegistration::Handle>, so create leaky
-      // handles instead.
-      (*registrations)[i] = WebServiceWorkerRegistrationImpl::CreateLeakyHandle(
-          GetOrAdoptRegistration(info, attr));
-    }
+    if (infos[i].handle_id == kInvalidServiceWorkerHandleId)
+      continue;
+    (*registrations)[i] = WebServiceWorkerRegistrationImpl::CreateHandle(
+        GetOrAdoptRegistration(infos[i], attrs[i]));
   }
 
   callbacks->onSuccess(std::move(registrations));

content/child/service_worker/web_service_worker_registration_impl.cc

@@ -200,14 +200,6 @@ WebServiceWorkerRegistrationImpl::CreateHandle(
   return base::MakeUnique<HandleImpl>(registration);
 }
 
-blink::WebServiceWorkerRegistration::Handle*
-WebServiceWorkerRegistrationImpl::CreateLeakyHandle(
-    const scoped_refptr<WebServiceWorkerRegistrationImpl>& registration) {
-  if (!registration)
-    return nullptr;
-  return new HandleImpl(registration);
-}
-
 WebServiceWorkerRegistrationImpl::~WebServiceWorkerRegistrationImpl() {
   ServiceWorkerDispatcher* dispatcher =
       ServiceWorkerDispatcher::GetThreadSpecificInstance();

content/child/service_worker/web_service_worker_registration_impl.h

@@ -78,13 +78,6 @@ class CONTENT_EXPORT WebServiceWorkerRegistrationImpl
   static std::unique_ptr<WebServiceWorkerRegistrationHandle> CreateHandle(
       const scoped_refptr<WebServiceWorkerRegistrationImpl>& registration);
 
-  // Same with CreateHandle(), but returns a raw pointer to the handle w/ its
-  // ownership instead. The caller must manage the ownership. This function must
-  // be used only for passing the handle to Blink API that does not support
-  // blink::WebPassOwnPtr.
-  static WebServiceWorkerRegistrationHandle* CreateLeakyHandle(
-      const scoped_refptr<WebServiceWorkerRegistrationImpl>& registration);
-
  private:
   friend class base::RefCounted<WebServiceWorkerRegistrationImpl>;
   ~WebServiceWorkerRegistrationImpl() override;

third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerContainer.cpp

@@ -29,6 +29,7 @@
  */
 #include "modules/serviceworkers/ServiceWorkerContainer.h"
 
+#include "bindings/core/v8/CallbackPromiseAdapter.h"
 #include "bindings/core/v8/ScriptPromise.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
@@ -62,41 +63,6 @@
 
 namespace blink {
 
-class RegistrationCallback
-    : public WebServiceWorkerProvider::WebServiceWorkerRegistrationCallbacks {
- public:
-  explicit RegistrationCallback(ScriptPromiseResolver* resolver)
-      : m_resolver(resolver) {}
-  ~RegistrationCallback() override {}
-
-  void onSuccess(
-      std::unique_ptr<WebServiceWorkerRegistration::Handle> handle) override {
-    if (!m_resolver->getExecutionContext() ||
-        m_resolver->getExecutionContext()->isContextDestroyed())
-      return;
-    m_resolver->resolve(ServiceWorkerRegistration::getOrCreate(
-        m_resolver->getExecutionContext(), WTF::wrapUnique(handle.release())));
-  }
-
-  void onError(const WebServiceWorkerError& error) override {
-    if (!m_resolver->getExecutionContext() ||
-        m_resolver->getExecutionContext()->isContextDestroyed())
-      return;
-    ScriptState::Scope scope(m_resolver->getScriptState());
-    if (error.errorType == WebServiceWorkerError::ErrorTypeType) {
-      m_resolver->reject(V8ThrowException::createTypeError(
-          m_resolver->getScriptState()->isolate(), error.message));
-    } else {
-      m_resolver->reject(
-          ServiceWorkerErrorForUpdate::take(m_resolver.get(), error));
-    }
-  }
-
- private:
-  Persistent<ScriptPromiseResolver> m_resolver;
-  WTF_MAKE_NONCOPYABLE(RegistrationCallback);
-};
-
 class GetRegistrationCallback : public WebServiceWorkerProvider::
                                     WebServiceWorkerGetRegistrationCallbacks {
  public:
@@ -132,42 +98,6 @@ class GetRegistrationCallback : public WebServiceWorkerProvider::
   WTF_MAKE_NONCOPYABLE(GetRegistrationCallback);
 };
 
-class GetRegistrationsCallback : public WebServiceWorkerProvider::
-                                     WebServiceWorkerGetRegistrationsCallbacks {
- public:
-  explicit GetRegistrationsCallback(ScriptPromiseResolver* resolver)
-      : m_resolver(resolver) {}
-  ~GetRegistrationsCallback() override {}
-
-  void onSuccess(
-      std::unique_ptr<WebVector<WebServiceWorkerRegistration::Handle*>>
-          webPassRegistrations) override {
-    Vector<std::unique_ptr<WebServiceWorkerRegistration::Handle>> handles;
-    std::unique_ptr<WebVector<WebServiceWorkerRegistration::Handle*>>
-        webRegistrations = WTF::wrapUnique(webPassRegistrations.release());
-    for (auto& handle : *webRegistrations) {
-      handles.append(WTF::wrapUnique(handle));
-    }
-
-    if (!m_resolver->getExecutionContext() ||
-        m_resolver->getExecutionContext()->isContextDestroyed())
-      return;
-    m_resolver->resolve(
-        ServiceWorkerRegistrationArray::take(m_resolver.get(), &handles));
-  }
-
-  void onError(const WebServiceWorkerError& error) override {
-    if (!m_resolver->getExecutionContext() ||
-        m_resolver->getExecutionContext()->isContextDestroyed())
-      return;
-    m_resolver->reject(ServiceWorkerError::take(m_resolver.get(), error));
-  }
-
- private:
-  Persistent<ScriptPromiseResolver> m_resolver;
-  WTF_MAKE_NONCOPYABLE(GetRegistrationsCallback);
-};
-
 class ServiceWorkerContainer::GetRegistrationForReadyCallback
     : public WebServiceWorkerProvider::
           WebServiceWorkerGetRegistrationForReadyCallbacks {
@@ -358,8 +288,11 @@ ScriptPromise ServiceWorkerContainer::registerServiceWorker(
     patternURL = enteredExecutionContext(scriptState->isolate())
                      ->completeURL(options.scope());
 
-  registerServiceWorkerImpl(executionContext, scriptURL, patternURL,
-                            WTF::makeUnique<RegistrationCallback>(resolver));
+  registerServiceWorkerImpl(
+      executionContext, scriptURL, patternURL,
+      WTF::makeUnique<CallbackPromiseAdapter<ServiceWorkerRegistration,
+                                             ServiceWorkerErrorForUpdate>>(
+          resolver));
 
   return promise;
 }
@@ -455,7 +388,8 @@ ScriptPromise ServiceWorkerContainer::getRegistrations(
   }
 
   m_provider->getRegistrations(
-      WTF::makeUnique<GetRegistrationsCallback>(resolver));
+      WTF::makeUnique<CallbackPromiseAdapter<ServiceWorkerRegistrationArray,
+                                             ServiceWorkerError>>(resolver));
 
   return promise;
 }

third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerError.cpp

@@ -137,6 +137,9 @@ v8::Local<v8::Value> ServiceWorkerErrorForUpdate::take(
       // a TypeError.
       return V8ThrowException::createTypeError(
           scriptState->isolate(), getExceptionParams(webError).message);
+    case WebServiceWorkerError::ErrorTypeType:
+      return V8ThrowException::createTypeError(scriptState->isolate(),
+                                               webError.message);
     default:
       return toV8(ServiceWorkerError::take(resolver, webError),
                   scriptState->context()->Global(), scriptState->isolate());

third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerRegistration.cpp

@@ -21,6 +21,12 @@
 
 namespace blink {
 
+ServiceWorkerRegistration* ServiceWorkerRegistration::take(
+    ScriptPromiseResolver* resolver,
+    std::unique_ptr<WebServiceWorkerRegistration::Handle> handle) {
+  return getOrCreate(resolver->getExecutionContext(), std::move(handle));
+}
+
 bool ServiceWorkerRegistration::hasPendingActivity() const {
   return !m_stopped;
 }

third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerRegistration.h

@@ -37,6 +37,12 @@ class ServiceWorkerRegistration final
   USING_PRE_FINALIZER(ServiceWorkerRegistration, dispose);
 
  public:
+  // Called from CallbackPromiseAdapter.
+  using WebType = std::unique_ptr<WebServiceWorkerRegistration::Handle>;
+  static ServiceWorkerRegistration* take(
+      ScriptPromiseResolver*,
+      std::unique_ptr<WebServiceWorkerRegistration::Handle>);
+
   // ScriptWrappable overrides.
   bool hasPendingActivity() const final;
 
@@ -102,14 +108,17 @@ class ServiceWorkerRegistrationArray {
   STATIC_ONLY(ServiceWorkerRegistrationArray);
 
  public:
+  // Called from CallbackPromiseAdapter.
+  using WebType = std::unique_ptr<
+      WebVector<std::unique_ptr<WebServiceWorkerRegistration::Handle>>>;
   static HeapVector<Member<ServiceWorkerRegistration>> take(
       ScriptPromiseResolver* resolver,
-      Vector<std::unique_ptr<WebServiceWorkerRegistration::Handle>>*
-          webServiceWorkerRegistrations) {
+      WebType webServiceWorkerRegistrations) {
     HeapVector<Member<ServiceWorkerRegistration>> registrations;
-    for (auto& registration : *webServiceWorkerRegistrations)
-      registrations.append(ServiceWorkerRegistration::getOrCreate(
-          resolver->getExecutionContext(), std::move(registration)));
+    for (auto& registration : *webServiceWorkerRegistrations) {
+      registrations.append(
+          ServiceWorkerRegistration::take(resolver, std::move(registration)));
+    }
     return registrations;
   }
 };

third_party/WebKit/public/platform/modules/serviceworker/WebServiceWorkerProvider.h

@@ -59,10 +59,11 @@ class WebServiceWorkerProvider {
       WebCallbacks<std::unique_ptr<WebServiceWorkerRegistration::Handle>,
                    const WebServiceWorkerError&>;
 
-  // Each element's ownership is transferred.
-  using WebServiceWorkerGetRegistrationsCallbacks = WebCallbacks<
-      std::unique_ptr<WebVector<WebServiceWorkerRegistration::Handle*>>,
-      const WebServiceWorkerError&>;
+  using WebServiceWorkerRegistrationHandles =
+      WebVector<std::unique_ptr<WebServiceWorkerRegistration::Handle>>;
+  using WebServiceWorkerGetRegistrationsCallbacks =
+      WebCallbacks<std::unique_ptr<WebServiceWorkerRegistrationHandles>,
+                   const WebServiceWorkerError&>;
   using WebServiceWorkerGetRegistrationForReadyCallbacks =
       WebCallbacks<std::unique_ptr<WebServiceWorkerRegistration::Handle>, void>;