1548896 - Assertion failure: aSize.width >= 0.0 && aSize.height >= 0.0, at src/layout/base/nsLayoutUtils.cpp:8845

Status: RESOLVED FIXED

(Core :: Layout: Columns, defect, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Assertion failure: aSize.width >= 0.0 && aSize.height >= 0.0, at src/layout/base/nsLayoutUtils.cpp:8845

eip = 0x87ed7818   esp = 0x8dcfddf0   ebp = 0x8dcfde68   ebx = 0x8d0deddc
esi = 0x0000228d   edi = 0x66a16550   eax = 0x8aa94ebb   ecx = 0x8e13317c
edx = 0x00000094   efl = 0x00210282
OS|Android|0.0.0 Linux 4.4.124+ #1 SMP PREEMPT Sun Nov 4 14:31:25 UTC 2018 i686
CPU|x86|GenuineIntel family 6 model 6 stepping 3|4
GPU|||
Crash|SIGSEGV|0x0|13
13|0|libxul.so|nsLayoutUtils::SetVisualViewportSize(mozilla::PresShell*, mozilla::gfx::SizeTyped<mozilla::CSSPixel, float>)|hg:hg.mozilla.org/mozilla-central:layout/base/nsLayoutUtils.cpp:725cc368d14453980d6e0b112edc263a12463ffe|8845|0x22
13|1|libxul.so|mozilla::GeckoMVMContext::SetVisualViewportSize(mozilla::gfx::SizeTyped<mozilla::CSSPixel, float> const&)|hg:hg.mozilla.org/mozilla-central:layout/base/GeckoMVMContext.cpp:725cc368d14453980d6e0b112edc263a12463ffe|135|0x1c
13|2|libxul.so|MobileViewportManager::UpdateVisualViewportSize(mozilla::gfx::IntSizeTyped<mozilla::ScreenPixel> const&, mozilla::gfx::ScaleFactor<mozilla::CSSPixel, mozilla::ScreenPixel> const&)|hg:hg.mozilla.org/mozilla-central:layout/base/MobileViewportManager.cpp:725cc368d14453980d6e0b112edc263a12463ffe|436|0x19
13|3|libxul.so|MobileViewportManager::UpdateResolution(nsViewportInfo const&, mozilla::gfx::IntSizeTyped<mozilla::ScreenPixel> const&, mozilla::gfx::SizeTyped<mozilla::CSSPixel, float> const&, mozilla::Maybe<float> const&, MobileViewportManager::UpdateType)|hg:hg.mozilla.org/mozilla-central:layout/base/MobileViewportManager.cpp:725cc368d14453980d6e0b112edc263a12463ffe|403|0x13
13|4|libxul.so|MobileViewportManager::RefreshViewportSize(bool)|hg:hg.mozilla.org/mozilla-central:layout/base/MobileViewportManager.cpp:725cc368d14453980d6e0b112edc263a12463ffe|529|0x26
13|5|libxul.so|mozilla::PresShell::ResizeReflow(int, int, int, int, mozilla::ResizeReflowOptions)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:725cc368d14453980d6e0b112edc263a12463ffe|1833|0x15
13|6|libxul.so|nsViewManager::DoSetWindowDimensions(int, int)|hg:hg.mozilla.org/mozilla-central:view/nsViewManager.cpp:725cc368d14453980d6e0b112edc263a12463ffe|183|0x20
13|7|libxul.so|nsViewManager::FlushDelayedResize(bool)|hg:hg.mozilla.org/mozilla-central:view/nsViewManager.cpp:725cc368d14453980d6e0b112edc263a12463ffe|231|0xd
13|8|libxul.so|mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.cpp:725cc368d14453980d6e0b112edc263a12463ffe|4184|0x1e
13|9|libxul.so|mozilla::PresShell::FlushPendingNotifications(mozilla::ChangesToFlush)|hg:hg.mozilla.org/mozilla-central:layout/base/PresShell.h:725cc368d14453980d6e0b112edc263a12463ffe|1442|0x18
13|10|libxul.so|nsRefreshDriver::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp)|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:725cc368d14453980d6e0b112edc263a12463ffe|1949|0x14
13|11|libxul.so|mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&)|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:725cc368d14453980d6e0b112edc263a12463ffe|348|0x33
13|12|libxul.so|mozilla::RefreshDriverTimer::Tick(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp)|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:725cc368d14453980d6e0b112edc263a12463ffe|341|0x4e
13|13|libxul.so|mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::layers::BaseTransactionId<mozilla::VsyncIdType>, mozilla::TimeStamp)|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:725cc368d14453980d6e0b112edc263a12463ffe|708|0x41
13|14|libxul.so|mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run()|hg:hg.mozilla.org/mozilla-central:layout/base/nsRefreshDriver.cpp:725cc368d14453980d6e0b112edc263a12463ffe|508|0x3d
13|15|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:725cc368d14453980d6e0b112edc263a12463ffe|1180|0x16
13|16|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:725cc368d14453980d6e0b112edc263a12463ffe|486|0x11
13|17|libxul.so|mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:725cc368d14453980d6e0b112edc263a12463ffe|88|0xd
13|18|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:725cc368d14453980d6e0b112edc263a12463ffe|315|0x16
13|19|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:725cc368d14453980d6e0b112edc263a12463ffe|290|0xb
13|20|libxul.so|nsBaseAppShell::Run()|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:725cc368d14453980d6e0b112edc263a12463ffe|137|0xe
13|21|libxul.so|nsAppStartup::Run()|hg:hg.mozilla.org/mozilla-central:toolkit/components/startup/nsAppStartup.cpp:725cc368d14453980d6e0b112edc263a12463ffe|276|0x18
13|22|libxul.so|XREMain::XRE_mainRun()|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsAppRunner.cpp:725cc368d14453980d6e0b112edc263a12463ffe|4554|0x10
13|23|libxul.so|XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsAppRunner.cpp:725cc368d14453980d6e0b112edc263a12463ffe|4692|0x8
13|24|libxul.so|XRE_main(int, char**, mozilla::BootstrapConfig const&)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsAppRunner.cpp:725cc368d14453980d6e0b112edc263a12463ffe|4773|0xf
13|25|libxul.so|GeckoStart|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsAndroidStartup.cpp:725cc368d14453980d6e0b112edc263a12463ffe|47|0xd
13|26|libxul.so|mozilla::BootstrapImpl::GeckoStart(_JNIEnv*, char**, int, mozilla::StaticXREAppData const&)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/Bootstrap.cpp:725cc368d14453980d6e0b112edc263a12463ffe|77|0x11
13|27|libmozglue.so|Java_org_mozilla_gecko_mozglue_GeckoLoader_nativeRun|hg:hg.mozilla.org/mozilla-central:mozglue/android/APKOpen.cpp:725cc368d14453980d6e0b112edc263a12463ffe|372|0x2a
13|28|libart.so||||0x634318

