Bug 110937 – Threaded HTML parser hits ASSERTION FAILED: this == frameLoader()->activeDocumentLoader()

• Component: WebCore Misc.
• Status: RESOLVED INVALID

[Adam Barth]

This happens on a bunch of tests.  For example, fast/text/reset-drag-on-mouse-down.html

crash log for DumpRenderTree (pid 26853):
STDOUT: <empty>
STDERR: ASSERTION FAILED: this == frameLoader()->activeDocumentLoader()
STDERR: ../../third_party/WebKit/Source/WebCore/loader/DocumentLoader.cpp(434) : void WebCore::DocumentLoader::checkLoadComplete()
STDERR: 1   0x7fd34ab53045
STDERR: 2   0x7fd349dc837b
STDERR: 3   0x7fd34aec3a9c
STDERR: 4   0x7fd34aeb1150
STDERR: 5   0x7fd34aeb0910
STDERR: 6   0x7fd34aea4048
STDERR: 7   0x7fd34aea3f81
STDERR: 8   0x7fd349a17708
STDERR: 9   0x7fd349a174b8
STDERR: 10  0x7fd34eae6f62
STDERR: 11  0x7fd34eae6e8b
STDERR: 12  0x7fd34eae6cc5
STDERR: 13  0x7fd34e720521
STDERR: 14  0x7fd34e768e47 MessageLoop::RunTask(base::PendingTask const&)
STDERR: 15  0x7fd34e768f5e MessageLoop::DeferOrRunPendingTask(base::PendingTask const&)
STDERR: 16  0x7fd34e7697ef MessageLoop::DoWork()
STDERR: 17  0x7fd34e704c9e base::MessagePumpGlib::RunWithDispatcher(base::MessagePump::Delegate*, base::MessagePumpDispatcher*)
STDERR: 18  0x7fd34e705120 base::MessagePumpGlib::Run(base::MessagePump::Delegate*)
STDERR: 19  0x7fd34e768a3b MessageLoop::RunInternal()
STDERR: 20  0x7fd34e7688f6 MessageLoop::RunHandler()
STDERR: 21  0x7fd34e7a0fe6 base::RunLoop::Run()
STDERR: 22  0x7fd34e768228 MessageLoop::Run()
STDERR: 23  0x4ace8d
STDERR: 24  0x472c26
STDERR: 25  0x46cd20
STDERR: 26  0x46593c
STDERR: 27  0x4664ec
STDERR: 28  0x7fd343232c4d __libc_start_main
STDERR: 29  0x463c99
STDERR: Received signal 11 SEGV_MAPERR 0000bbadbeef
STDERR:  [0x7fd34e72add0] base::debug::StackTrace::StackTrace()
STDERR:  [0x7fd34e72a67a] base::debug::(anonymous namespace)::StackDumpSignalHandler()
STDERR:  [0x7fd3435a68f0] <unknown>
STDERR:  [0x7fd34ab5304f] WebCore::DocumentLoader::checkLoadComplete()
STDERR:  [0x7fd349dc837b] WebCore::Document::decrementActiveParserCount()
STDERR:  [0x7fd34aec3a9c] WebCore::ActiveParserSession::~ActiveParserSession()
STDERR:  [0x7fd34aeb1150] WebCore::HTMLDocumentParser::processParsedChunkFromBackgroundParser()
STDERR:  [0x7fd34aeb0910] WebCore::HTMLDocumentParser::didReceiveParsedChunkFromBackgroundParser()
STDERR:  [0x7fd34aea4048] WTF::FunctionWrapper<>::operator()()
STDERR:  [0x7fd34aea3f81] WTF::BoundFunctionImpl<>::operator()()
STDERR:  [0x7fd349a17708] WTF::Function<>::operator()()
STDERR:  [0x7fd349a174b8] WTF::callFunctionObject()
STDERR:  [0x7fd34eae6f62] base::internal::RunnableAdapter<>::Run()
STDERR:  [0x7fd34eae6e8b] base::internal::InvokeHelper<>::MakeItSo()
STDERR:  [0x7fd34eae6cc5] base::internal::Invoker<>::Run()
STDERR:  [0x7fd34e720521] base::Callback<>::Run()
STDERR:  [0x7fd34e768e47] MessageLoop::RunTask()
STDERR:  [0x7fd34e768f5e] MessageLoop::DeferOrRunPendingTask()
STDERR:  [0x7fd34e7697ef] MessageLoop::DoWork()
STDERR:  [0x7fd34e704c9e] base::MessagePumpGlib::RunWithDispatcher()
STDERR:  [0x7fd34e705120] base::MessagePumpGlib::Run()
STDERR:  [0x7fd34e768a3b] MessageLoop::RunInternal()
STDERR:  [0x7fd34e7688f6] MessageLoop::RunHandler()
STDERR:  [0x7fd34e7a0fe6] base::RunLoop::Run()
STDERR:  [0x7fd34e768228] MessageLoop::Run()
STDERR:  [0x0000004ace8d] webkit_support::RunMessageLoop()
STDERR:  [0x000000472c26] TestShell::waitTestFinished()
STDERR:  [0x00000046cd20] TestShell::runFileTest()
STDERR:  [0x00000046593c] runTest()
STDERR:  [0x0000004664ec] main
STDERR:  [0x7fd343232c4d] __libc_start_main
STDERR:  [0x000000463c99] <unknown>
STDERR:   r8: 00007fd33d1e47e0  r9: 00007fd34b6689fe r10: 00000000004002d0 r11: 0000000000000000
STDERR:  r12: 0000000000000050 r13: 00007fffdebeb5a0 r14: 0000000000000000 r15: 0000000000000000
STDERR:   di: 0000000000000000  si: 00000000efcdab90  bp: 00007fffdebe9c60  bx: 00000f40972cd458
STDERR:   dx: 00007fd343592e00  ax: 00000000bbadbeef  cx: 00007fd3432ecacd  sp: 00007fffdebe9c50
STDERR:   ip: 00007fd34ab5304f efl: 0000000000010246 cgf: 0000000000000033 erf: 0000000000000006
STDERR:  trp: 000000000000000e msk: 0000000000000000 cr2: 00000000bbadbeef

