RESOLVED FIXED 192318
Update process assertions after swapping processes
https://bugs.webkit.org/show_bug.cgi?id=192318
Summary Update process assertions after swapping processes
Alex Christensen
Reported 2018-12-03 12:58:57 PST
Update process assertions after swapping processes
Attachments
Patch (4.96 KB, patch)
2018-12-03 13:02 PST, Alex Christensen
no flags
Patch (5.49 KB, patch)
2018-12-03 13:07 PST, Alex Christensen
no flags
Alex Christensen
Comment 1 2018-12-03 13:02:25 PST
Alex Christensen
Comment 2 2018-12-03 13:02:27 PST
Chris Dumez
Comment 3 2018-12-03 13:06:22 PST
Comment on attachment 356395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=356395&action=review > Source/WebKit/ChangeLog:10 > + after navigation and process swapping we need the new WebProcess to update its process assertions. typo: and process swapping -> and *after* ? > Source/WebKit/UIProcess/Cocoa/NavigationState.mm:1232 > + // Take the process assertion from the old web process. I would say: // Transfer our background assertion from the old process to the new one.
Alex Christensen
Comment 4 2018-12-03 13:07:33 PST
EWS Watchlist
Comment 5 2018-12-03 13:10:18 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Alex Christensen
Comment 6 2018-12-03 13:20:17 PST
Chris Dumez
Comment 8 2018-12-03 16:45:18 PST
(In reply to Truitt Savell from comment #7) > It looks like the changes in https://trac.webkit.org/changeset/238817/webkit > > may have caused sever process swap api failures on iOS. > > build: > https://build.webkit.org/builders/ > Apple%20iOS%2012%20Simulator%20Release%20WK2%20%28Tests%29/builds/1316 > > output: > https://build.webkit.org/builders/ > Apple%20iOS%2012%20Simulator%20Release%20WK2%20%28Tests%29/builds/1316/steps/ > run-api-tests/logs/stdio Seems to indicate Page Cache is not working. My bet is that the suspension relies on the process assertion on the old process, which gets dropped in this patch when we take the assertion for the new process. If so, we may need to SuspendedPageProxy to hold a background token for its WebProcess until SuspendedPageProxy::didFinishLoad() is called.
Chris Dumez
Comment 9 2018-12-03 20:16:26 PST
(In reply to Chris Dumez from comment #8) > (In reply to Truitt Savell from comment #7) > > It looks like the changes in https://trac.webkit.org/changeset/238817/webkit > > > > may have caused sever process swap api failures on iOS. > > > > build: > > https://build.webkit.org/builders/ > > Apple%20iOS%2012%20Simulator%20Release%20WK2%20%28Tests%29/builds/1316 > > > > output: > > https://build.webkit.org/builders/ > > Apple%20iOS%2012%20Simulator%20Release%20WK2%20%28Tests%29/builds/1316/steps/ > > run-api-tests/logs/stdio > > Seems to indicate Page Cache is not working. My bet is that the suspension > relies on the process assertion on the old process, which gets dropped in > this patch when we take the assertion for the new process. > > If so, we may need to SuspendedPageProxy to hold a background token for its > WebProcess until SuspendedPageProxy::didFinishLoad() is called. Dealing with those API test failures via Bug 192348.
Note You need to log in before you can comment on or make changes to this bug.