Comment 1

[Daniel Holbert [:dholbert]]

Hmm, I can't reproduce in my local debug build. Any hints about how to make it repro?

Note that the testcase's window.resizeTo() call is non-functional unless this is run in a popup that has been opened by a separate HTML launcher file (with the "resizable" option), I think.

So I used the following launcher.html file to load your testcase (which I saved locally and named test.html):

<!DOCTYPE html>
<button onclick='window.open("test.html", "my-window", "resizable")'>Click</button>

But even with that, I don't get any assertion failures. So there must be something more required here.

Comment 2

[Tyson Smith [:tsmith]]

(In reply to Daniel Holbert [:dholbert] from comment #1)

So there must be something more required here.

Did you try on Android? I was only able to repro on a Fennec build.

Comment 3

[Daniel Holbert [:dholbert]]

Ah, no -- I was running on Linux (and don't have an android build/test env set up, so I can't test there easily at the moment.)

For future reference when someone gets to this, though: do you get the issue when you just load the test file directly, on Android? Or do you need a launcher.html helper file like the one I posted, e.g. in order for resizeTo to do anything? (and/or anything more beyond that?)

Comment 4

[Daniel Holbert [:dholbert]]

hg log MobileViewportManager.cpp shows that bradwerth has been working in the vicinity of MobileViewportManager::UpdateVisualViewportSize (near the bottom of the crash stack), so this might be in his wheelhouse.

Comment 5

[Brad Werth [:bradwerth] | Out until Feb 2025]

Needinfo'ing myself to remind me to get to this after some other work.

Comment 6

[Daniel Holbert [:dholbert]]

(ni for Tyson, for end of comment 3)

Comment 7

[Tyson Smith [:tsmith]]

Oh yes launcher.html is required, thanks Dan.

Setting dom.disable_window_move_resize=false is also required (along with layout.css.column-span.enabled=true of course)

Comment 8

[Brad Werth [:bradwerth] | Out until Feb 2025]

Looks like any pathway that sets a width or height of zero on a window with a MobileViewportManager will hit this assert. I'm not sure what general protections we have about setting those sizes, but the code in MVM doesn't specifically need to avoid those values (no divide-by-zero). The values here will become the values cached in PresShell::mVisualViewportSize and are gettable with PresShell::GetVisualViewportSize(). At least some of the callers of that getter seem prepared to handle zero width or zero height.

Anyway, I think the simple solution is to remove the assert. It looks like we otherwise handle zero width and zero height viewport sizes and there's no reason to object to it here.

Comment 9

[Brad Werth [:bradwerth] | Out until Feb 2025]

Whoops, I just looked more carefully and noticed that the assert tolerates zero width and zero height. What's actually happening is that MobileViewportManager::UpdateVisualViewportSize is calling MobileViewportManager::GetCompositionSize which subtracts away scrollbar sizes from the supplied size, and that's computing to a negative size. I'll clamp it there to (0,0) and we should be good.

Comment 10

[Daniel Holbert [:dholbert]]

Great! I was just writing up a comment much to the same effect. :) (I didn't realize it was specifically scrollbars, but I was guessing there was some piece of UI that was stealing some size and pushing us from 0 into negative territory.)

Thanks, Brad!

Comment 11

[Brad Werth [:bradwerth] | Out until Feb 2025]

Attachment: Bug 1548896 Part 1: Prevent MobileViewportManager::GetCompositionSize from returning negative sizes.

Comment 12

[Brad Werth [:bradwerth] | Out until Feb 2025]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba9b0666816802624c38e64e0e37a72324cc1dd6

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0eff48265098
Part 1: Prevent MobileViewportManager::GetCompositionSize from returning negative sizes. r=botond

Comment 14

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/0eff48265098