Bug 229769 – [ BigSur arm64 Debug EWS ] ASSERTION FAILED: m_uncommittedState.state == State::Provisional

• Component: New Bugs
• Status: RESOLVED FIXED

imported/w3c/web-platform-tests/html/semantics/forms/the-button-element/button-submit-children.html 

Is a flaky crash on macOS-AppleSilicon-Big-Sur-Debug-WK2-Tests-EWS.

It appears that it has crashed one time in the open source directory: https://results.webkit.org/?suite=layout-tests&test=imported/w3c/web-platform-tests/html/semantics/forms/the-button-element/button-submit-children.html

On EWS, it started being flaky at https://ews-build.webkit.org/#/builders/60/builds/11011

ASSERTION FAILED: m_uncommittedState.state == State::Provisional
/Volumes/Data/worker/macOS-AppleSilicon-Big-Sur-Debug-Build-EWS/build/Source/WebKit/UIProcess/PageLoadState.cpp(325) : void WebKit::PageLoadState::didCommitLoad(const Transaction::Token &, WebKit::WebCertificateInfo &, bool, bool)
1   0x100b2ebec WTFCrash
2   0x109750f88 std::__1::optional<JSC::JSValue>::optional<JSC::JSValue, 0>(JSC::JSValue&&)
3   0x10abf9324 WebKit::PageLoadState::didCommitLoad(WebKit::PageLoadState::Transaction::Token const&, WebKit::WebCertificateInfo&, bool, bool)
4   0x10ad5eefc WebKit::WebPageProxy::didCommitLoadForFrame(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, unsigned long long, WTF::String const&, bool, WebCore::FrameLoadType, WebCore::CertificateInfo const&, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData const&)
5   0x10ad5e5a0 WebKit::WebPageProxy::commitProvisionalPage(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, unsigned long long, WTF::String const&, bool, WebCore::FrameLoadType, WebCore::CertificateInfo const&, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData const&)
6   0x10ac00a18 WebKit::ProvisionalPageProxy::didCommitLoadForFrame(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, unsigned long long, WTF::String const&, bool, WebCore::FrameLoadType, WebCore::CertificateInfo const&, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData const&)
7   0x10ac60c4c void IPC::callMemberFunctionImpl<WebKit::ProvisionalPageProxy, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, unsigned long long, WTF::String const&, bool, WebCore::FrameLoadType, WebCore::CertificateInfo const&, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData const&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData, WebCore::ResourceRequest, unsigned long long, WTF::String, bool, WebCore::FrameLoadType, WebCore::CertificateInfo, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData>, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul, 9ul, 10ul, 11ul, 12ul>(WebKit::ProvisionalPageProxy*, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, unsigned long long, WTF::String const&, bool, WebCore::FrameLoadType, WebCore::CertificateInfo const&, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData const&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData, WebCore::ResourceRequest, unsigned long long, WTF::String, bool, WebCore::FrameLoadType, WebCore::CertificateInfo, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul, 9ul, 10ul, 11ul, 12ul>)
8   0x10ac5303c void IPC::callMemberFunction<WebKit::ProvisionalPageProxy, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, unsigned long long, WTF::String const&, bool, WebCore::FrameLoadType, WebCore::CertificateInfo const&, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData const&), std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData, WebCore::ResourceRequest, unsigned long long, WTF::String, bool, WebCore::FrameLoadType, WebCore::CertificateInfo, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData>, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul, 7ul, 8ul, 9ul, 10ul, 11ul, 12ul> >(std::__1::tuple<WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData, WebCore::ResourceRequest, unsigned long long, WTF::String, bool, WebCore::FrameLoadType, WebCore::CertificateInfo, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData>&&, WebKit::ProvisionalPageProxy*, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, unsigned long long, WTF::String const&, bool, WebCore::FrameLoadType, WebCore::CertificateInfo const&, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData const&))
9   0x10ac02e40 void IPC::handleMessage<Messages::WebPageProxy::DidCommitLoadForFrame, WebKit::ProvisionalPageProxy, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, unsigned long long, WTF::String const&, bool, WebCore::FrameLoadType, WebCore::CertificateInfo const&, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData const&)>(IPC::Decoder&, WebKit::ProvisionalPageProxy*, void (WebKit::ProvisionalPageProxy::*)(WTF::ObjectIdentifier<WebCore::FrameIdentifierType>, WebKit::FrameInfoData&&, WebCore::ResourceRequest&&, unsigned long long, WTF::String const&, bool, WebCore::FrameLoadType, WebCore::CertificateInfo const&, bool, bool, std::__1::optional<WebCore::HasInsecureContent>, WebCore::MouseEventPolicy, WebKit::UserData const&))
10  0x10ac021e4 WebKit::ProvisionalPageProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
11  0x109adfeac IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&)
12  0x10ab9dc8c WebKit::AuxiliaryProcessProxy::dispatchMessage(IPC::Connection&, IPC::Decoder&)
13  0x10ae94a3c WebKit::WebProcessProxy::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
14  0x109587ff8 IPC::Connection::dispatchMessage(IPC::Decoder&)
15  0x109588740 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)
16  0x109586d40 IPC::Connection::dispatchIncomingMessages()
17  0x1095ab52c IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_12::operator()()
18  0x1095ab434 WTF::Detail::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_12, void>::call()
19  0x100b58894 WTF::Function<void ()>::operator()() const
20  0x100bdf91c WTF::RunLoop::performWork()
21  0x100be46e8 WTF::RunLoop::performWork(void*)
22  0x18c0caad4 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
23  0x18c0caa20 __CFRunLoopDoSource0
24  0x18c0ca70c __CFRunLoopDoSources0
25  0x18c0c9094 __CFRunLoopRun
26  0x18c0c85e8 CFRunLoopRunSpecific
27  0x18ce71688 -[NSRunLoop(NSRunLoop) runMode:beforeDate:]
28  0x1004674ac WTR::TestController::platformRunUntil(bool&, WTF::Seconds)
29  0x10041da68 WTR::TestController::runUntil(bool&, WTF::Seconds)
30  0x10049336c WTR::TestInvocation::invoke()
31  0x100425008 WTR::TestController::runTest(char const*)
LEAK: 2 WebFrame
LEAK: 8 RenderObject
LEAK: 1 Page
LEAK: 2 Frame
LEAK: 4 CachedResource
LEAK: 26 WebCoreNode
LEAK: 1 WebPage
LEAK: 2 WebFrame
LEAK: 8 RenderObject
LEAK: 1 Page
LEAK: 2 Frame
LEAK: 4 CachedResource
LEAK: 26 WebCoreNode

