Bug 195362 – REGRESSION: (r242181) API test DragAndDropTests.ExternalSourcePlainTextToIFrame is Timing out

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Truitt Savell]

The following API test is timing out on iOS debug

DragAndDropTests.ExternalSourcePlainTextToIFrame

Probable cause:

This began timing out with https://trac.webkit.org/changeset/242181/webkit. I confirmed this using command:

run-api-tests DragAndDropTests.ExternalSourcePlainTextToIFrame --ios-simulator --debug

on r242181 which produces a timeout. Running this on r242180 produces no timeouts.

Timeout

    TestWebKitAPI.DragAndDropTests.ExternalSourcePlainTextToIFrame
        ASSERTION FAILED: areOriginsMatching(origin1, origin2) == (origin1.toString() == origin2.toString())
        ./page/SecurityOrigin.cpp(499) : bool WebCore::originsMatch(const WebCore::SecurityOrigin &, const WebCore::SecurityOrigin &)
        1   0x222720bf9 WTFCrash
        2   0x2267efeeb WTFCrashWithInfo(int, char const*, char const*, int)
        3   0x229430c12 WebCore::originsMatch(WebCore::SecurityOrigin const&, WebCore::SecurityOrigin const&)
        4   0x22913c68e WebCore::DocumentLoader::shouldOpenExternalURLsPolicyToPropagate() const
        5   0x228887b0d WebCore::Document::shouldOpenExternalURLsPolicyToPropagate() const
        6   0x229315cd8 WebCore::DragController::performDragOperation(WebCore::DragData const&)
        7   0x109363210 WebKit::WebPage::performDragControllerAction(WebKit::DragControllerAction, WebCore::DragData const&, WebKit::SandboxExtension::Handle&&, WebKit::SandboxExtension::HandleArray&&)
        8   0x1093ee8fd void IPC::callMemberFunctionImpl<WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::DragControllerAction, WebCore::DragData const&, WebKit::SandboxExtension::Handle&&, WebKit::SandboxExtension::HandleArray&&), std::__1::tuple<WebKit::DragControllerAction, WebCore::DragData, WebKit::SandboxExtension::Handle, WebKit::SandboxExtension::HandleArray>, 0ul, 1ul, 2ul, 3ul>(WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::DragControllerAction, WebCore::DragData const&, WebKit::SandboxExtension::Handle&&, WebKit::SandboxExtension::HandleArray&&), std::__1::tuple<WebKit::DragControllerAction, WebCore::DragData, WebKit::SandboxExtension::Handle, WebKit::SandboxExtension::HandleArray>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul>)
        9   0x1093ee3b0 void IPC::callMemberFunction<WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::DragControllerAction, WebCore::DragData const&, WebKit::SandboxExtension::Handle&&, WebKit::SandboxExtension::HandleArray&&), std::__1::tuple<WebKit::DragControllerAction, WebCore::DragData, WebKit::SandboxExtension::Handle, WebKit::SandboxExtension::HandleArray>, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul> >(std::__1::tuple<WebKit::DragControllerAction, WebCore::DragData, WebKit::SandboxExtension::Handle, WebKit::SandboxExtension::HandleArray>&&, WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::DragControllerAction, WebCore::DragData const&, WebKit::SandboxExtension::Handle&&, WebKit::SandboxExtension::HandleArray&&))
        10  0x1093cf554 void IPC::handleMessage<Messages::WebPage::PerformDragControllerAction, WebKit::WebPage, void (WebKit::WebPage::*)(WebKit::DragControllerAction, WebCore::DragData const&, WebKit::SandboxExtension::Handle&&, WebKit::SandboxExtension::HandleArray&&)>(IPC::Decoder&, WebKit::WebPage*, void (WebKit::WebPage::*)(WebKit::DragControllerAction, WebCore::DragData const&, WebKit::SandboxExtension::Handle&&, WebKit::SandboxExtension::HandleArray&&))
        11  0x1093be8d8 WebKit::WebPage::didReceiveWebPageMessage(IPC::Connection&, IPC::Decoder&)
        12  0x10936606e WebKit::WebPage::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
        13  0x10834524a IPC::MessageReceiverMap::dispatchMessage(IPC::Connection&, IPC::Decoder&)
        14  0x109004b1d WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&)
        15  0x1082f851c IPC::Connection::dispatchMessage(IPC::Decoder&)
        16  0x1082eadf1 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)
        17  0x1082f92e7 IPC::Connection::dispatchOneIncomingMessage()
        18  0x108319f78 IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()()
        19  0x108319e89 WTF::Function<void ()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14>::call()
        20  0x22274b0bd WTF::Function<void ()>::operator()() const
        21  0x2227aa823 WTF::RunLoop::performWork()
        22  0x2227ab1b4 WTF::RunLoop::performWork(void*)
        23  0x10e267721 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__
        24  0x10e266f93 __CFRunLoopDoSources0
        25  0x10e26163f __CFRunLoopRun
        26  0x10e260e11 CFRunLoopRunSpecific
        27  0x107d34322 -[NSRunLoop(NSRunLoop) runMode:beforeDate:]
        28  0x107d34492 -[NSRunLoop(NSRunLoop) run]
        29  0x10fa58812 _xpc_objc_main
        30  0x10fa5acbd xpc_main
        31  0x108778497 WebKit::XPCServiceMain(int, char const**)

