RESOLVED FIXED207868
ASSERT_NOT_REACHED() under WebPageProxy::suspendCurrentPageIfPossible() 
https://bugs.webkit.org/show_bug.cgi?id=207868
Summary ASSERT_NOT_REACHED() under WebPageProxy::suspendCurrentPageIfPossible()
Chris Dumez
Reported 2020-02-17 16:26:19 PST
ASSERT_NOT_REACHED() under WebPageProxy::suspendCurrentPageIfPossible() when doing a client-side redirect cross-site in an ephemeral session: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x000000010eb251fe WTFCrash + 14 (Assertions.cpp:305) 1 com.apple.WebKit 0x0000000119e9869b WTFCrashWithInfo(int, char const*, char const*, int) + 27 2 com.apple.WebKit 0x000000011aba3aa5 WebKit::WebPageProxy::suspendCurrentPageIfPossible(API::Navigation&, WTF::Optional<WTF::ObjectIdentifier<WebCore::FrameIdentifierType> >, WebKit::ProcessSwapRequestedByClient, WebKit::ShouldDelayClosingUntilEnteringAcceleratedCompositingMode) + 1909 (WebPageProxy.cpp:814) 3 com.apple.WebKit 0x000000011abb827a WebKit::WebPageProxy::commitProvisionalPage(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData const&) + 1338 (WebPageProxy.cpp:3200) 4 com.apple.WebKit 0x000000011aa2fc79 WebKit::ProvisionalPageProxy::didCommitLoadForFrame(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData const&) + 953 5 com.apple.WebKit 0x000000011aa72eaa void IPC::callMemberFunctionImpl<WebKit::ProvisionalPageProxy, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData const&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String, bool, unsigned int, WebCore::CertificateInfo, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData>, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul, 9ul>(WebKit::ProvisionalPageProxy*, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData const&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String, bool, unsigned int, WebCore::CertificateInfo, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul, 9ul>) + 538 (HandleMessage.h:42) 6 com.apple.WebKit 0x000000011aa6a480 void IPC::callMemberFunction<WebKit::ProvisionalPageProxy, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData const&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String, bool, unsigned int, WebCore::CertificateInfo, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData>, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul, 9ul> >(std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String, bool, unsigned int, WebCore::CertificateInfo, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData>&&, WebKit::ProvisionalPageProxy*, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData const&)) + 112 (HandleMessage.h:48) 7 com.apple.WebKit 0x000000011aa32b9c void IPC::handleMessage<Messages::WebPageProxy::DidCommitLoadForFrame, WebKit::ProvisionalPageProxy, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData const&)>(IPC::Decoder&, WebKit::ProvisionalPageProxy*, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, unsigned long long, WTF::String const&, bool, unsigned int, WebCore::CertificateInfo const&, bool, bool, WTF::Optional<WebCore::HasInsecureContent>, WebKit::UserData const&)) + 268 (HandleMessage.h:121) 8 com.apple.WebKit 0x000000011aa317ee WebKit::ProvisionalPageProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&) + 2654 (ProvisionalPageProxy.cpp:477) 9 com.apple.WebKit 0x0000000119fa372b IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&) + 315 (MessageReceiverMap.cpp:124) 10 com.apple.WebKit 0x000000011a9d99ee WebKit::AuxiliaryProcessProxy::dispatchMessage(IPC::Connection&, IPC::Decoder&) + 46 (AuxiliaryProcessProxy.cpp:196)
Attachments
Patch (5.04 KB, patch)
2020-02-17 16:34 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Patch (8.48 KB, patch)
2020-02-18 08:07 PST, Chris Dumez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Chris Dumez
Comment 1 2020-02-17 16:26:31 PST
<rdar://problem/59464606>
Chris Dumez
Comment 2 2020-02-17 16:34:29 PST
Created attachment 391001 [details] Patch
John Wilander
Comment 3 2020-02-17 16:46:45 PST
Comment on attachment 391001 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391001&action=review See the one comment. > Source/WebCore/loader/HistoryController.cpp:554 > + bool needsPrivacy = page->usesEphemeralSession(); Privacy is such a broad term. I'd prefer to stick with ephemeral as the term because that's the only privacy promise it makes. Would isEphemeral work? Or just usesEphemeralSession?
Chris Dumez
Comment 4 2020-02-17 16:48:53 PST
Comment on attachment 391001 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391001&action=review >> Source/WebCore/loader/HistoryController.cpp:554 >> + bool needsPrivacy = page->usesEphemeralSession(); > > Privacy is such a broad term. I'd prefer to stick with ephemeral as the term because that's the only privacy promise it makes. Would isEphemeral work? Or just usesEphemeralSession? I chose this to be consistent with the rest of this class (see other uses of usesEphemeralSession in this class). What do you want me to do, rename all of them?
John Wilander
Comment 5 2020-02-17 17:08:16 PST
(In reply to Chris Dumez from comment #4) > Comment on attachment 391001 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=391001&action=review > > >> Source/WebCore/loader/HistoryController.cpp:554 > >> + bool needsPrivacy = page->usesEphemeralSession(); > > > > Privacy is such a broad term. I'd prefer to stick with ephemeral as the term because that's the only privacy promise it makes. Would isEphemeral work? Or just usesEphemeralSession? > > I chose this to be consistent with the rest of this class (see other uses of > usesEphemeralSession in this class). What do you want me to do, rename all > of them? That's unfortunate. Then either way is fine with me, i.e. fix this one instance now or land it consistent with the rest.
Chris Dumez
Comment 6 2020-02-18 08:07:13 PST
Created attachment 391046 [details] Patch
WebKit Commit Bot
Comment 7 2020-02-18 09:00:15 PST
Comment on attachment 391046 [details] Patch Clearing flags on attachment: 391046 Committed r256831: <https://trac.webkit.org/changeset/256831>
WebKit Commit Bot
Comment 8 2020-02-18 09:00:17 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.