Bug 207868

Summary: ASSERT_NOT_REACHED() under WebPageProxy::suspendCurrentPageIfPossible()
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, commit-queue, dbates, ews-watchlist, ggaren, japhet, simon.fraser, webkit-bug-importer, wilander, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Description Chris Dumez 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)
Comment 1 Chris Dumez 2020-02-17 16:26:31 PST 
<rdar://problem/59464606>
Comment 2 Chris Dumez 2020-02-17 16:34:29 PST 
Created attachment 391001 [details]
Patch
Comment 3 John Wilander 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?
Comment 4 Chris Dumez 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?
Comment 5 John Wilander 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.
Comment 6 Chris Dumez 2020-02-18 08:07:13 PST 
Created attachment 391046 [details]
Patch
Comment 7 WebKit Commit Bot 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>
Comment 8 WebKit Commit Bot 2020-02-18 09:00:17 PST 
All reviewed patches have been landed.  Closing bug.