[Alexey Proskuryakov]

What is the next step here? 8 days ago seems like maybe roll back?

[Brady Eidson]

(In reply to Truitt Savell from comment #0)
> 
> This began timing out with https://trac.webkit.org/changeset/242181/webkit

That change caused a *different* assertion, as mentioned in https://bugs.webkit.org/show_bug.cgi?id=195126

Also as mentioned in https://bugs.webkit.org/show_bug.cgi?id=195126, I identified it as a bogus assertion so I removed it in https://trac.webkit.org/changeset/242249/webkit

That was 7-8 days ago.

The assertion being reported in this bug is a different one that Youenn re-added in https://trac.webkit.org/changeset/242283/webkit

[Brady Eidson]

(In reply to Alexey Proskuryakov from comment #1)
> What is the next step here? 8 days ago seems like maybe roll back?

The regression caused 8 days ago was fixed 8 days ago.

This assertion was added 6 days ago in https://trac.webkit.org/changeset/242283/webkit

I would suggest contacting Youenn about that instead of me.

[youenn fablet]

Created attachment 363976 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 363976 [details]
Patch

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

> Source/WebCore/page/SecurityOrigin.cpp:499
> +    ASSERT(!areOriginsMatching(origin1, origin2) || (origin1.toString() == origin2.toString()));

This assertion says that matching origins must have identical toString() results. Why is this invariant important? This should be explained in a comment, as otherwise someone will just remove it when it starts to fail after another change.

For example, if toString() begins to output file URL host, which it kind of is supposed to do, I'm not sure how it follows that we would have to treat the URLs as not matching - in fact, file://localhost/ is usually same as file://. This assertion alone does very little to explain why a change like the former would be wrong.

[youenn fablet]

(In reply to Alexey Proskuryakov from comment #5)
> Comment on attachment 363976 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=363976&action=review
> 
> > Source/WebCore/page/SecurityOrigin.cpp:499
> > +    ASSERT(!areOriginsMatching(origin1, origin2) || (origin1.toString() == origin2.toString()));
> 
> This assertion says that matching origins must have identical toString()
> results. Why is this invariant important? This should be explained in a
> comment, as otherwise someone will just remove it when it starts to fail
> after another change.
> 
> For example, if toString() begins to output file URL host, which it kind of
> is supposed to do, I'm not sure how it follows that we would have to treat
> the URLs as not matching - in fact, file://localhost/ is usually same as
> file://. This assertion alone does very little to explain why a change like
> the former would be wrong.

Agreed, https://bugs.webkit.org/show_bug.cgi?id=195572 is planning to rename originsMatch to originSerializationsMatch which hopefully makes it clearer.
Above bug should also remove some current uses of originsMatch as I believe, SecurityOrigin::isSameOriginAs might be better in most cases.

[WebKit Commit Bot]

Comment on attachment 363976 [details]
Patch

Clearing flags on attachment: 363976

Committed r242761: <https://trac.webkit.org/changeset/242761>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48792151>