[Adam Barth]

I silenced the ASSERT in http://trac.webkit.org/changeset/144139, but we should dig into this issue a bit more to understand whether the ASSERT should be re-enabled.

[Eric Seidel (no email)]

Yeah, this sounds very useful.  We can look tomorrow.

[Darin Adler]

Might be better to put the assert back in, inside an #if, so we have a chance to catch regressions unrelated to the threaded HTML parser work while we continue to investigate the new issue. On platforms where the threaded HTML parser is turned off at compile time we could still get some value from the assertion.

Maybe an unimportant point if this is resolved quickly enough.

[Adam Barth]

Created attachment 190553 [details]
Patch

[Adam Barth]

> Might be better to put the assert back in, inside an #if, so we have a chance to catch regressions unrelated to the threaded HTML parser work while we continue to investigate the new issue.

Good idea.

[Darin Adler]

Comment on attachment 190553 [details]
Patch

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

> Source/WebCore/loader/DocumentLoader.cpp:434
> +#if ENABLE(THREADED_HTML_PARSER)

#if !ENABLE

[Adam Barth]

Created attachment 190577 [details]
Patch for landing

[Eric Seidel (no email)]

Comment on attachment 190577 [details]
Patch for landing

Didn't we turn off the threaded parser by default again?  If so, can't we just add back the ASSERT and be sure to fix it before turning it back on?  In any case, this is fine too.

[Eric Seidel (no email)]

Comment on attachment 190577 [details]
Patch for landing

Oh.  This is likely that we're calling checkLoadComplete() too often.  I guess we just need to check in decrementActiveParserCount() before calling.

[WebKit Review Bot]

Comment on attachment 190577 [details]
Patch for landing

Clearing flags on attachment: 190577

Committed r144232: <http://trac.webkit.org/changeset/144232>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Adam Barth]

Re-opening to actually fix the bug.

[Eric Seidel (no email)]

This appears to have been fixed by bug 110951.

[Eric Seidel (no email)]

Created attachment 190771 [details]
Patch

[Eric Seidel (no email)]

I ran all the layout tests with --chromium --debug and saw no ASSERTs.

[Adam Barth]

Comment on attachment 190771 [details]
Patch

Great!

[Adam Barth]

(In reply to comment #15)
> I ran all the layout tests with --chromium --debug and saw no ASSERTs.

Did you check the mentioned test on apple-mac WebKit1?

[Eric Seidel (no email)]

Comment on attachment 190771 [details]
Patch

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

> Source/WebCore/loader/DocumentLoader.cpp:290
>      if (document() && document()->hasActiveParser())

The magic is that document() returns null in the case we were trying to avoid:
    if (m_frame && m_frame->loader()->documentLoader() == this)
        return m_frame->document();

[Eric Seidel (no email)]

Comment on attachment 190771 [details]
Patch

Sorry, missed your question.

[Eric Seidel (no email)]

run-webkit-tests --debug http/tests/security/feed-urls-from-remote.html  still passes with the #ifdef removed on Mac WK1.

[Eric Seidel (no email)]

Created attachment 190802 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 190802 [details]
Patch for landing

Clearing flags on attachment: 190802

Committed r144370: <http://trac.webkit.org/changeset/144370>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by bug 111136