Summary: | Make DeferredPromise behave nicely with regards to the back/forward cache | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | achristensen, cmarcelo, commit-queue, dbates, eric.carlson, esprehn+autocc, ews-watchlist, ggaren, glenn, gyuyoung.kim, hta, jer.noble, kangil.han, keith_miller, macpherson, mark.lam, menard, msaboff, philipj, rniwa, saam, sergio, tommyw, tsavell, tzagallo, webkit-bug-importer, youennf, ysuzuki | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=203101 https://bugs.webkit.org/show_bug.cgi?id=204232 |
||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 202293 | ||||||||||||
Attachments: |
|
Description
Chris Dumez
2019-11-07 13:33:40 PST
I believe this is the way to fix ReadableStream as well. (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? (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. Created attachment 383076 [details]
WIP Patch
Created attachment 383080 [details]
WIP Patch
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? (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. Created attachment 383090 [details]
Patch
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. 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 commit-queue@webkit.org... 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 Created attachment 383158 [details]
Patch
Comment on attachment 383158 [details] Patch Clearing flags on attachment: 383158 Committed r252263: <https://trac.webkit.org/changeset/252263> All reviewed patches have been landed. Closing bug. 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 http/tests/navigation/page-cache-getUserMedia-pending-promise.html is crashing on Mac Debug WK2 (In reply to Truitt Savell from comment #16) > http/tests/navigation/page-cache-getUserMedia-pending-promise.html > > is crashing on Mac Debug WK2 Looking. (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. |