[background-sync] Remove WebSyncError and SyncCallbacks

third_party/WebKit/Source/modules/background_sync/BackgroundSyncProvider.cpp

Index: third_party/WebKit/Source/modules/background_sync/BackgroundSyncProvider.cpp
diff --git a/third_party/WebKit/Source/modules/background_sync/BackgroundSyncProvider.cpp b/third_party/WebKit/Source/modules/background_sync/BackgroundSyncProvider.cpp
index 63229a7daf1e0c2fbd853d67b191ab69eff83068..5598d569ba4b06790a6c1d8f2bbf350bc3494c1e 100644
--- a/third_party/WebKit/Source/modules/background_sync/BackgroundSyncProvider.cpp
+++ b/third_party/WebKit/Source/modules/background_sync/BackgroundSyncProvider.cpp
@@ -4,35 +4,85 @@
 
 #include "modules/background_sync/BackgroundSyncProvider.h"
 
+#include "bindings/core/v8/ScriptPromiseResolver.h"
+#include "modules/background_sync/SyncError.h"
+#include "platform/heap/Persistent.h"
 #include "public/platform/InterfaceProvider.h"
 #include "public/platform/Platform.h"
-#include "public/platform/modules/background_sync/WebSyncError.h"
 #include "public/platform/modules/serviceworker/WebServiceWorkerRegistration.h"
 #include "wtf/Functional.h"
 
 namespace blink {
 
+// static
+void BackgroundSyncProvider::onRegisterSuccess(
+    ScriptPromiseResolver* resolver,
+    mojom::blink::SyncRegistrationPtr syncRegistration) {
+  if (!resolver->getExecutionContext() ||

[jbroman, 2016/11/18 16:53:31]

Here and below: ScriptPromiseResolver::resolveOrReject already checks internally
whether active DOM objects are stopped; is this necessary here (and below)?

If you do remove this check here, then this function becomes extremely short,
and it might be simpler to just inline it at its call site.

[adithyas, 2016/11/18 20:05:00]

Yeah, it's not necessary, I'll just inline it. The old implementation of the
callbacks had the check and I just copied them over.

+      resolver->getExecutionContext()->activeDOMObjectsAreStopped()) {
+    return;
+  }
+
+  if (!syncRegistration) {
+    resolver->resolve(v8::Null(resolver->getScriptState()->isolate()));
+    return;
+  }
+  resolver->resolve();
+}
+
+// static
+void BackgroundSyncProvider::onRegisterError(ScriptPromiseResolver* resolver,
+                                             const SyncError& error) {
+  if (!resolver->getExecutionContext() ||
+      resolver->getExecutionContext()->activeDOMObjectsAreStopped()) {
+    return;
+  }
+  resolver->reject(SyncError::take(resolver, error));
+}
+
+// static
+void BackgroundSyncProvider::onGetRegistrationsSuccess(
+    ScriptPromiseResolver* resolver,
+    const Vector<mojom::blink::SyncRegistrationPtr>& syncRegistrations) {
+  if (!resolver->getExecutionContext() ||
+      resolver->getExecutionContext()->activeDOMObjectsAreStopped()) {
+    return;
+  }
+  Vector<String> tags;
+  for (const mojom::blink::SyncRegistrationPtr& r : syncRegistrations) {
+    tags.append(r->tag);
+  }
+  resolver->resolve(tags);
+}
+
+// static
+void BackgroundSyncProvider::onGetRegistrationsError(

[jbroman, 2016/11/18 16:53:31]

If you don't take my suggestion above and remove this function, it's probably
clearer to just remove this one and rename "onRegisterError" to a more generic
name, like "onError".

+    ScriptPromiseResolver* resolver,
+    const SyncError& error) {
+  onRegisterError(resolver, error);
+}
+
 void BackgroundSyncProvider::registerBackgroundSync(
     mojom::blink::SyncRegistrationPtr options,
     WebServiceWorkerRegistration* serviceWorkerRegistration,
-    std::unique_ptr<SyncRegistrationCallbacks> callbacks) {
+    ScriptPromiseResolver* resolver) {
   DCHECK(options);
   DCHECK(serviceWorkerRegistration);
-  DCHECK(callbacks);
+  DCHECK(resolver);
   int64_t serviceWorkerRegistrationId =
       serviceWorkerRegistration->registrationId();
 
   GetBackgroundSyncServicePtr()->Register(
       std::move(options), serviceWorkerRegistrationId,
       convertToBaseCallback(WTF::bind(&BackgroundSyncProvider::registerCallback,
-                                      WTF::passed(std::move(callbacks)))));
+                                      wrapPersistent(resolver))));
 }
 
 void BackgroundSyncProvider::getRegistrations(
     WebServiceWorkerRegistration* serviceWorkerRegistration,
-    std::unique_ptr<SyncGetRegistrationsCallbacks> callbacks) {
+    ScriptPromiseResolver* resolver) {
   DCHECK(serviceWorkerRegistration);
-  DCHECK(callbacks);
+  DCHECK(resolver);
   int64_t serviceWorkerRegistrationId =
       serviceWorkerRegistration->registrationId();
 
@@ -40,57 +90,52 @@ void BackgroundSyncProvider::getRegistrations(
       serviceWorkerRegistrationId,
       convertToBaseCallback(
           WTF::bind(&BackgroundSyncProvider::getRegistrationsCallback,
-                    WTF::passed(std::move(callbacks)))));
+                    wrapPersistent(resolver))));
 }
 
 // static
 void BackgroundSyncProvider::registerCallback(
-    std::unique_ptr<SyncRegistrationCallbacks> callbacks,
+    ScriptPromiseResolver* resolver,
     mojom::blink::BackgroundSyncError error,
     mojom::blink::SyncRegistrationPtr options) {
   // TODO(iclelland): Determine the correct error message to return in each case
   switch (error) {
     case mojom::blink::BackgroundSyncError::NONE:
-      if (!options.is_null())
-        callbacks->onSuccess(std::move(options));
+      onRegisterSuccess(resolver, std::move(options));
       break;
     case mojom::blink::BackgroundSyncError::NOT_FOUND:
       NOTREACHED();
       break;
     case mojom::blink::BackgroundSyncError::STORAGE:
-      callbacks->onError(blink::WebSyncError(WebSyncError::ErrorTypeUnknown,
-                                             "Background Sync is disabled."));
+      onRegisterError(resolver, SyncError(SyncError::ErrorTypeUnknown,
+                                          "Background Sync is disabled."));
       break;
     case mojom::blink::BackgroundSyncError::NOT_ALLOWED:
-      callbacks->onError(
-          blink::WebSyncError(WebSyncError::ErrorTypeNoPermission,
-                              "Attempted to register a sync event without a "
-                              "window or registration tag too long."));
+      onRegisterError(resolver,
+                      SyncError(SyncError::ErrorTypeNoPermission,
+                                "Attempted to register a sync event without a "
+                                "window or registration tag too long."));
       break;
     case mojom::blink::BackgroundSyncError::PERMISSION_DENIED:
-      callbacks->onError(blink::WebSyncError(
-          WebSyncError::ErrorTypePermissionDenied, "Permission denied."));
+      onRegisterError(resolver, SyncError(SyncError::ErrorTypePermissionDenied,
+                                          "Permission denied."));
       break;
     case mojom::blink::BackgroundSyncError::NO_SERVICE_WORKER:
-      callbacks->onError(blink::WebSyncError(WebSyncError::ErrorTypeUnknown,
-                                             "No service worker is active."));
+      onRegisterError(resolver, SyncError(SyncError::ErrorTypeUnknown,
+                                          "No service worker is active."));
       break;
   }
 }
 
 // static
 void BackgroundSyncProvider::getRegistrationsCallback(
-    std::unique_ptr<SyncGetRegistrationsCallbacks> syncGetRegistrationCallbacks,
+    ScriptPromiseResolver* resolver,
     mojom::blink::BackgroundSyncError error,
     mojo::WTFArray<mojom::blink::SyncRegistrationPtr> registrations) {
   // TODO(iclelland): Determine the correct error message to return in each case
   switch (error) {
     case mojom::blink::BackgroundSyncError::NONE: {
-      WebVector<mojom::blink::SyncRegistration*> results(registrations.size());
-      for (size_t i = 0; i < registrations.size(); ++i) {
-        results[i] = registrations[i].get();
-      }
-      syncGetRegistrationCallbacks->onSuccess(results);
+      onGetRegistrationsSuccess(resolver, registrations.storage());
       break;
     }
     case mojom::blink::BackgroundSyncError::NOT_FOUND:
@@ -101,12 +146,14 @@ void BackgroundSyncProvider::getRegistrationsCallback(
       NOTREACHED();
       break;
     case mojom::blink::BackgroundSyncError::STORAGE:
-      syncGetRegistrationCallbacks->onError(blink::WebSyncError(
-          WebSyncError::ErrorTypeUnknown, "Background Sync is disabled."));
+      onGetRegistrationsError(resolver,
+                              SyncError(SyncError::ErrorTypeUnknown,
+                                        "Background Sync is disabled."));
       break;
     case mojom::blink::BackgroundSyncError::NO_SERVICE_WORKER:
-      syncGetRegistrationCallbacks->onError(blink::WebSyncError(
-          WebSyncError::ErrorTypeUnknown, "No service worker is active."));
+      onGetRegistrationsError(resolver,
+                              SyncError(SyncError::ErrorTypeUnknown,
+                                        "No service worker is active."));
       break;
   }
 }