Bug 203976 – Make DeferredPromise behave nicely with regards to the back/forward cache

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Make DeferredPromise behave nicely with regards to the back/forward cache. Right now, we require call sites to queue tasks to resolve / reject promises in order to play nicely with the back/forward cache.
Ideally, DeferredPromise would do this for us and developers could resolve/reject promises without worrying about the back/forward cache.

[youenn fablet]

I believe this is the way to fix ReadableStream as well.

[Ryosuke Niwa]

(In reply to Chris Dumez from comment #0)
> Make DeferredPromise behave nicely with regards to the back/forward cache.
> Right now, we require call sites to queue tasks to resolve / reject promises
> in order to play nicely with the back/forward cache.
> Ideally, DeferredPromise would do this for us and developers could
> resolve/reject promises without worrying about the back/forward cache.

Should we integrate DeferredPromise with EventLoop somehow?

[Chris Dumez]

(In reply to Ryosuke Niwa from comment #2)
> (In reply to Chris Dumez from comment #0)
> > Make DeferredPromise behave nicely with regards to the back/forward cache.
> > Right now, we require call sites to queue tasks to resolve / reject promises
> > in order to play nicely with the back/forward cache.
> > Ideally, DeferredPromise would do this for us and developers could
> > resolve/reject promises without worrying about the back/forward cache.
> 
> Should we integrate DeferredPromise with EventLoop somehow?

That's what I am doing. If we're not in back/forward cache, then resolve/reject as usual. If we're in the back/forward cache, then queue a task on the event loop to reject/resolve. Patch will be ready soon.

[Chris Dumez]

Created attachment 383076 [details]
WIP Patch

[Chris Dumez]

Created attachment 383080 [details]
WIP Patch

[Ryosuke Niwa]

Comment on attachment 383080 [details]
WIP Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383080&action=review

> Source/WebCore/Modules/cache/DOMCache.cpp:62
> -        enqueueTask([promise = WTFMove(promise), result = WTFMove(result)]() mutable {
> +        queueTaskKeepingObjectAlive(*this, TaskSource::DOMManipulation, [promise = WTFMove(promise), result = WTFMove(result)]() mutable {

Maybe we can make this change to DOMCache in a separate patch?

[Chris Dumez]

(In reply to Ryosuke Niwa from comment #6)
> Comment on attachment 383080 [details]
> WIP Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=383080&action=review
> 
> > Source/WebCore/Modules/cache/DOMCache.cpp:62
> > -        enqueueTask([promise = WTFMove(promise), result = WTFMove(result)]() mutable {
> > +        queueTaskKeepingObjectAlive(*this, TaskSource::DOMManipulation, [promise = WTFMove(promise), result = WTFMove(result)]() mutable {
> 
> Maybe we can make this change to DOMCache in a separate patch?

Sure.

[Chris Dumez]

Created attachment 383090 [details]
Patch

[Chris Dumez]

Comment on attachment 383080 [details]
WIP Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=383080&action=review

>>> Source/WebCore/Modules/cache/DOMCache.cpp:62
>>> +        queueTaskKeepingObjectAlive(*this, TaskSource::DOMManipulation, [promise = WTFMove(promise), result = WTFMove(result)]() mutable {
>> 
>> Maybe we can make this change to DOMCache in a separate patch?
> 
> Sure.

Will do this via https://bugs.webkit.org/show_bug.cgi?id=203985.

[WebKit Commit Bot]

Comment on attachment 383090 [details]
Patch

Rejecting attachment 383090 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 383090, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as [e-mail]...
Fetching: https://bugs.webkit.org/attachment.cgi?id=383090&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=203976&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Processing patch 383090 from bug 203976.
Fetching: https://bugs.webkit.org/attachment.cgi?id=383090
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Ryosuke Niwa']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 24 diffs from patch file(s).
patching file Source/JavaScriptCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/JavaScriptCore/heap/Handle.h
patching file Source/JavaScriptCore/heap/Strong.h
patching file Source/JavaScriptCore/heap/StrongInlines.h
patching file Source/WebCore/Modules/cache/DOMCacheStorage.cpp
patching file Source/WebCore/Modules/cache/DOMCacheStorage.h
patching file Source/WebCore/Modules/fetch/FetchBodyOwner.cpp
patching file Source/WebCore/Modules/fetch/FetchBodyOwner.h
patching file Source/WebCore/Modules/mediastream/UserMediaRequest.cpp
patching file Source/WebCore/WebCore.xcodeproj/project.pbxproj
Hunk #2 succeeded at 28744 (offset 29 lines).
Hunk #3 FAILED at 30675.
Hunk #4 FAILED at 31177.
2 out of 4 hunks FAILED -- saving rejects to file Source/WebCore/WebCore.xcodeproj/project.pbxproj.rej
patching file Source/WebCore/bindings/IDLTypes.h
patching file Source/WebCore/bindings/js/JSDOMGuardedObject.h
patching file Source/WebCore/bindings/js/JSDOMPromiseDeferred.cpp
patching file Source/WebCore/bindings/js/JSDOMPromiseDeferred.h
patching file Source/WebCore/css/FontFaceSet.cpp
patching file Source/WebCore/dom/ActiveDOMCallback.cpp
patching file Source/WebCore/dom/ActiveDOMCallback.h
patching file Source/WebCore/dom/Element.h
patching file Source/WebCore/dom/ScriptExecutionContext.h
patching file Source/WebCore/page/DOMWindow.h
patching file Source/WebCore/page/RemoteDOMWindow.h
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/mediastream/MediaDevices-getUserMedia-expected.txt

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Ryosuke Niwa']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/13227628

[Chris Dumez]

Created attachment 383158 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 383158 [details]
Patch

Clearing flags on attachment: 383158

Committed r252263: <https://trac.webkit.org/changeset/252263>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/57040422>

[Truitt Savell]

It looks like the changes in https://trac.webkit.org/changeset/252263/webkit

caused http/tests/navigation/page-cache-getUserMedia-pending-promise.html to crash.

History:
https://results.webkit.org/?suite=layout-tests&test=http%2Ftests%2Fnavigation%2Fpage-cache-getUserMedia-pending-promise.html

CRASHING TEST: /navigation/page-cache-getUserMedia-pending-promise.html

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x000000017ae83a40 WTFCrash + 16
1   com.apple.WebCore             	0x000000016272d85b WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.WebCore             	0x0000000164391b6b WebCore::UserMediaRequest::allow(WebCore::CaptureDevice&&, WebCore::CaptureDevice&&, WTF::String&&, WTF::CompletionHandler<void ()>&&)::$_2::operator()(WTF::RefPtr<WebCore::MediaStreamPrivate, WTF::DumbPtrTraits<WebCore::MediaStreamPrivate> >&&) + 587

Log:
https://build.webkit.org/results/Apple-Catalina-Debug-WK2-Tests/r252274%20(564)/http/tests/navigation/page-cache-getUserMedia-pending-promise-crash-log.txt

[Truitt Savell]

http/tests/navigation/page-cache-getUserMedia-pending-promise.html

is crashing on Mac Debug WK2

[Chris Dumez]

(In reply to Truitt Savell from comment #16)
> http/tests/navigation/page-cache-getUserMedia-pending-promise.html
> 
> is crashing on Mac Debug WK2

Looking.

[Chris Dumez]

(In reply to Truitt Savell from comment #16)
> http/tests/navigation/page-cache-getUserMedia-pending-promise.html
> 
> is crashing on Mac Debug WK2

Fixing via https://bugs.webkit.org/show_bug.cgi?id=204232.