1587629 - ASSERT in xul.dll!mozilla::ViewportFrame::BuildDisplayListForTopLayer

Status: NEW

(Core :: Layout, defect, P3)

Description

[thomasmo]

Attachment: fxr_fullscreen_ViewportFrame_assert.patch

While investigating Bug 1587521 - Enable FullScreen in FxR for Desktop (which is built upon Bug 1505916 - [Fission] Make the fullscreen code Fission-aware), I hit this assert while trying to go full screen

      if (!(frame->GetStateBits() & NS_FRAME_OUT_OF_FLOW)) {
        MOZ_ASSERT(!elem->GetParent()->IsHTMLElement(),
                   "HTML element should always be out-of-flow if in the top "
                   "layer");
        continue;
      }

To repro:

• apply this patch: https://d3kxowhw4s8amj.cloudfront.net/file/data/kdda53itoxcekqaldcea/PHID-FILE-aqhwkjkmkqr2y5c5easq/D35026.diff fxr_fullscreen_ViewportFrame_assert_base.patch
• then apply this attached patch: fxr_fullscreen_ViewportFrame_assert.patch
• build a debug version of firefox
• run mach run --chrome chrome://fxr/content/fxrui.html
• click 'fullscreen' button
• observe assert

#01: mozilla::ScrollFrameHelper::MaybeAddTopLayerItems (e:\src3\mozilla-central\layout\generic\nsGfxScrollFrame.cpp:3823)
#02: mozilla::ScrollFrameHelper::BuildDisplayList (e:\src3\mozilla-central\layout\generic\nsGfxScrollFrame.cpp:3741)
#03: nsHTMLScrollFrame::BuildDisplayList (e:\src3\mozilla-central\layout\generic\nsGfxScrollFrame.h:786)
#04: nsIFrame::BuildDisplayListForChild (e:\src3\mozilla-central\layout\generic\nsFrame.cpp:4135)
#05: mozilla::ViewportFrame::BuildDisplayList (e:\src3\mozilla-central\layout\generic\ViewportFrame.cpp:64)
#06: nsIFrame::BuildDisplayListForStackingContext (e:\src3\mozilla-central\layout\generic\nsFrame.cpp:3343)
#07: nsLayoutUtils::GetFramesForArea (e:\src3\mozilla-central\layout\base\nsLayoutUtils.cpp:3114)
#08: nsLayoutUtils::GetFrameForPoint (e:\src3\mozilla-central\layout\base\nsLayoutUtils.cpp:3078)
#09: mozilla::FindFrameTargetedByInputEvent (e:\src3\mozilla-central\layout\base\PositionedEventTargeting.cpp:548)
#10: mozilla::PresShell::EventHandler::GetFrameToHandleNonTouchEvent (e:\src3\mozilla-central\layout\base\PresShell.cpp:6774)
#11: mozilla::PresShell::EventHandler::ComputeEventTargetFrameAndPresShellAtEventPoint (e:\src3\mozilla-central\layout\base\PresShell.cpp:6824)
#12: mozilla::PresShell::EventHandler::HandleEventUsingCoordinates (e:\src3\mozilla-central\layout\base\PresShell.cpp:6650)
#13: mozilla::PresShell::EventHandler::HandleEvent (e:\src3\mozilla-central\layout\base\PresShell.cpp:6531)
#14: mozilla::PresShell::HandleEvent (e:\src3\mozilla-central\layout\base\PresShell.cpp:6459)
#15: nsViewManager::DispatchEvent (e:\src3\mozilla-central\view\nsViewManager.cpp:751)
#16: mozilla::PresShell::DispatchSynthMouseMove (e:\src3\mozilla-central\layout\base\PresShell.cpp:3665)
#17: mozilla::PresShell::ProcessSynthMouseMoveEvent (e:\src3\mozilla-central\layout\base\PresShell.cpp:5463)
#18: mozilla::PresShell::nsSynthMouseMoveEvent::WillRefresh (e:\src3\gecko_build_debug\dist\include\mozilla\PresShell.h:1951)
#19: nsRefreshDriver::Tick (e:\src3\mozilla-central\layout\base\nsRefreshDriver.cpp:1931)
#20: mozilla::RefreshDriverTimer::TickDriver (e:\src3\mozilla-central\layout\base\nsRefreshDriver.cpp:373)
#21: mozilla::RefreshDriverTimer::TickRefreshDrivers (e:\src3\mozilla-central\layout\base\nsRefreshDriver.cpp:344)
#22: mozilla::RefreshDriverTimer::Tick (e:\src3\mozilla-central\layout\base\nsRefreshDriver.cpp:369)
#23: mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers (e:\src3\mozilla-central\layout\base\nsRefreshDriver.cpp:807)
#24: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver (e:\src3\mozilla-central\layout\base\nsRefreshDriver.cpp:729)
#25: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run (e:\src3\mozilla-central\layout\base\nsRefreshDriver.cpp:525)
#26: nsThread::ProcessNextEvent (e:\src3\mozilla-central\xpcom\threads\nsThread.cpp:1225)
#27: NS_ProcessNextEvent (e:\src3\mozilla-central\xpcom\threads\nsThreadUtils.cpp:486)
#28: mozilla::ipc::MessagePump::Run (e:\src3\mozilla-central\ipc\glue\MessagePump.cpp:88)
#29: MessageLoop::RunInternal (e:\src3\mozilla-central\ipc\chromium\src\base\message_loop.cc:315)
#30: MessageLoop::RunHandler (e:\src3\mozilla-central\ipc\chromium\src\base\message_loop.cc:309)
#31: MessageLoop::Run (e:\src3\mozilla-central\ipc\chromium\src\base\message_loop.cc:291)
#32: nsBaseAppShell::Run (e:\src3\mozilla-central\widget\nsBaseAppShell.cpp:139)
#33: nsAppShell::Run (e:\src3\mozilla-central\widget\windows\nsAppShell.cpp:406)
#34: nsAppStartup::Run (e:\src3\mozilla-central\toolkit\components\startup\nsAppStartup.cpp:276)
#35: XREMain::XRE_mainRun (e:\src3\mozilla-central\toolkit\xre\nsAppRunner.cpp:4600)
#36: XREMain::XRE_main (e:\src3\mozilla-central\toolkit\xre\nsAppRunner.cpp:4735)
#37: XRE_main (e:\src3\mozilla-central\toolkit\xre\nsAppRunner.cpp:4816)
#38: mozilla::BootstrapImpl::XRE_main (e:\src3\mozilla-central\toolkit\xre\Bootstrap.cpp:45)
#39: do_main (e:\src3\mozilla-central\browser\app\nsBrowserApp.cpp:218)
#40: NS_internal_main (e:\src3\mozilla-central\browser\app\nsBrowserApp.cpp:300)
#41: wmain (e:\src3\mozilla-central\toolkit\xre\nsWindowsWMain.cpp:131)
#42: __scrt_common_main_seh (d:\agent\_work\2\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
#43: BaseThreadInitThunk[C:\WINDOWS\System32\KERNEL32.DLL +0x17bd4]
#44: RtlUserThreadStart[C:\WINDOWS\SYSTEM32\ntdll.dll +0x6ced1]