First crash in the open source directory is at r281832

[Radar WebKit Bug Importer]

<rdar://problem/82645706>

Marked test expectations: https://trac.webkit.org/changeset/281876/webkit

[Chris Dumez]

Doesn't seem to reproduce easily for me.

[Chris Dumez]

I really wish I was able to reproduce this. Based on the crash trace, we can tell the UIProcess is crashing when committing the load in the new process, after a process-swap. It is impossible to tell if the process-swap was due to PSON or COOP but since this is a recent regression, COOP may be more likely.

The reason we hit the assertion is that the WebPageProxy doesn't think a provisional load is going on when the load gets committed. This could be because:
1. The UIProcess did not receive the DidStartProvisionalLoad IPC before the DidCommitLoad IPC
2. The UIProcess got a DidFailProvisionalLoad IPC before the DidCommitLoad IPC
3. PageLoadState::reset() was called between the DidStartProvisionalLoad IPC and the DidCommitLoad IPC

The expected behavior when doing a COOP process-swap is:
1. Process A sends a DidStartProvisionalLoad IPC to the UIProcess
2. Process A send DecidePolicyForResponse IPC to the UIProcess and we decide to process-swap due to COOP header
3. The UIProcess responds with PolicyAction::Ignore for the DecidePolicyForResponse IPC to Process A. This causes process A to send back a DidFailProvisionalLoad IPC but it gets ignored in WebPageProxy::didFailProvisionalLoadForFrame() (early return).
4. The UIProcess launches a new Process B and creates a ProvisionalPageProxy and starts the load in this new process / page.
5. Process B doesn't send a DidStartProvisionalLoad IPC to the UIProcess because we have a check in WebPageLoadClient::didStartProvisionalLoadForFrame() and we are continuing a load after the response policy.
6. Process B sends a DidCommitLoad IPC to the UIProcess.

At step 6, the UIProcess should be in provisional load state since it got a DidStartProvisionalLoad from process A and nothing should have changed the state in between. This works in general and we have API tests covering this. However, something seems to go wrong in the layout tests in some cases.

[Chris Dumez]

The check to discard the DidFailProvisionalLoad IPC from the previous process after a swap looks like so:
```
void WebPageProxy::didFailProvisionalLoadForFrame(FrameIdentifier frameID, FrameInfoData&& frameInfo, WebCore::ResourceRequest&& request, uint64_t navigationID, const String& provisionalURL, const ResourceError& error, WillContinueLoading willContinueLoading, const UserData& userData)
{
    if (m_provisionalPage && m_provisionalPage->navigationID() == navigationID) {
        // The load did not fail, it is merely happening in a new provisional process.
        return;
    }
```

It is a bit fragile. I worry, process A could be sending us a didFailProvisionalLoadForFrame with a navigationID that is 0 or not the navigationID of the provisional frame for some reason.

[Chris Dumez]

Created attachment 437090 [details]
Patch

[Alex Christensen]

Comment on attachment 437090 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437090&action=review

> Source/WebKit/UIProcess/WebPageProxy.cpp:4788
> +    if (m_provisionalPage && frame->isMainFrame()) {
>          // The load did not fail, it is merely happening in a new provisional process.

Can a provisional navigation fail in the new process before m_provisionalPage is cleared?

[Chris Dumez]

(In reply to Alex Christensen from comment #8)
> Comment on attachment 437090 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=437090&action=review
> 
> > Source/WebKit/UIProcess/WebPageProxy.cpp:4788
> > +    if (m_provisionalPage && frame->isMainFrame()) {
> >          // The load did not fail, it is merely happening in a new provisional process.
> 
> Can a provisional navigation fail in the new process before
> m_provisionalPage is cleared?

A few things:
1. didFailProvisionalLoadForFrame() only gets called due to IPC from the currently committed process. It is didFailProvisionalLoadForFrameShared() that may get called due to IPC from either the committed or the provisional process.
2. didFailProvisionalLoadForFrameShared() has logic to clear m_provisionalPage if the provisional load fails in the provisional process.

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 437090 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[Chris Dumez]

Committed r281964 (241271@main): <https://commits.webkit.org/241271@main>

[Chris Dumez]

*** Bug 229880 has been marked as a duplicate of this bug. ***