Changeset 232156 in webkit

Timestamp:

May 24, 2018, 11:10:05 AM (8 years ago)

Author:

Chris Dumez

Message:

[iOS WK2] Layout Test imported/w3c/web-platform-tests/service-workers/service-worker/update-after-navigation-fetch-event.https.html is a flaky failure
​https://bugs.webkit.org/show_bug.cgi?id=181499
<rdar://problem/36443428>

Reviewed by Youenn Fablet.

Source/WebCore:

After resolving a registration promise, we send an IPC back to the StorageProcess
for synchronization purposes, to make sure the registration does not get updated
before the promise's JS code has been executed. However, resolving a promise
schedules a microtask to run the JS and we would therefore send the IPC back too
early, thus causing flakiness. We now only send the IPC back back only after that
microtask has run and the JS has been executed.

• bindings/js/JSDOMPromiseDeferred.cpp:

(WebCore::DeferredPromise::callFunction):
(WebCore::DeferredPromise::whenSettled):

• bindings/js/JSDOMPromiseDeferred.h:
• workers/service/ServiceWorkerContainer.cpp:

(WebCore::ServiceWorkerContainer::jobResolvedWithRegistration):

LayoutTests:

Unskip test that should no longer be flaky.

• platform/ios/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/ios/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/bindings/js/JSDOMPromise.cpp (modified) (2 diffs)
• Source/WebCore/bindings/js/JSDOMPromise.h (modified) (1 diff)
• Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp (modified) (2 diffs)
• Source/WebCore/bindings/js/JSDOMPromiseDeferred.h (modified) (2 diffs)
• Source/WebCore/workers/service/ServiceWorkerContainer.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-05-24  Chris Dumez  <cdumez@apple.com>
+
+        [iOS WK2] Layout Test imported/w3c/web-platform-tests/service-workers/service-worker/update-after-navigation-fetch-event.https.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=181499
+        <rdar://problem/36443428>
+
+        Reviewed by Youenn Fablet.
+
+        Unskip test that should no longer be flaky.
+
+        * platform/ios/TestExpectations:
+
 2018-05-24  Jinho Bang  <zino@chromium.org>
 

trunk/LayoutTests/platform/ios/TestExpectations

@@ -3268 +3268 @@
 webkit.org/b/180062 fast/text/user-installed-fonts/shadow-postscript-family.html [ ImageOnlyFailure ]
 
-webkit.org/b/181499 [ Release ] imported/w3c/web-platform-tests/service-workers/service-worker/update-after-navigation-fetch-event.https.html [ Pass Failure ]
-
 webkit.org/b/181838 js/slow-stress/Int32Array-alloc-huge-long-lived.html [ Slow ]
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-05-24  Chris Dumez  <cdumez@apple.com>
+
+        [iOS WK2] Layout Test imported/w3c/web-platform-tests/service-workers/service-worker/update-after-navigation-fetch-event.https.html is a flaky failure
+        https://bugs.webkit.org/show_bug.cgi?id=181499
+        <rdar://problem/36443428>
+
+        Reviewed by Youenn Fablet.
+
+        After resolving a registration promise, we send an IPC back to the StorageProcess
+        for synchronization purposes, to make sure the registration does not get updated
+        before the promise's JS code has been executed. However, resolving a promise
+        schedules a microtask to run the JS and we would therefore send the IPC back too
+        early, thus causing flakiness. We now only send the IPC back back only after that
+        microtask has run and the JS has been executed.
+
+        * bindings/js/JSDOMPromiseDeferred.cpp:
+        (WebCore::DeferredPromise::callFunction):
+        (WebCore::DeferredPromise::whenSettled):
+        * bindings/js/JSDOMPromiseDeferred.h:
+        * workers/service/ServiceWorkerContainer.cpp:
+        (WebCore::ServiceWorkerContainer::jobResolvedWithRegistration):
+
 2018-05-24  Jinho Bang  <zino@chromium.org>
 

trunk/Source/WebCore/bindings/js/JSDOMPromise.cpp

