Attachment 341202 Details for Bug 181499 – Patch

[patch] Patch

bug-181499-20180524101504.patch (text/plain), 10.10 KB, created by Chris Dumez on 2018-05-24 10:15:05 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-05-24 10:15:05 PDT

Size: 10.10 KB

patch

obsolete

>Subversion Revision: 232081
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index d54854e3ffbbacc1fbd115752f69ceac2105936d..d6844d4841ac70e64d9a6a1eb2f4cf79e6fa747a 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2018-05-23  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-22  Brent Fulgham  <bfulgham@apple.com>
> 
>         Avoid keeping FormState alive longer than necessary
>diff --git a/Source/WebCore/bindings/js/JSDOMPromise.cpp b/Source/WebCore/bindings/js/JSDOMPromise.cpp
>index 979fa5c720f18687166d86e852c3b48da29e79c7..7a88d8305569a2e73852d9245b83b81232518150 100644
>--- a/Source/WebCore/bindings/js/JSDOMPromise.cpp
>+++ b/Source/WebCore/bindings/js/JSDOMPromise.cpp
>@@ -53,16 +53,20 @@ static inline JSC::JSValue callFunction(JSC::ExecState& state, JSC::JSValue jsFu
> 
> 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());
>     });
> 
>     const JSC::Identifier& privateName = vm.propertyNames->builtinNames().thenPrivateName();
>-    auto* promise = this->promise();
>     auto thenFunction = promise->get(&state, privateName);
>     ASSERT(thenFunction.isFunction(vm));
> 
>diff --git a/Source/WebCore/bindings/js/JSDOMPromise.h b/Source/WebCore/bindings/js/JSDOMPromise.h
>index ecdb6db6c23085206e9c15ac0f8e8e18657f68b4..a9fb62318d882759104078df01f61e4f3775d861 100644
>--- a/Source/WebCore/bindings/js/JSDOMPromise.h
>+++ b/Source/WebCore/bindings/js/JSDOMPromise.h
>@@ -50,6 +50,8 @@ public:
>     enum class Status { Pending, Fulfilled, Rejected };
>     Status status() const;
> 
>+    static void whenPromiseIsSettled(JSDOMGlobalObject*, JSC::JSObject* promise, std::function<void()>&&);
>+
> private:
>     DOMPromise(JSDOMGlobalObject& globalObject, JSC::JSPromise& promise)
>         : DOMGuarded<JSC::JSPromise>(globalObject, promise)
>diff --git a/Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp b/Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp
>index 47f541cd0333b40314c3a1a0cab9bfb48e2cd0c4..2bc257d5c93ffe0ff04ece53f6645732f25d8608 100644
>--- a/Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp
>+++ b/Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp
>@@ -27,6 +27,7 @@
> #include "JSDOMPromiseDeferred.h"
> 
> #include "DOMWindow.h"
>+#include "JSDOMPromise.h"
> #include "JSDOMWindow.h"
> #include <JavaScriptCore/BuiltinNames.h>
> #include <JavaScriptCore/Exception.h>
>@@ -68,6 +69,11 @@ void DeferredPromise::callFunction(ExecState& exec, JSValue function, JSValue re
>         clear();
> }
> 
>+void DeferredPromise::whenSettled(std::function<void()>&& callback)
>+{
>+    DOMPromise::whenPromiseIsSettled(globalObject(), deferred()->promise(), WTFMove(callback));
>+}
>+
> void DeferredPromise::reject()
> {
>     if (isSuspended())
>diff --git a/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h b/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h
>index 7961fb54ffed1779b7a9788e03f6e74bb839f071..2b7af0d31e611eae6bc6a7880b4251f5921c6475 100644
>--- a/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h
>+++ b/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h
>@@ -134,6 +134,8 @@ public:
> 
>     JSC::JSValue promise() const;
> 
>+    void whenSettled(std::function<void()>&&);
>+
> private:
>     DeferredPromise(JSDOMGlobalObject& globalObject, JSC::JSPromiseDeferred& deferred, Mode mode)
>         : DOMGuarded<JSC::JSPromiseDeferred>(globalObject, deferred)
>@@ -144,6 +146,7 @@ private:
>     JSC::JSPromiseDeferred* deferred() const { return guarded(); }
> 
>     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); }
> 
>diff --git a/Source/WebCore/workers/service/ServiceWorkerContainer.cpp b/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
>index 115f3371b5be2d554a7b3b5b646e88ec3c04a034..5edbc53b505b6a0b854d9f852c2f9638762f9f2d 100644
>--- a/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
>+++ b/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
>@@ -419,28 +419,31 @@ void ServiceWorkerContainer::jobResolvedWithRegistration(ServiceWorkerJob& job,
>         CONTAINER_RELEASE_LOG_IF_ALLOWED("jobResolvedWithRegistration: Update job %" PRIu64 " succeeded", job.identifier().toUInt64());
>     }
> 
>-    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);
>             });
>         };
>     }
> 
>     if (isStopped()) {
>-        notifyWhenResolvedIfNeeded();
>+        if (notifyWhenResolvedIfNeeded)
>+            notifyWhenResolvedIfNeeded();
>         return;
>     }
> 
>     if (!job.promise()) {
>-        notifyWhenResolvedIfNeeded();
>+        if (notifyWhenResolvedIfNeeded)
>+            notifyWhenResolvedIfNeeded();
>         return;
>     }
> 
>     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;
>         }
> 
>@@ -448,9 +451,13 @@ void ServiceWorkerContainer::jobResolvedWithRegistration(ServiceWorkerJob& job,
> 
>         CONTAINER_RELEASE_LOG_IF_ALLOWED("jobResolvedWithRegistration: Resolving promise for job %" PRIu64 ". Registration ID: %" PRIu64, job->identifier().toUInt64(), registration->identifier().toUInt64());
> 
>-        job->promise()->resolve<IDLInterface<ServiceWorkerRegistration>>(WTFMove(registration));
>+        if (notifyWhenResolvedIfNeeded) {
>+            job->promise()->whenSettled([notifyWhenResolvedIfNeeded = WTFMove(notifyWhenResolvedIfNeeded)] {
>+                notifyWhenResolvedIfNeeded();
>+            });
>+        }
> 
>-        notifyWhenResolvedIfNeeded();
>+        job->promise()->resolve<IDLInterface<ServiceWorkerRegistration>>(WTFMove(registration));
>     });
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 778a0572489e0100098fd27dc5afbaed7464c75d..bfde6dd1c9e4e125dc00d2df63ad541231837f85 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-05-23  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-22  David Fenton  <david_fenton@apple.com>
> 
>         Update test expectations for newly imported css-text layout tests
>diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
>index c328b9e6cf5ff513c9d4e130f23580c2c67df778..eb9ed736a4b54320b4c4d42dd0e4f381fee97ede 100644
>--- a/LayoutTests/platform/ios/TestExpectations
>+++ b/LayoutTests/platform/ios/TestExpectations
>@@ -3267,8 +3267,6 @@ webkit.org/b/180062 fast/text/user-installed-fonts/shadow-postscript.html [ Imag
> webkit.org/b/180062 fast/text/user-installed-fonts/shadow.html [ ImageOnlyFailure ]
> 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 ]
> 
> webkit.org/b/182422 imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-origin.sub.html [ Skip ]

Actions: View | Formatted Diff | Diff

Attachments on bug 181499: 341134 | 341145 | 341153 | 341202