Summary: | [Curl] Fix DRT crash related to private browsing. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Basuke Suzuki <Basuke.Suzuki> | ||||||
Component: | Platform | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, commit-queue, don.olmstead, Hironori.Fujii, mcatanzaro, ross.kirsling, takashi.komori, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=194141 | ||||||||
Attachments: |
|
Description
Basuke Suzuki
2019-01-24 23:56:17 PST
Created attachment 360574 [details]
Patch
Curl port crashes when DRT tests private browsing mode. This is because lack of inmplementation of WebFrameNetworkingContext::ensurePrivateBrowsingSession() for Curl port. Attached patch fixes it. Seems good from my perspective, but I'd want Alex or Michael to give the r+ here. Comment on attachment 360574 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=360574&action=review > Source/WebKitLegacy/WebCoreSupport/NetworkStorageSessionMap.cpp:115 > + addResult.iterator->value = std::make_unique<WebCore::NetworkStorageSession>(sessionID, nullptr); Unrelated: I think you should remove the second parameter from your NetworkStorageSession constructor. It's a curl-specific constructor, and I believe you have to always pass nullptr for it to not crash. :) Comment on attachment 360574 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=360574&action=review Looks good to me. > Source/WebKitLegacy/WebCoreSupport/NetworkStorageSessionMap.cpp:111 > + auto addResult = globalSessionMap().add(sessionID, nullptr); You could use HashMap::ensure here. > Source/WebKitLegacy/win/WebCoreSupport/WebFrameNetworkingContext.cpp:91 > + if (auto privateSession = NetworkStorageSessionMap::storageSession(PAL::SessionID::legacyPrivateSessionID())) > + return *privateSession; This seems unnecessary. Created attachment 360700 [details]
Patch
(In reply to Michael Catanzaro from comment #4) > Unrelated: I think you should remove the second parameter from your > NetworkStorageSession constructor. It's a curl-specific constructor, and I > believe you have to always pass nullptr for it to not crash. :) I agree. I will open another ticket for removing the parameter. (In reply to Alex Christensen from comment #5) > Comment on attachment 360574 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=360574&action=review > > Looks good to me. > > > Source/WebKitLegacy/WebCoreSupport/NetworkStorageSessionMap.cpp:111 > > + auto addResult = globalSessionMap().add(sessionID, nullptr); > > You could use HashMap::ensure here. > > > Source/WebKitLegacy/win/WebCoreSupport/WebFrameNetworkingContext.cpp:91 > > + if (auto privateSession = NetworkStorageSessionMap::storageSession(PAL::SessionID::legacyPrivateSessionID())) > > + return *privateSession; > > This seems unnecessary. Fixed. Comment on attachment 360700 [details] Patch Clearing flags on attachment: 360700 Committed r240791: <https://trac.webkit.org/changeset/240791> All reviewed patches have been landed. Closing bug. |