Bug 202244

Summary: Add very basic PageCache support for RTCPeerConnection
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebRTCAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, commit-queue, eric.carlson, ews-watchlist, ggaren, glenn, hta, jer.noble, philipj, sergio, tommyw, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 202293    
Attachments:
Description Flags
Patch
none
Patch none

Chris Dumez
Reported 2019-09-25 15:14:33 PDT
Add very basic PageCache support for RTCPeerConnection.
Attachments
Patch (9.97 KB, patch)
2019-09-25 16:19 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (12.47 KB, patch)
2019-09-25 19:32 PDT, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2019-09-25 16:19:49 PDT
Created attachment 379588 [details] Patch
Radar WebKit Bug Importer
Comment 2 2019-09-25 16:20:19 PDT
<rdar://problem/55723524>
Alex Christensen
Comment 3 2019-09-25 17:12:01 PDT
Comment on attachment 379588 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379588&action=review > Source/WebCore/Modules/mediastream/RTCPeerConnection.h:190 > + promise.whenSettled([pendingActivity = makeRefPtr(makePendingActivity(*this).ptr())] { }); Why not just make whenSettled take a WTF::Function?
Chris Dumez
Comment 4 2019-09-25 19:32:00 PDT
Created attachment 379613 [details] Patch
Geoffrey Garen
Comment 5 2019-09-25 20:51:05 PDT
Comment on attachment 379613 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=379613&action=review r=me > Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:502 > + // FIXME: We should do better here, it is way too easy to prevent PageCache. > bool RTCPeerConnection::canSuspendForDocumentSuspension() const > { > return !hasPendingActivity(); I kinda think we should be able to remove this. Pending promised will be resolved before any navigation. And events are OK to delay.
Chris Dumez
Comment 6 2019-09-25 20:55:49 PDT
(In reply to Geoffrey Garen from comment #5) > Comment on attachment 379613 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=379613&action=review > > r=me > > > Source/WebCore/Modules/mediastream/RTCPeerConnection.cpp:502 > > + // FIXME: We should do better here, it is way too easy to prevent PageCache. > > bool RTCPeerConnection::canSuspendForDocumentSuspension() const > > { > > return !hasPendingActivity(); > > I kinda think we should be able to remove this. Pending promised will be > resolved before any navigation. And events are OK to delay. Promises are guaranteed to be resolved before navigating away? What if I call a JS function that returns a promise from inside the pagehide event handler?
Geoffrey Garen
Comment 7 2019-09-25 21:00:21 PDT
> Promises are guaranteed to be resolved before navigating away? What if I > call a JS function that returns a promise from inside the pagehide event > handler? Maybe I misread this code. I guess this is a set of promises that will resolve when future events happen? Yeah, I guess we would need to implement an explicit way to delay them if so.
WebKit Commit Bot
Comment 8 2019-09-25 22:32:07 PDT
Comment on attachment 379613 [details] Patch Clearing flags on attachment: 379613 Committed r250379: <https://trac.webkit.org/changeset/250379>
WebKit Commit Bot
Comment 9 2019-09-25 22:32:09 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.