@@ -54 +54 @@
 void DOMPromise::whenSettled(std::function<void()>&& callback)
 {
-    auto& state = *globalObject()->globalExec();
+    whenPromiseIsSettled(globalObject(), promise(), WTFMove(callback));
+}
+
+void DOMPromise::whenPromiseIsSettled(JSDOMGlobalObject* globalObject, JSC::JSObject* promise, std::function<void()>&& callback)
+{
+    auto& state = *globalObject->globalExec();
     auto& vm = state.vm();
     JSLockHolder lock(vm);
-    auto* handler = JSC::JSNativeStdFunction::create(vm, globalObject(), 1, String { }, [callback = WTFMove(callback)] (ExecState*) mutable {
+    auto* handler = JSC::JSNativeStdFunction::create(vm, globalObject, 1, String { }, [callback = WTFMove(callback)] (ExecState*) mutable {
         callback();
         return JSC::JSValue::encode(JSC::jsUndefined());
@@ -63 +68 @@
 
     const JSC::Identifier& privateName = vm.propertyNames->builtinNames().thenPrivateName();
-    auto* promise = this->promise();
     auto thenFunction = promise->get(&state, privateName);
     ASSERT(thenFunction.isFunction(vm));

trunk/Source/WebCore/bindings/js/JSDOMPromise.h

@@ -51 +51 @@
     Status status() const;
 
+    static void whenPromiseIsSettled(JSDOMGlobalObject*, JSC::JSObject* promise, std::function<void()>&&);
+
 private:
     DOMPromise(JSDOMGlobalObject& globalObject, JSC::JSPromise& promise)

trunk/Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp

@@ -28 +28 @@
 
 #include "DOMWindow.h"
+#include "JSDOMPromise.h"
 #include "JSDOMWindow.h"
 #include <JavaScriptCore/BuiltinNames.h>
@@ -69 +70 @@
 }
 
+void DeferredPromise::whenSettled(std::function<void()>&& callback)
+{
+    DOMPromise::whenPromiseIsSettled(globalObject(), deferred()->promise(), WTFMove(callback));
+}
+
 void DeferredPromise::reject()
 {

trunk/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h

@@ -135 +135 @@
     JSC::JSValue promise() const;
 
+    void whenSettled(std::function<void()>&&);
+
 private:
     DeferredPromise(JSDOMGlobalObject& globalObject, JSC::JSPromiseDeferred& deferred, Mode mode)
@@ -145 +147 @@
 
     WEBCORE_EXPORT void callFunction(JSC::ExecState&, JSC::JSValue function, JSC::JSValue resolution);
+
     void resolve(JSC::ExecState& state, JSC::JSValue resolution) { callFunction(state, deferred()->resolve(), resolution); }
     void reject(JSC::ExecState& state, JSC::JSValue resolution) { callFunction(state, deferred()->reject(), resolution); }

trunk/Source/WebCore/workers/service/ServiceWorkerContainer.cpp

@@ -420 +420 @@
     }
 
-    WTF::Function<void()> notifyWhenResolvedIfNeeded = [] { };
+    std::function<void()> notifyWhenResolvedIfNeeded;
     if (shouldNotifyWhenResolved == ShouldNotifyWhenResolved::Yes) {
-        notifyWhenResolvedIfNeeded = [connection = m_swConnection, registrationKey = data.key.isolatedCopy()]() mutable {
-            callOnMainThread([connection = WTFMove(connection), registrationKey = WTFMove(registrationKey)] {
+        notifyWhenResolvedIfNeeded = [connection = m_swConnection, registrationKey = data.key]() mutable {
+            callOnMainThread([connection = WTFMove(connection), registrationKey = registrationKey.isolatedCopy()] {
                 connection->didResolveRegistrationPromise(registrationKey);
             });
@@ -430 +430 @@
 
     if (isStopped()) {
-        notifyWhenResolvedIfNeeded();
+        if (notifyWhenResolvedIfNeeded)
+            notifyWhenResolvedIfNeeded();
         return;
     }
 
     if (!job.promise()) {
-        notifyWhenResolvedIfNeeded();
+        if (notifyWhenResolvedIfNeeded)
+            notifyWhenResolvedIfNeeded();
         return;
     }
@@ -441 +443 @@
     scriptExecutionContext()->postTask([this, protectedThis = makeRef(*this), job = makeRef(job), data = WTFMove(data), notifyWhenResolvedIfNeeded = WTFMove(notifyWhenResolvedIfNeeded)](ScriptExecutionContext& context) mutable {
         if (isStopped() || !context.sessionID().isValid()) {
-            notifyWhenResolvedIfNeeded();
+            if (notifyWhenResolvedIfNeeded)
+                notifyWhenResolvedIfNeeded();
             return;
         }
@@ -449 +452 @@
         CONTAINER_RELEASE_LOG_IF_ALLOWED("jobResolvedWithRegistration: Resolving promise for job %" PRIu64 ". Registration ID: %" PRIu64, job->identifier().toUInt64(), registration->identifier().toUInt64());
 
+        if (notifyWhenResolvedIfNeeded) {
+            job->promise()->whenSettled([notifyWhenResolvedIfNeeded = WTFMove(notifyWhenResolvedIfNeeded)] {
+                notifyWhenResolvedIfNeeded();
+            });
+        }
+
         job->promise()->resolve<IDLInterface<ServiceWorkerRegistration>>(WTFMove(registration));
-
-        notifyWhenResolvedIfNeeded();
     });
 }