Comment 1

[Emilio Cobos Álvarez (:emilio)]

apply this patch: https://d3kxowhw4s8amj.cloudfront.net/file/data/kdda53itoxcekqaldcea/PHID-FILE-aqhwkjkmkqr2y5c5easq/D35026.diff

Doesn't seem to apply locally... What is elem->GetParent() at the point of the crash?

You can print that with elem->GetParent()->List(stderr) or such.

Comment 2

[thomasmo]

Here's the parent

div@000001921CE4D670 id="eBrowserContainer" class="browser_container" state=[180020800004] flags=[00040000] primaryframe=000001921CE3D3A8 refcount=3<
  XUL* browser@000001921CE4EDF0 type="content" remote="true" state=[80020800206] flags=[00000002] primaryframe=000001921D298200 refcount=139<>
>

Also, the chrome:// is referring to this
https://searchfox.org/mozilla-central/source/browser/fxr/content/fxrui.html
and the <browser> is created and added to DOM via
https://searchfox.org/mozilla-central/source/browser/fxr/content/fxrui.js#47

Comment 3

[thomasmo]

Attachment: fxr_fullscreen_ViewportFrame_assert_base.patch

If the long link to the base patch doesn't work, here is an export of my import, based upon
Changeset: (e1a65223d498) Bug 1580097 - Revert bogus metadata change, r=aryx …

Comment 4

[thomasmo]

For triage purposes: this bug is no longer blocking me, as I found that this assert is not directly related to the root cause of my local issue.
It does make it harder for me to self-host debug builds, but I can remove the assert locally when I need to.

Comment 5

[Daniel Holbert [:dholbert]]

(the demo-patch in comment 3 mistakenly made Bugzilla think it should assign the bug to you -- undoing that)

Do let us know if you are adding (or intend to add) content to Firefox that triggers the assertion (in debug builds), because that still indicates we're getting into an unexpected & potentially unstable situation, and we probably don't want to be doing that during normal operation.

Comment 6

[thomasmo]

Yes-- will do!