Bug 192318

Created attachment 356395 [details] Patch

<rdar://problem/46377552>

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.

Created attachment 356397 [details] Patch

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

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

(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.

(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.