[BackgroundSync] Use appopriate type parameters for WebCallbacks (2/4)

This series of CLs fixes WebCallback parameter types used in BackgroundSync.

 - WebSyncRegistration* (ownership transferred)
  => PassOwnPtr<WebServiceWorkerResponse>
 - bool* (ownership transferred)
  => bool
 - WebVector<WebSyncRegstration*>* (ownership transferred)
  => const WebVector<WebSyncRegistration*>&
 - WebPermissionStatus* (ownership transferred)
  => WebPermissionStatus
 - WebSyncError* (ownership transferred)
  => const WebSyncError&

1/4: https://codereview.chromium.org/1309143002/
2/4: https://codereview.chromium.org/1311053002/
3/4: https://codereview.chromium.org/1308273002/
4/4: https://codereview.chromium.org/1308283002/

BUG=493531

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

git-svn-id: svn://svn.chromium.org/blink/trunk@201312 bbb929c8-8fbe-4397-9dbb-9b2b20218538

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

@@ -20,12 +20,11 @@
 
 namespace blink {
 
-PeriodicSyncRegistration* PeriodicSyncRegistration::take(ScriptPromiseResolver*, WebSyncRegistration* syncRegistration, ServiceWorkerRegistration* serviceWorkerRegistration)
+PeriodicSyncRegistration* PeriodicSyncRegistration::take(ScriptPromiseResolver*, PassOwnPtr<WebSyncRegistration> registration, ServiceWorkerRegistration* serviceWorkerRegistration)
 {
-    OwnPtr<WebSyncRegistration> registration = adoptPtr(syncRegistration);
     PeriodicSyncRegistrationOptions options = PeriodicSyncRegistrationOptions();
-    options.setMinPeriod(syncRegistration->minPeriodMs);
-    switch (syncRegistration->networkState) {
+    options.setMinPeriod(registration->minPeriodMs);
+    switch (registration->networkState) {
     case WebSyncRegistration::NetworkState::NetworkStateAny:
         options.setNetworkState("any");
         break;
@@ -36,7 +35,7 @@ PeriodicSyncRegistration* PeriodicSyncRegistration::take(ScriptPromiseResolver*,
         options.setNetworkState("online");
         break;
     }
-    switch (syncRegistration->powerState) {
+    switch (registration->powerState) {
     case WebSyncRegistration::PowerState::PowerStateAuto:
         options.setPowerState("auto");
         break;
@@ -44,14 +43,8 @@ PeriodicSyncRegistration* PeriodicSyncRegistration::take(ScriptPromiseResolver*,
         options.setPowerState("avoid-draining");
         break;
     }
-    options.setTag(syncRegistration->tag);
-    return new PeriodicSyncRegistration(syncRegistration->id, options, serviceWorkerRegistration);
-}
-
-void PeriodicSyncRegistration::dispose(WebSyncRegistration* syncRegistration)
-{
-    if (syncRegistration)
-        delete syncRegistration;
+    options.setTag(registration->tag);
+    return new PeriodicSyncRegistration(registration->id, options, serviceWorkerRegistration);
 }
 
 PeriodicSyncRegistration::PeriodicSyncRegistration(int64_t id, const PeriodicSyncRegistrationOptions& options, ServiceWorkerRegistration* serviceWorkerRegistration)

third_party/WebKit/Source/modules/background_sync/PeriodicSyncRegistration.h

@@ -20,8 +20,7 @@ struct WebSyncRegistration;
 class PeriodicSyncRegistration final : public GarbageCollectedFinalized<PeriodicSyncRegistration>, public ScriptWrappable {
     DEFINE_WRAPPERTYPEINFO();
 public:
-    static PeriodicSyncRegistration* take(ScriptPromiseResolver*, WebSyncRegistration*, ServiceWorkerRegistration*);
-    static void dispose(WebSyncRegistration* registrationRaw);
+    static PeriodicSyncRegistration* take(ScriptPromiseResolver*, PassOwnPtr<WebSyncRegistration>, ServiceWorkerRegistration*);
 
     virtual ~PeriodicSyncRegistration();
 

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

@@ -27,31 +27,30 @@ SyncRegistrationCallbacks::~SyncRegistrationCallbacks()
 {
 }
 
-void SyncRegistrationCallbacks::onSuccess(WebSyncRegistration* webSyncRegistration)
+void SyncRegistrationCallbacks::onSuccess(WebPassOwnPtr<WebSyncRegistration> webSyncRegistration)
 {
     if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
-        SyncRegistration::dispose(webSyncRegistration);
         return;
     }
 
-    if (!webSyncRegistration) {
+    OwnPtr<WebSyncRegistration> registration = webSyncRegistration.release();
+    if (!registration) {
         m_resolver->resolve(v8::Null(m_resolver->scriptState()->isolate()));
         return;
     }
-    switch (webSyncRegistration->periodicity) {
+    switch (registration->periodicity) {
     case WebSyncRegistration::PeriodicityPeriodic:
-        m_resolver->resolve(PeriodicSyncRegistration::take(m_resolver.get(), webSyncRegistration, m_serviceWorkerRegistration));
+        m_resolver->resolve(PeriodicSyncRegistration::take(m_resolver.get(), registration.release(), m_serviceWorkerRegistration));
         break;
     case WebSyncRegistration::PeriodicityOneShot:
-        m_resolver->resolve(SyncRegistration::take(m_resolver.get(), webSyncRegistration, m_serviceWorkerRegistration));
+        m_resolver->resolve(SyncRegistration::take(m_resolver.get(), registration.release(), m_serviceWorkerRegistration));
         break;
     }
 }
 
-void SyncRegistrationCallbacks::onError(WebSyncError* error)
+void SyncRegistrationCallbacks::onError(const WebSyncError& error)
 {
     if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
-        SyncError::dispose(error);
         return;
     }
     m_resolver->reject(SyncError::take(m_resolver.get(), error));
@@ -69,20 +68,18 @@ SyncNotifyWhenDoneCallbacks::~SyncNotifyWhenDoneCallbacks()
 {
 }
 
-void SyncNotifyWhenDoneCallbacks::onSuccess(bool* status)
+void SyncNotifyWhenDoneCallbacks::onSuccess(bool status)
 {
-    OwnPtr<bool> statusPtr = adoptPtr(status);
     if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
         return;
     }
 
-    m_resolver->resolve(*status);
+    m_resolver->resolve(status);
 }
 
-void SyncNotifyWhenDoneCallbacks::onError(WebSyncError* error)
+void SyncNotifyWhenDoneCallbacks::onError(const WebSyncError& error)
 {
     if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
-        SyncError::dispose(error);
         return;
     }
     m_resolver->reject(SyncError::take(m_resolver.get(), error));
@@ -100,20 +97,18 @@ SyncUnregistrationCallbacks::~SyncUnregistrationCallbacks()
 {
 }
 
-void SyncUnregistrationCallbacks::onSuccess(bool* status)
+void SyncUnregistrationCallbacks::onSuccess(bool status)
 {
-    OwnPtr<bool> statusPtr = adoptPtr(status);
     if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
         return;
     }
 
-    m_resolver->resolve(*status);
+    m_resolver->resolve(status);
 }
 
-void SyncUnregistrationCallbacks::onError(WebSyncError* error)
+void SyncUnregistrationCallbacks::onError(const WebSyncError& error)
 {
     if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
-        SyncError::dispose(error);
         return;
     }
     m_resolver->reject(SyncError::take(m_resolver.get(), error));
@@ -131,47 +126,36 @@ SyncGetRegistrationsCallbacks::~SyncGetRegistrationsCallbacks()
 {
 }
 
-void SyncGetRegistrationsCallbacks::onSuccess(WebVector<WebSyncRegistration*>* webSyncRegistrations)
+void SyncGetRegistrationsCallbacks::onSuccess(const WebVector<WebSyncRegistration*>& webSyncRegistrations)
 {
-    if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
-        if (webSyncRegistrations) {
-            for (size_t i = 0; i < webSyncRegistrations->size(); ++i)
-                SyncRegistration::dispose((*webSyncRegistrations)[i]);
-            delete (webSyncRegistrations);
-        }
-        return;
+    Vector<OwnPtr<WebSyncRegistration>> registrations;
+    for (WebSyncRegistration* r : webSyncRegistrations) {
+        registrations.append(adoptPtr(r));
     }
-
-    if (!webSyncRegistrations) {
-        m_resolver->resolve(v8::Null(m_resolver->scriptState()->isolate()));
+    if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
         return;
     }
 
-    if (webSyncRegistrations->size() && (*webSyncRegistrations)[0]->periodicity == WebSyncRegistration::PeriodicityOneShot) {
+    if (webSyncRegistrations.size() && webSyncRegistrations[0]->periodicity == WebSyncRegistration::PeriodicityOneShot) {
         Vector<SyncRegistration*> syncRegistrations;
-        for (size_t i = 0; i < webSyncRegistrations->size(); ++i) {
-            WebSyncRegistration* webSyncRegistration = (*webSyncRegistrations)[i];
-            SyncRegistration* reg = SyncRegistration::take(m_resolver.get(), webSyncRegistration, m_serviceWorkerRegistration);
+        for (auto& r : registrations) {
+            SyncRegistration* reg = SyncRegistration::take(m_resolver.get(), r.release(), m_serviceWorkerRegistration);
             syncRegistrations.append(reg);
         }
-        delete (webSyncRegistrations);
         m_resolver->resolve(syncRegistrations);
     } else {
         Vector<PeriodicSyncRegistration*> syncRegistrations;
-        for (size_t i = 0; i < webSyncRegistrations->size(); ++i) {
-            WebSyncRegistration* webSyncRegistration = (*webSyncRegistrations)[i];
-            PeriodicSyncRegistration* reg = PeriodicSyncRegistration::take(m_resolver.get(), webSyncRegistration, m_serviceWorkerRegistration);
+        for (auto& r : registrations) {
+            PeriodicSyncRegistration* reg = PeriodicSyncRegistration::take(m_resolver.get(), r.release(), m_serviceWorkerRegistration);
             syncRegistrations.append(reg);
         }
-        delete (webSyncRegistrations);
         m_resolver->resolve(syncRegistrations);
     }
 }
 
-void SyncGetRegistrationsCallbacks::onError(WebSyncError* error)
+void SyncGetRegistrationsCallbacks::onError(const WebSyncError& error)
 {
     if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
-        SyncError::dispose(error);
         return;
     }
     m_resolver->reject(SyncError::take(m_resolver.get(), error));
@@ -189,20 +173,18 @@ SyncGetPermissionStatusCallbacks::~SyncGetPermissionStatusCallbacks()
 {
 }
 
-void SyncGetPermissionStatusCallbacks::onSuccess(WebSyncPermissionStatus* status)
+void SyncGetPermissionStatusCallbacks::onSuccess(WebSyncPermissionStatus status)
 {
-    OwnPtr<WebSyncPermissionStatus> statusPtr = adoptPtr(status);
     if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
         return;
     }
 
-    m_resolver->resolve(permissionString(*statusPtr));
+    m_resolver->resolve(permissionString(status));
 }
 
-void SyncGetPermissionStatusCallbacks::onError(WebSyncError* error)
+void SyncGetPermissionStatusCallbacks::onError(const WebSyncError& error)
 {
     if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
-        SyncError::dispose(error);
         return;
     }
     m_resolver->reject(SyncError::take(m_resolver.get(), error));

third_party/WebKit/Source/modules/background_sync/SyncCallbacks.h

@@ -31,8 +31,8 @@ public:
     SyncRegistrationCallbacks(ScriptPromiseResolver*, ServiceWorkerRegistration*);
     ~SyncRegistrationCallbacks() override;
 
-    void onSuccess(WebSyncRegistration*) override;
-    void onError(WebSyncError*) override;
+    void onSuccess(WebPassOwnPtr<WebSyncRegistration>) override;
+    void onError(const WebSyncError&) override;
 
 private:
     Persistent<ScriptPromiseResolver> m_resolver;
@@ -50,8 +50,8 @@ public:
     SyncNotifyWhenDoneCallbacks(ScriptPromiseResolver*, ServiceWorkerRegistration*);
     ~SyncNotifyWhenDoneCallbacks() override;
 
-    void onSuccess(bool*) override;
-    void onError(WebSyncError*) override;
+    void onSuccess(bool) override;
+    void onError(const WebSyncError&) override;
 
 private:
     Persistent<ScriptPromiseResolver> m_resolver;
@@ -69,8 +69,8 @@ public:
     SyncUnregistrationCallbacks(ScriptPromiseResolver*, ServiceWorkerRegistration*);
     ~SyncUnregistrationCallbacks() override;
 
-    void onSuccess(bool*) override;
-    void onError(WebSyncError*) override;
+    void onSuccess(bool) override;
+    void onError(const WebSyncError&) override;
 
 private:
     Persistent<ScriptPromiseResolver> m_resolver;
@@ -87,8 +87,8 @@ public:
     SyncGetRegistrationsCallbacks(ScriptPromiseResolver*, ServiceWorkerRegistration*);
     ~SyncGetRegistrationsCallbacks() override;
 
-    void onSuccess(WebVector<WebSyncRegistration*>*) override;
-    void onError(WebSyncError*) override;
+    void onSuccess(const WebVector<WebSyncRegistration*>&) override;
+    void onError(const WebSyncError&) override;
 
 private:
     Persistent<ScriptPromiseResolver> m_resolver;
@@ -104,8 +104,8 @@ public:
     SyncGetPermissionStatusCallbacks(ScriptPromiseResolver*, ServiceWorkerRegistration*);
     ~SyncGetPermissionStatusCallbacks() override;
 
-    void onSuccess(WebSyncPermissionStatus*) override;
-    void onError(WebSyncError*) override;
+    void onSuccess(WebSyncPermissionStatus) override;
+    void onError(const WebSyncError&) override;
 
 private:
     static String permissionString(WebSyncPermissionStatus);

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

@@ -11,26 +11,20 @@
 
 namespace blink {
 
-DOMException* SyncError::take(ScriptPromiseResolver*, WebSyncError* webErrorRaw)
+DOMException* SyncError::take(ScriptPromiseResolver*, const WebSyncError& webError)
 {
-    OwnPtr<WebSyncError> webError = adoptPtr(webErrorRaw);
-    switch (webError->errorType) {
+    switch (webError.errorType) {
     case WebSyncError::ErrorTypeAbort:
-        return DOMException::create(AbortError, webError->message);
+        return DOMException::create(AbortError, webError.message);
     case WebSyncError::ErrorTypeNoPermission:
-        return DOMException::create(InvalidAccessError, webError->message);
+        return DOMException::create(InvalidAccessError, webError.message);
     case WebSyncError::ErrorTypeNotFound:
-        return DOMException::create(NotFoundError, webError->message);
+        return DOMException::create(NotFoundError, webError.message);
     case WebSyncError::ErrorTypeUnknown:
-        return DOMException::create(UnknownError, webError->message);
+        return DOMException::create(UnknownError, webError.message);
     }
     ASSERT_NOT_REACHED();
     return DOMException::create(UnknownError);
 }
 
-void SyncError::dispose(WebSyncError* webErrorRaw)
-{
-    delete webErrorRaw;
-}
-
 } // namespace blink

third_party/WebKit/Source/modules/background_sync/SyncError.h

@@ -16,8 +16,7 @@ class ScriptPromiseResolver;
 class SyncError {
     WTF_MAKE_NONCOPYABLE(SyncError);
 public:
-    static DOMException* take(ScriptPromiseResolver*, WebSyncError*);
-    static void dispose(WebSyncError*);
+    static DOMException* take(ScriptPromiseResolver*, const WebSyncError&);
 
 private:
     SyncError() = delete;

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

@@ -27,16 +27,9 @@ SyncRegistration* SyncRegistration::create(const WebSyncRegistration& syncRegist
     return new SyncRegistration(syncRegistration.id, options, serviceWorkerRegistration);
 }
 
-SyncRegistration* SyncRegistration::take(ScriptPromiseResolver*, WebSyncRegistration* syncRegistration, ServiceWorkerRegistration* serviceWorkerRegistration)
+SyncRegistration* SyncRegistration::take(ScriptPromiseResolver*, PassOwnPtr<WebSyncRegistration> registration, ServiceWorkerRegistration* serviceWorkerRegistration)
 {
-    OwnPtr<WebSyncRegistration> registration = adoptPtr(syncRegistration);
-    return create(*syncRegistration, serviceWorkerRegistration);
-}
-
-void SyncRegistration::dispose(WebSyncRegistration* syncRegistration)
-{
-    if (syncRegistration)
-        delete syncRegistration;
+    return create(*registration, serviceWorkerRegistration);
 }
 
 SyncRegistration::SyncRegistration(int64_t id, const SyncRegistrationOptions& options, ServiceWorkerRegistration* serviceWorkerRegistration)

third_party/WebKit/Source/modules/background_sync/SyncRegistration.h

@@ -22,8 +22,7 @@ class MODULES_EXPORT SyncRegistration final : public GarbageCollectedFinalized<S
     DEFINE_WRAPPERTYPEINFO();
 public:
     static SyncRegistration* create(const WebSyncRegistration&, ServiceWorkerRegistration*);
-    static SyncRegistration* take(ScriptPromiseResolver*, WebSyncRegistration*, ServiceWorkerRegistration*);
-    static void dispose(WebSyncRegistration* registrationRaw);
+    static SyncRegistration* take(ScriptPromiseResolver*, PassOwnPtr<WebSyncRegistration>, ServiceWorkerRegistration*);
 
     virtual ~SyncRegistration();
 

third_party/WebKit/public/platform/modules/background_sync/WebSyncClient.h

@@ -7,14 +7,13 @@
 
 #include "public/platform/WebCallbacks.h"
 #include "public/platform/modules/background_sync/WebSyncError.h"
+#include "public/platform/modules/background_sync/WebSyncProvider.h"
 
 namespace blink {
 
 class WebServiceWorkerRegistration;
 struct WebSyncRegistration;
 
-using WebSyncRegistrationCallbacks = WebCallbacks<WebSyncRegistration*, WebSyncError*>;
-
 class WebSyncClient {
 public:
     virtual ~WebSyncClient() { }

third_party/WebKit/public/platform/modules/background_sync/WebSyncProvider.h

@@ -7,6 +7,7 @@
 
 #include "public/platform/WebCallbacks.h"
 #include "public/platform/WebCommon.h"
+#include "public/platform/WebPassOwnPtr.h"
 #include "public/platform/WebString.h"
 #include "public/platform/WebVector.h"
 #include "public/platform/modules/background_sync/WebSyncError.h"
@@ -16,12 +17,78 @@
 namespace blink {
 
 class WebServiceWorkerRegistration;
+template <>
+class WebCallbacks<WebPassOwnPtr<WebSyncRegistration>, const WebSyncError&> {
+public:
+    virtual ~WebCallbacks() {}
+    void onSuccess(WebSyncRegistration* r) { onSuccess(adoptWebPtr(r)); }
+    void onError(WebSyncError* e)
+    {
+        onError(*e);
+        delete e;
+    }
+    virtual void onSuccess(WebPassOwnPtr<WebSyncRegistration>) = 0;
+    virtual void onError(const WebSyncError&) = 0;
+};
+using WebSyncRegistrationCallbacks = WebCallbacks<WebPassOwnPtr<WebSyncRegistration>, const WebSyncError&>;
+
+template <>
+class WebCallbacks<bool, const WebSyncError&> {
+public:
+    virtual ~WebCallbacks() {}
+    void onSuccess(bool* r)
+    {
+        onSuccess(*r);
+        delete r;
+    }
+    void onError(WebSyncError* e)
+    {
+        onError(*e);
+        delete e;
+    }
+    virtual void onSuccess(bool) = 0;
+    virtual void onError(const WebSyncError&) = 0;
+};
+using WebSyncNotifyWhenDoneCallbacks = WebCallbacks<bool, const WebSyncError&>;
+using WebSyncUnregistrationCallbacks = WebCallbacks<bool, const WebSyncError&>;
+
+template <>
+class WebCallbacks<const WebVector<WebSyncRegistration*>&, const WebSyncError&> {
+public:
+    virtual ~WebCallbacks() {}
+    void onSuccess(WebVector<WebSyncRegistration*>* r)
+    {
+        onSuccess(*r);
+        delete r;
+    }
+    void onError(WebSyncError* e)
+    {
+        onError(*e);
+        delete e;
+    }
+    virtual void onSuccess(const WebVector<WebSyncRegistration*>&) = 0;
+    virtual void onError(const WebSyncError&) = 0;
+};
+using WebSyncGetRegistrationsCallbacks = WebCallbacks<const WebVector<WebSyncRegistration*>&, const WebSyncError&>;
 
-using WebSyncRegistrationCallbacks = WebCallbacks<WebSyncRegistration*, WebSyncError*>;
-using WebSyncNotifyWhenDoneCallbacks = WebCallbacks<bool*, WebSyncError*>;
-using WebSyncUnregistrationCallbacks = WebCallbacks<bool*, WebSyncError*>;
-using WebSyncGetRegistrationsCallbacks = WebCallbacks<WebVector<WebSyncRegistration*>*, WebSyncError*>;
-using WebSyncGetPermissionStatusCallbacks = WebCallbacks<WebSyncPermissionStatus*, WebSyncError*>;
+template <>
+class WebCallbacks<WebSyncPermissionStatus, const WebSyncError&> {
+public:
+    virtual ~WebCallbacks() {}
+    void onSuccess(WebSyncPermissionStatus* r)
+    {
+        onSuccess(*r);
+        delete r;
+    }
+    void onError(WebSyncError* e)
+    {
+        onError(*e);
+        delete e;
+    }
+    virtual void onSuccess(WebSyncPermissionStatus) = 0;
+    virtual void onError(const WebSyncError&) = 0;
+};
+using WebSyncGetPermissionStatusCallbacks = WebCallbacks<WebSyncPermissionStatus, const WebSyncError&>;
 
 class WebSyncProvider {
 public:
@@ -58,7 +125,7 @@ public:
     virtual void notifyWhenDone(int64_t syncId, WebSyncNotifyWhenDoneCallbacks* callbacks)
     {
         // TODO(jkarlin): After landing both legs of the notifyWhenDone CLs, make this a pure virtual function.
-        callbacks->onError(new WebSyncError(WebSyncError::ErrorTypeAbort, "Function not implemented."));
+        callbacks->onError(WebSyncError(WebSyncError::ErrorTypeAbort, "Function not implemented."));
         delete callbacks;
     }