1427824 - Assertion failure: aNextSibling->GetPrevSibling() || aParentFrame->PrincipalChildList().FirstChild() == aNextSibling (next sibling must be on the principal child list here), at /builds/worker/workspace/build/src/layout/base/nsCSSFrameConstructor.cpp:6507

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Jason Kratzer [:jkratzer]]

Attachment: trigger.html

Testcase found while fuzzing mozilla-central rev ac93fdadf102.

rax = 0x0000000000000000   rdx = 0x0000000000000000
rcx = 0x00007f7f384c02ad   rbx = 0x00007f7f14d02a58
rsi = 0x00007f7f3878f770   rdi = 0x00007f7f3878e540
rbp = 0x00007ffd7b22f9a0   rsp = 0x00007ffd7b22f990
r8 = 0x00007f7f3878f770    r9 = 0x00007f7f39a75740
r10 = 0x0000000000000039   r11 = 0x0000000000000000
r12 = 0x00007f7f14d02b88   r13 = 0x00007f7f14d02910
r14 = 0x00007f7f161b5b80   r15 = 0x00007f7f14d02b88
rip = 0x00007f7f290576e1
OS|Linux|0.0.0 Linux 4.4.0-104-generic #127-Ubuntu SMP Mon Dec 11 12:16:42 UTC 2017 x86_64
CPU|amd64|family 6 model 78 stepping 3|1
GPU|||
Crash|SIGSEGV|0x0|0
0|0|libxul.so|FindAppendPrevSibling|hg:hg.mozilla.org/mozilla-central:layout/base/nsCSSFrameConstructor.cpp:ac93fdadf102|6505|0x5
0|1|libxul.so|nsCSSFrameConstructor::ContentAppended|hg:hg.mozilla.org/mozilla-central:layout/base/nsCSSFrameConstructor.cpp:ac93fdadf102|7679|0xf
0|2|libxul.so|mozilla::RestyleManager::ProcessRestyledFrames|hg:hg.mozilla.org/mozilla-central:layout/base/RestyleManager.cpp:ac93fdadf102|1408|0x11
0|3|libxul.so|mozilla::ServoRestyleManager::DoProcessPendingRestyles|hg:hg.mozilla.org/mozilla-central:layout/base/ServoRestyleManager.cpp:ac93fdadf102|1161|0xb
0|4|libxul.so|mozilla::PresShell::DoFlushPendingNotifications|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:ac93fdadf102|4225|0x18
0|5|libxul.so|nsRefreshDriver::Tick|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:ac93fdadf102|1891|0x5
0|6|libxul.so|mozilla::RefreshDriverTimer::TickRefreshDrivers|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:ac93fdadf102|306|0xf
0|7|libxul.so|mozilla::RefreshDriverTimer::Tick|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:ac93fdadf102|328|0x12
0|8|libxul.so|mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:ac93fdadf102|769|0x5
0|9|libxul.so|mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:ac93fdadf102|583|0xc
0|10|libxul.so|mozilla::layout::VsyncChild::RecvNotify|hg:hg.mozilla.org/mozilla-central:layout/ipc/VsyncChild.cpp:ac93fdadf102|68|0x9
0|11|libxul.so|mozilla::layout::PVsyncChild::OnMessageReceived|s3:gecko-generated-sources:06086093ccb59dd5a99cf8c9f9fb7f4860fd8ddbfd516af5e5b3508be62029679421dcf2abdf6b1c945b6a054050bd403c9574aad49f857cb4a31d3f4cf56b9a/ipc/ipdl/PVsyncChild.cpp:|155|0xf
0|12|libxul.so|mozilla::ipc::MessageChannel::DispatchAsyncMessage|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:ac93fdadf102|2110|0x6
0|13|libxul.so|mozilla::ipc::MessageChannel::DispatchMessage|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:ac93fdadf102|2040|0xb
0|14|libxul.so|mozilla::ipc::MessageChannel::RunMessage|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:ac93fdadf102|1886|0xb
0|15|libxul.so|mozilla::ipc::MessageChannel::MessageTask::Run|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessageChannel.cpp:ac93fdadf102|1919|0xc
0|16|libxul.so|nsThread::ProcessNextEvent|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:ac93fdadf102|1039|0x15
0|17|libxul.so|NS_ProcessNextEvent|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:ac93fdadf102|510|0x11
0|18|libxul.so|mozilla::ipc::MessagePump::Run|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:ac93fdadf102|97|0xa
0|19|libxul.so|MessageLoop::RunInternal|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:ac93fdadf102|326|0x17
0|20|libxul.so|MessageLoop::Run|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:ac93fdadf102|319|0x8
0|21|libxul.so|nsBaseAppShell::Run|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:ac93fdadf102|157|0xd
0|22|libxul.so|XRE_RunAppShell|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:ac93fdadf102|875|0x11
0|23|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:ac93fdadf102|269|0x5
0|24|libxul.so|MessageLoop::RunInternal|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:ac93fdadf102|326|0x17
0|25|libxul.so|MessageLoop::Run|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:ac93fdadf102|319|0x8
0|26|libxul.so|XRE_InitChildProcess|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:ac93fdadf102|701|0x8
0|27|firefox|content_process_main|hg:hg.mozilla.org/mozilla-central:ipc/contentproc/plugin-container.cpp:ac93fdadf102|63|0x14
0|28|firefox|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:ac93fdadf102|280|0x11
0|29|libc-2.23.so||||0x20830
0|30|firefox|MOZ_ReportAssertionFailure|hg:hg.mozilla.org/mozilla-central:mfbt/Assertions.h:ac93fdadf102|165|0x5

Comment 1

[Emilio Cobos Álvarez (:emilio)]

So this is because the col groups are not in the principal child list, and that's because of:

  https://searchfox.org/mozilla-central/rev/b24e6342d744c5a83fab5c15972e11eeb69d68e6/layout/tables/nsTableFrame.cpp#339

I don't have much background on that code, but I suspect the assertion is just not valid, so it either needs to account for this, which doesn't seem particularly trivial, or be removed...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Hmm.  The assertion _used_ to be valid, assuming we really only passed ::after frames in there.

In this case, the next sibling is the colgroup, I assume.  What's the thing being inserted, that it thinks its nextsibling is the colgroup?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Probably fallout from bug 1419964...

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: testcase.html

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #2)
> Hmm.  The assertion _used_ to be valid, assuming we really only passed
> ::after frames in there.
> 
> In this case, the next sibling is the colgroup, I assume.  What's the thing
> being inserted, that it thinks its nextsibling is the colgroup?

Another colgroup. Here's a more straight-forward test-case. I don't think it's fallout from bug 1419964 fwiw, siblings of display: contents nodes already appeared there, they were just not named correctly.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Alright.  Given that, I think this assert is just bogus and should go away.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1427824: Remove invalid assertion in presence of display: contents.

Review commit: https://reviewboard.mozilla.org/r/210586/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/210586/

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8940291 [details]
Bug 1427824: Remove invalid assertion in presence of display: contents.

https://reviewboard.mozilla.org/r/210586/#review216242

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c592bcac2149
Remove invalid assertion in presence of display: contents. r=bz

Comment 9

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/c592bcac2149