Make sure we never end up with NetworkDataTaskCocoa TaskIdentifier conflicts.
Created attachment 382930 [details] Patch
Comment on attachment 382930 [details] Patch Clearing flags on attachment: 382930 Committed r252141: <https://trac.webkit.org/changeset/252141>
All reviewed patches have been landed. Closing bug.
<rdar://problem/56953662>
Comment on attachment 382930 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382930&action=review > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1247 > + RELEASE_ASSERT(!map.contains(taskIdentifier)); This will be hit in practice. I think we should just roll this out and fix it the right way in https://bugs.webkit.org/show_bug.cgi?id=203934
(In reply to Alex Christensen from comment #5) > Comment on attachment 382930 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=382930&action=review > > > Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1247 > > + RELEASE_ASSERT(!map.contains(taskIdentifier)); > > This will be hit in practice. I think we should just roll this out and fix > it the right way in https://bugs.webkit.org/show_bug.cgi?id=203934 I did not add any new assertion, so I am not sure I understand why. Please do NOT revert.
Comment on attachment 382930 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382930&action=review >>> Source/WebKit/NetworkProcess/cocoa/NetworkSessionCocoa.mm:1247 >>> + RELEASE_ASSERT(!map.contains(taskIdentifier)); >> >> This will be hit in practice. I think we should just roll this out and fix it the right way in https://bugs.webkit.org/show_bug.cgi?id=203934 > > I did not add any new assertion, so I am not sure I understand why. > > Please do NOT revert. I don't think we generally revert patches without proof that something is wrong. You claim that this assertion will be hit in practice but have not explained at all why it would be. Also, note that I merely turned this debug assertion into a release one. If it really gets hit, I'd argue a crash is better than a security bug. My patch is simply refactoring the code to have it in a single place and strengthening the assertions. Therefore, I am not sure what you mean by fixing the right way. https://bugs.webkit.org/show_bug.cgi?id=203934 re-enables isolated sessions, it does not simply fix things.