<rdar://problem/8927952>
Created attachment 92453 [details] Patch
Comment on attachment 92453 [details] Patch Committed in http://trac.webkit.org/changeset/86016
This change caused crashes on the WK2 Windows bots, so I had sheriffbot roll it out in http://trac.webkit.org/changeset/86018
Created attachment 93207 [details] Path supplementing the one already reviewed that should fix the crashes on the WK2 bots
Comment on attachment 93207 [details] Path supplementing the one already reviewed that should fix the crashes on the WK2 bots Recommitted the patch with this fix in http://trac.webkit.org/changeset/86285. And it passes on the tests! http://build.webkit.org/results/Windows%207%20Release%20(WebKit2%20Tests)/r86285%20(6745)/results.html
There are a few outstanding issues that need addressing.
Created attachment 93299 [details] Patch with a better fix for the crash and with a fix for a loose end
Comment on attachment 93299 [details] Patch with a better fix for the crash and with a fix for a loose end View in context: https://bugs.webkit.org/attachment.cgi?id=93299&action=review > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:45 > + , serializedDefaultStorageSessionIsValid(false) I don't think this extra member is needed. You can still encode and decode the boolean just using local variables. There's no need to pass the boolean around in the parameters struct itself.
Created attachment 93318 [details] Patch with a better fix for the crash and with a fix for a loose end (Take 2)
Comment on attachment 93318 [details] Patch with a better fix for the crash and with a fix for a loose end (Take 2) View in context: https://bugs.webkit.org/attachment.cgi?id=93318&action=review > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:166 > - if (parameters.serializedDefaultStorageSession && !CoreIPC::decode(decoder, parameters.serializedDefaultStorageSession)) > + bool hasStorageSession = false; > + if (!decoder->decode(hasStorageSession)) > + return false; > + if (hasStorageSession && !CoreIPC::decode(decoder, parameters.serializedDefaultStorageSession)) > return false; What zeroes out parameters.serializedDefaultStorageSession?
Comment on attachment 93318 [details] Patch with a better fix for the crash and with a fix for a loose end (Take 2) View in context: https://bugs.webkit.org/attachment.cgi?id=93318&action=review > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:89 > + CFDataRef storageSession = serializedDefaultStorageSession.get(); > + encoder->encodeBool(storageSession); > + if (storageSession) > + CoreIPC::encode(encoder, storageSession); You could probably get away without the local storageSession variable. But having it seems fine.
(In reply to comment #10) > (From update of attachment 93318 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=93318&action=review > > > Source/WebKit2/Shared/WebProcessCreationParameters.cpp:166 > > - if (parameters.serializedDefaultStorageSession && !CoreIPC::decode(decoder, parameters.serializedDefaultStorageSession)) > > + bool hasStorageSession = false; > > + if (!decoder->decode(hasStorageSession)) > > + return false; > > + if (hasStorageSession && !CoreIPC::decode(decoder, parameters.serializedDefaultStorageSession)) > > return false; > > What zeroes out parameters.serializedDefaultStorageSession? It is a RetainPtr, which is automatically initialized to 0 when the WebProcessCreationParameters is constructed in the Web Process.