Commit 122203e5 authored by nhiroki@chromium.org's avatar nhiroki@chromium.org

ServiceWorker: Bypass resolving a promise when ExecutionContext is gone

This change makes CallbackPromiseAdapter bypass resolving/rejecting a promise
if ExecutionContext is gone and calls WebType::dispose() to clean up passed
WebType instance.


BUG=385906,386501
TEST=run_webkit_tests.py --debug --enable-leak-detection http/tests/serviceworker/chromium/resolve-after-window-close.html
TEST=run_webkit_tests.py --debug --enable-leak-detection push_messaging/push-messaging-resolve-after-detached.html
TEST=manual (run crash.html in the issue)

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

git-svn-id: svn://svn.chromium.org/blink/trunk@179043 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 2c19c447
Test that resolving a promise after the window gets closed does not assert or crash
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE html>
<script src="/js-test-resources/js-test.js"></script>
<body>
<script>
window.jsTestIsAsync = true;
description('Test that resolving a promise after the window gets closed does not assert or crash');
if (window.testRunner) {
testRunner.setCanOpenWindows();
openWindow();
} else {
document.write('<p>Click <a href="javascript:openWindow()">this link</a>. A window should open and close without asserting or crashing.</p>');
}
function openWindow() {
window.open('resources/resolve-after-window-close.html');
}
function done() {
finishJSTest();
}
</script>
</body>
<html>
<script>
var scope = 'empty';
navigator.serviceWorker.unregister(scope).then(function() {
var promise = navigator.serviceWorker.register('empty-worker.js', { scope: scope });
window.close();
window.opener.done();
});
</script>
</html>
Test that resolving a promise after the window gets closed does not assert or crash.
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
PASS successfullyParsed is true
TEST COMPLETE
<!DOCTYPE html>
<html>
<head>
<script src="../resources/js-test.js"></script>
</head>
<body>
<script>
description("Test that resolving a promise after the window gets closed does not assert or crash.");
var jsTestIsAsync = true;
function openWindow() {
window.open('resources/push-messaging-resolve-after-detached.html');
}
function done() {
finishJSTest();
}
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.setCanOpenWindows();
openWindow();
}
</script>
</body>
</html>
<!DOCTYPE html>
<html>
<script>
var promise = navigator.push.register('senderId');
window.close();
window.opener.done();
</script>
</html>
...@@ -60,6 +60,13 @@ namespace blink { ...@@ -60,6 +60,13 @@ namespace blink {
// resolver->promise().then(...); // resolver->promise().then(...);
// } // }
// //
// // Called when aborting to resolve/reject a promise due to an empty
// // execution context.
// static void dispose(blink::WebMyClass* webInstance) {
// // delete as appropriate, but often it's just:
// delete webInstance;
// }
//
// Now when calling into a WebKit API that requires a WebCallbacks<blink::WebMyClass, blink::WebMyClass>*: // Now when calling into a WebKit API that requires a WebCallbacks<blink::WebMyClass, blink::WebMyClass>*:
// //
// // call signature: callSomeMethod(WebCallbacks<MyClass, MyClass>* callbacks) // // call signature: callSomeMethod(WebCallbacks<MyClass, MyClass>* callbacks)
...@@ -74,17 +81,28 @@ public: ...@@ -74,17 +81,28 @@ public:
CallbackPromiseAdapter(PassRefPtr<ScriptPromiseResolver> resolver) CallbackPromiseAdapter(PassRefPtr<ScriptPromiseResolver> resolver)
: m_resolver(resolver) : m_resolver(resolver)
{ {
ASSERT(m_resolver);
} }
virtual ~CallbackPromiseAdapter() { } virtual ~CallbackPromiseAdapter() { }
virtual void onSuccess(typename S::WebType* result) OVERRIDE virtual void onSuccess(typename S::WebType* result) OVERRIDE
{ {
if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
S::dispose(result);
return;
}
m_resolver->resolve(S::from(m_resolver.get(), result)); m_resolver->resolve(S::from(m_resolver.get(), result));
} }
virtual void onError(typename T::WebType* error) OVERRIDE virtual void onError(typename T::WebType* error) OVERRIDE
{ {
if (!m_resolver->executionContext() || m_resolver->executionContext()->activeDOMObjectsAreStopped()) {
T::dispose(error);
return;
}
m_resolver->reject(T::from(m_resolver.get(), error)); m_resolver->reject(T::from(m_resolver.get(), error));
} }
private: private:
RefPtr<ScriptPromiseResolver> m_resolver; RefPtr<ScriptPromiseResolver> m_resolver;
WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter); WTF_MAKE_NONCOPYABLE(CallbackPromiseAdapter);
......
...@@ -23,4 +23,9 @@ PassRefPtrWillBeRawPtr<DOMException> PushError::from(ScriptPromiseResolver*, Web ...@@ -23,4 +23,9 @@ PassRefPtrWillBeRawPtr<DOMException> PushError::from(ScriptPromiseResolver*, Web
return DOMException::create(UnknownError); return DOMException::create(UnknownError);
} }
void PushError::dispose(WebType* webErrorRaw)
{
delete webErrorRaw;
}
} // namespace blink } // namespace blink
...@@ -19,6 +19,7 @@ public: ...@@ -19,6 +19,7 @@ public:
// For CallbackPromiseAdapter. // For CallbackPromiseAdapter.
typedef blink::WebPushError WebType; typedef blink::WebPushError WebType;
static PassRefPtrWillBeRawPtr<DOMException> from(ScriptPromiseResolver*, WebType* webErrorRaw); static PassRefPtrWillBeRawPtr<DOMException> from(ScriptPromiseResolver*, WebType* webErrorRaw);
static void dispose(WebType* webErrorRaw);
private: private:
PushError() WTF_DELETED_FUNCTION; PushError() WTF_DELETED_FUNCTION;
......
...@@ -5,8 +5,21 @@ ...@@ -5,8 +5,21 @@
#include "config.h" #include "config.h"
#include "modules/push_messaging/PushRegistration.h" #include "modules/push_messaging/PushRegistration.h"
#include "wtf/OwnPtr.h"
namespace blink { namespace blink {
PushRegistration* PushRegistration::from(ScriptPromiseResolver*, WebType* registrationRaw)
{
OwnPtr<WebType> registration = adoptPtr(registrationRaw);
return new PushRegistration(registration->endpoint, registration->registrationId);
}
void PushRegistration::dispose(WebType* registrationRaw)
{
delete registrationRaw;
}
PushRegistration::PushRegistration(const String& pushEndpoint, const String& pushRegistrationId) PushRegistration::PushRegistration(const String& pushEndpoint, const String& pushRegistrationId)
: m_pushEndpoint(pushEndpoint) : m_pushEndpoint(pushEndpoint)
, m_pushRegistrationId(pushRegistrationId) , m_pushRegistrationId(pushRegistrationId)
......
...@@ -8,7 +8,6 @@ ...@@ -8,7 +8,6 @@
#include "bindings/core/v8/ScriptWrappable.h" #include "bindings/core/v8/ScriptWrappable.h"
#include "platform/heap/Handle.h" #include "platform/heap/Handle.h"
#include "public/platform/WebPushRegistration.h" #include "public/platform/WebPushRegistration.h"
#include "wtf/OwnPtr.h"
#include "wtf/text/WTFString.h" #include "wtf/text/WTFString.h"
namespace blink { namespace blink {
...@@ -19,11 +18,8 @@ class PushRegistration FINAL : public GarbageCollectedFinalized<PushRegistration ...@@ -19,11 +18,8 @@ class PushRegistration FINAL : public GarbageCollectedFinalized<PushRegistration
public: public:
// For CallbackPromiseAdapter. // For CallbackPromiseAdapter.
typedef blink::WebPushRegistration WebType; typedef blink::WebPushRegistration WebType;
static PushRegistration* from(ScriptPromiseResolver*, WebType* registrationRaw) static PushRegistration* from(ScriptPromiseResolver*, WebType* registrationRaw);
{ static void dispose(WebType* registrationRaw);
OwnPtr<WebType> registration = adoptPtr(registrationRaw);
return new PushRegistration(registration->endpoint, registration->registrationId);
}
virtual ~PushRegistration(); virtual ~PushRegistration();
......
...@@ -163,6 +163,12 @@ PassRefPtrWillBeRawPtr<ServiceWorker> ServiceWorker::from(ScriptPromiseResolver* ...@@ -163,6 +163,12 @@ PassRefPtrWillBeRawPtr<ServiceWorker> ServiceWorker::from(ScriptPromiseResolver*
return serviceWorker; return serviceWorker;
} }
void ServiceWorker::dispose(WebType* worker)
{
if (worker && !worker->proxy())
delete worker;
}
void ServiceWorker::setProxyState(ProxyState state) void ServiceWorker::setProxyState(ProxyState state)
{ {
if (m_proxyState == state) if (m_proxyState == state)
......
...@@ -57,6 +57,7 @@ public: ...@@ -57,6 +57,7 @@ public:
static PassRefPtrWillBeRawPtr<ServiceWorker> from(ScriptPromiseResolver*, WebType* worker); static PassRefPtrWillBeRawPtr<ServiceWorker> from(ScriptPromiseResolver*, WebType* worker);
static PassRefPtrWillBeRawPtr<ServiceWorker> from(ExecutionContext*, WebType*); static PassRefPtrWillBeRawPtr<ServiceWorker> from(ExecutionContext*, WebType*);
static void dispose(WebType*);
void postMessage(ExecutionContext*, PassRefPtr<SerializedScriptValue> message, const MessagePortArray*, ExceptionState&); void postMessage(ExecutionContext*, PassRefPtr<SerializedScriptValue> message, const MessagePortArray*, ExceptionState&);
......
...@@ -30,6 +30,10 @@ namespace { ...@@ -30,6 +30,10 @@ namespace {
} }
return clients; return clients;
} }
static void dispose(WebType* webClientsRaw)
{
delete webClientsRaw;
}
private: private:
WTF_MAKE_NONCOPYABLE(ClientArray); WTF_MAKE_NONCOPYABLE(ClientArray);
......
...@@ -130,7 +130,6 @@ ScriptPromise ServiceWorkerContainer::registerServiceWorker(ScriptState* scriptS ...@@ -130,7 +130,6 @@ ScriptPromise ServiceWorkerContainer::registerServiceWorker(ScriptState* scriptS
class UndefinedValue { class UndefinedValue {
public: public:
#ifdef DISABLE_SERVICE_WORKER_REGISTRATION #ifdef DISABLE_SERVICE_WORKER_REGISTRATION
typedef WebServiceWorker WebType; typedef WebServiceWorker WebType;
#else #else
...@@ -141,6 +140,10 @@ public: ...@@ -141,6 +140,10 @@ public:
ASSERT(!registration); // Anything passed here will be leaked. ASSERT(!registration); // Anything passed here will be leaked.
return V8UndefinedType(); return V8UndefinedType();
} }
static void dispose(WebServiceWorker* worker)
{
ASSERT(!worker); // Anything passed here will be leaked.
}
private: private:
UndefinedValue(); UndefinedValue();
......
...@@ -63,4 +63,10 @@ PassRefPtrWillBeRawPtr<DOMException> ServiceWorkerError::from(ScriptPromiseResol ...@@ -63,4 +63,10 @@ PassRefPtrWillBeRawPtr<DOMException> ServiceWorkerError::from(ScriptPromiseResol
return DOMException::create(UnknownError); return DOMException::create(UnknownError);
} }
// static
void ServiceWorkerError::dispose(WebType* webErrorRaw)
{
delete webErrorRaw;
}
} // namespace blink } // namespace blink
...@@ -45,6 +45,7 @@ public: ...@@ -45,6 +45,7 @@ public:
// For CallbackPromiseAdapter // For CallbackPromiseAdapter
typedef blink::WebServiceWorkerError WebType; typedef blink::WebServiceWorkerError WebType;
static PassRefPtrWillBeRawPtr<DOMException> from(ScriptPromiseResolver*, WebType* webErrorRaw); static PassRefPtrWillBeRawPtr<DOMException> from(ScriptPromiseResolver*, WebType* webErrorRaw);
static void dispose(WebType* webErrorRaw);
private: private:
WTF_MAKE_NONCOPYABLE(ServiceWorkerError); WTF_MAKE_NONCOPYABLE(ServiceWorkerError);
......
...@@ -31,6 +31,10 @@ public: ...@@ -31,6 +31,10 @@ public:
ASSERT(!registration); // Anything passed here will be leaked. ASSERT(!registration); // Anything passed here will be leaked.
return V8UndefinedType(); return V8UndefinedType();
} }
static void dispose(WebType* registration)
{
ASSERT(!registration); // Anything passed here will be leaked.
}
private: private:
UndefinedValue(); UndefinedValue();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment