Attachment 341134 Details for Bug 181499 – Patch

[patch] Patch

bug-181499-20180523150942.patch (text/plain), 9.26 KB, created by Chris Dumez on 2018-05-23 15:09:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-05-23 15:09:42 PDT

Size: 9.26 KB

patch

obsolete

>Subversion Revision: 232081
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index d54854e3ffbbacc1fbd115752f69ceac2105936d..c377bc77d4dbc406d7a80566cb9977dc05aa6fe9 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 NOBODY (OOPS!).
>+
>+        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/JSDOMPromiseDeferred.cpp b/Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp
>index 47f541cd0333b40314c3a1a0cab9bfb48e2cd0c4..a7ad00f4417f50d0cd3ffb002e9e1d18deb0c59f 100644
>--- a/Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp
>+++ b/Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp
>@@ -42,30 +42,53 @@ JSC::JSValue DeferredPromise::promise() const
>     return deferred()->promise();
> }
> 
>-void DeferredPromise::callFunction(ExecState& exec, JSValue function, JSValue resolution)
>+void DeferredPromise::callFunction(ExecState& state, JSValue function, JSValue resolution)
> {
>-    if (!canInvokeCallback())
>-        return;
>-
>-    VM& vm = exec.vm();
>-    auto scope = DECLARE_THROW_SCOPE(vm);
>-
>-    CallData callData;
>-    CallType callType = getCallData(function, callData);
>-    ASSERT(callType != CallType::None);
>-
>     MarkedArgumentBuffer arguments;
>     arguments.append(resolution);
>     ASSERT(!arguments.hasOverflowed());
> 
>-    call(&exec, function, callType, callData, jsUndefined(), arguments);
>+    callFunction(state, function, jsUndefined(), arguments);
>+
>+    if (m_mode == Mode::ClearPromiseOnResolve)
>+        clear();
>+}
>+
>+void DeferredPromise::callFunction(JSC::ExecState& state, JSC::JSValue jsFunction, JSC::JSValue thisValue, const JSC::ArgList& arguments)
>+{
>+    if (!canInvokeCallback())
>+        return;
>+
>+    auto scope = DECLARE_THROW_SCOPE(state.vm());
>+    JSC::CallData callData;
>+    auto callType = JSC::getCallData(jsFunction, callData);
>+    RELEASE_ASSERT(callType != JSC::CallType::None);
>+    call(&state, jsFunction, callType, callData, thisValue, arguments);
> 
>     // DeferredPromise should only be used by internal implementations that are well behaved.
>     // In practice, the only exception we should ever see here is the TerminatedExecutionException.
>     EXCEPTION_ASSERT_UNUSED(scope, !scope.exception() || isTerminatedExecutionException(vm, scope.exception()));
>+}
> 
>-    if (m_mode == Mode::ClearPromiseOnResolve)
>-        clear();
>+void DeferredPromise::whenSettled(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 {
>+        callback();
>+        return JSC::JSValue::encode(JSC::jsUndefined());
>+    });
>+
>+    const JSC::Identifier& privateName = vm.propertyNames->builtinNames().thenPrivateName();
>+    auto* promise = this->deferred()->promise();
>+    auto thenFunction = promise->get(&state, privateName);
>+    RELEASE_ASSERT(thenFunction.isFunction(vm));
>+
>+    JSC::MarkedArgumentBuffer arguments;
>+    arguments.append(handler);
>+    arguments.append(handler);
>+    callFunction(state, thenFunction, promise, arguments);
> }
> 
> void DeferredPromise::reject()
>diff --git a/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h b/Source/WebCore/bindings/js/JSDOMPromiseDeferred.h
>index 7961fb54ffed1779b7a9788e03f6e74bb839f071..fb2af6700b7dd10ab8849515ace2f524dc6fd122 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,8 @@ private:
>     JSC::JSPromiseDeferred* deferred() const { return guarded(); }
> 
>     WEBCORE_EXPORT void callFunction(JSC::ExecState&, JSC::JSValue function, JSC::JSValue resolution);
>+    void callFunction(JSC::ExecState&, JSC::JSValue function, JSC::JSValue, const JSC::ArgList& arguments);
>+
>     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..73ce2e329811770486e7161099788b26ea79bcb9 100644
>--- a/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
>+++ b/Source/WebCore/workers/service/ServiceWorkerContainer.cpp
>@@ -419,10 +419,10 @@ 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);
>             });
>         };
>@@ -448,9 +448,11 @@ 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));
>+        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..8b0a416e97d0e25fd1098f39016c7018605d395a 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 NOBODY (OOPS!).
>+
>+        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