Don't assert when ServiceWorker::from is passed an empty promise

If the document gets detached before Service Worker registration completes, the CallbackPromiseAdapter promise gets cleared. Then when registration completes, ServiceWorker::from is passed the resolver with the empty promise. Before this patch, an assert would then occur because ScriptPromise::then bails before adopting its fulfilled/rejected arguments. This patch fixes an overly aggressive assert in V8GarbageCollected and also makes ServiceWorker creation skip waiting on an empty promise. BUG=384498 Review URL: https://codereview.chromium.org/337053004 git-svn-id: svn://svn.chromium.org/blink/trunk@176282 bbb929c8-8fbe-4397-9dbb-9b2b20218538

Don't assert when ServiceWorker::from is passed an empty promise
If the document gets detached before Service Worker registration completes, the CallbackPromiseAdapter promise gets cleared. Then when registration completes, ServiceWorker::from is passed the resolver with the empty promise. Before this patch, an assert would then occur because ScriptPromise::then bails before adopting its fulfilled/rejected arguments. This patch fixes an overly aggressive assert in V8GarbageCollected and also makes ServiceWorker creation skip waiting on an empty promise. BUG=384498 Review URL: https://codereview.chromium.org/337053004 git-svn-id: svn://svn.chromium.org/blink/trunk@176282 bbb929c8-8fbe-4397-9dbb-9b2b20218538
82940c19 · falken@chromium.org · 1a0d7748 · 82940c19 · 82940c19 · 82940c19
Commit 82940c19 authored Jun 17, 2014 by falken@chromium.org
6 changed files
--- a/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/window-close-during-registration.html
+++ b/third_party/WebKit/LayoutTests/http/tests/serviceworker/resources/window-close-during-registration.html
+<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();
+    return promise;
+}).catch(function(error) {
+    window.opener.testFailed(error.message);
+    window.opener.done();
+});
+</script>
+</html>
--- a/third_party/WebKit/LayoutTests/http/tests/serviceworker/window-close-during-registration-expected.txt
+++ b/third_party/WebKit/LayoutTests/http/tests/serviceworker/window-close-during-registration-expected.txt
+Test that closing the window during Service Worker registration does not assert or crash
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+FAIL Service Worker support is disabled.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
--- a/third_party/WebKit/LayoutTests/http/tests/serviceworker/window-close-during-registration.html
+++ b/third_party/WebKit/LayoutTests/http/tests/serviceworker/window-close-during-registration.html
+<!DOCTYPE html>
+<script src="/js-test-resources/js-test.js"></script>
+<body>
+<script>
+window.jsTestIsAsync = true;
+description('Test that closing the window during Service Worker registration 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/window-close-during-registration.html');
+}
+
+function done() {
+    setTimeout(function() {
+        finishJSTest();
+    }, 100);
+}
+</script>
+</body>
--- a/third_party/WebKit/LayoutTests/virtual/serviceworker/http/tests/serviceworker/window-close-during-registration-expected.txt
+++ b/third_party/WebKit/LayoutTests/virtual/serviceworker/http/tests/serviceworker/window-close-during-registration-expected.txt
+Test that closing the window during Service Worker registration 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
+
--- a/third_party/WebKit/Source/bindings/v8/V8GarbageCollected.h
+++ b/third_party/WebKit/Source/bindings/v8/V8GarbageCollected.h
@@ -51,12 +51,14 @@ protected:
    V8GarbageCollected(v8::Isolate* isolate)
        : m_isolate(isolate)
        , m_handle(isolate, v8::External::New(isolate, static_cast<T*>(this)))
+        , m_releasedToV8GarbageCollector(false)
    {
    }

    v8::Handle<v8::External> releaseToV8GarbageCollector()
    {
        ASSERT(!m_handle.isWeak()); // Call this exactly once.
+        m_releasedToV8GarbageCollector = true;
        v8::Handle<v8::External> result = m_handle.newLocal(m_isolate);
        m_handle.setWeak(static_cast<T*>(this), &weakCallback);
        return result;
@@ -64,7 +66,7 @@ protected:

    ~V8GarbageCollected()
    {
-        ASSERT(m_handle.isEmpty());
+        ASSERT(!m_releasedToV8GarbageCollector || m_handle.isEmpty());
    }

    v8::Isolate* isolate() { return m_isolate; }
@@ -79,6 +81,7 @@ private:

    v8::Isolate* m_isolate;
    ScopedPersistent<v8::External> m_handle;
+    bool m_releasedToV8GarbageCollector;
 };

 } // namespace WebCore

--- a/third_party/WebKit/Source/modules/serviceworkers/ServiceWorker.cpp
+++ b/third_party/WebKit/Source/modules/serviceworkers/ServiceWorker.cpp
@@ -195,6 +195,12 @@ void ServiceWorker::onPromiseResolved()

 void ServiceWorker::waitOnPromise(ScriptPromise promise)
 {
+    if (promise.isEmpty()) {
+        // The document was detached during registration. The state doesn't really
+        // matter since this ServiceWorker will immediately die.
+        setProxyState(ContextStopped);
+        return;
+    }
    setProxyState(RegisterPromisePending);
    promise.then(ThenFunction::create(this));
 }