1746084 - Resizing Our Facebook App Crashes Firefox from a Stack Overflow Error

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[cphillips]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:95.0) Gecko/20100101 Firefox/95.0

Steps to reproduce:

I am a developer for an app called "Slingo Casino" hosted on Facebook using AngularJS. We have discovered hat opening the app in Firefox, and resizing the window, can cause it to crash.
Repro steps:
Open Slingo Casino using the URL below
Allow the game to load
Stay in the main menu, or click on one of the sub lobbies
Repeatedly maximize, minimize, resize in any order or way, experimenting with different approaches, as in the video
If necessary, refresh the game and start again
observe that soon enough, either after a few seconds, or perhaps longer, the game crashes

https://apps.facebook.com/slingocasino

Actual results:

The crash report indicates that a Stack Overflow occurs, with 2 stack frame signatures being repeatedly applied, indicating a loop that exhausts the memory application, possibly related to resizing. https://crash-stats.mozilla.org/report/index/320cb9fd-a7ed-4ddd-9605-a99530211214#allthreads

I am unable to ascertain exactly what is causing this. We do know however that this was not always occurring, so some change to our code enabled the problem to occur. We think we have pinned it down to a specific branch too. Some of the changes in that branch are additional APNG files, not using some PNG files as compressed, using some css animations, and using some css filters (e.g. brightness) but we'd be happy to share that in a private channel, as well as a video of the repro.

Expected results:

The game should of course not crash. Our very similar predecessor app Slingo Arcade does not have this issue, so something we have done surely triggered the bug to occur. The bug also does not occur in any other browser.

Comment 1

[cphillips]

Attachment: crashingapp.mp4

This is a video of the repro

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Toolkit::Crash Reporting' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:gsvelto, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Gabriele Svelto [:gsvelto] (PTO)]

Moving to the appropriate component; the crash appears to be happening because of extremely deep recursion in the layout code. Also adding the crash signature so that we can assess the impact on users.

Comment 5

[Gabriele Svelto [:gsvelto] (PTO)]

Adding two more signatures. Given this is a stack overflow it's likely we can't intercept it under Linux and macOS so the volume here is underestimating the actual impact. See bug 1678152 for the potential fix on Linux.

Comment 6

[Gabriele Svelto [:gsvelto] (PTO)]

Fixed a typo.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

I can repro, can try to take a look.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1746084 - Avoid generating InterpolateMatrix operations if there are no size dependencies. r=hiro,boris,birtles

The issue here is that we end up with a transition between mismatched
transform lists that ends up generating an InterpolateMatrix {}
operation. So far so good, but we end up interpolating that a lot of
times and generating an unboundedly-deep operation list.

This implementas an optimization that flattens them to a single matrix
when possible (when there's no dependencies on the containing box).

This is similar to:

https://chromium.googlesource.com/chromium/src.git/+/2b89cc4df436e672ef9cf940d1c0dc73fef82a4a

We fix the to_pixel_length() behavior for LenghtPercentage to be
correct (and update callers to preserve behavior).

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/517d3dae5dc1
Avoid generating InterpolateMatrix operations if there are no size dependencies. r=hiro

Comment 10

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/517d3dae5dc1

Comment 11

[cphillips]

:malexandru Can I please ask, if you don't mind, approximately what date this fix would roll out to the main feature release of F.Fox?

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Can you confirm that a Nightly build doesn't repro the issue for you anymore? I just tried to repro and couldn't, just confirming that the fix works on your end too (it should) since it was somewhat intermittent for me too.

This landed on time for Firefox 97, which according to this page should ship in about a month (2022-02-08). I can try to merge it into the 96 branch, though I suspect it will be somewhat hard because it's in release-candidate period right now (ships tomorrow) and this is not a recent regression per se.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9257047 [details]
Bug 1746084 - Avoid generating InterpolateMatrix operations if there are no size dependencies. r=hiro,boris,birtles

Beta/Release Uplift Approval Request

• User impact if declined: Crashes like the ones described in comment 0. Note that crash count could be higher than our crash stats indicate as per comment 5.

Probably won't make it for 96.0, but might be worth taking in a dot-release if we have one.

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Relatively straight-forward optimization to avoid the stack overflow in a well-tested codepath.
• String changes made/needed: none

Comment 14

[cphillips]

:emilio I confirmed with my team that the issue was not able to repro on the nightly build! The fix works!

Comment 15

[Catalin Sasca, Desktop QA [:csasca]]

Was able to reproduce the tab crash on Firefox 97.0a1 (2021-12-14) under macOS 12.1 using the info provided in Comment 0.

The issue is no longer reproducible on latest Firefox Nightly (2022-01-10) and Firefox 97.0b1. Tests were performed on macOS 12.1, Ubuntu 20.04 and Windows 11.

Will verify it on dot release as well if it gets the approval.

Comment 16

[Dianna Smith [:diannaS]]

Comment on attachment 9257047 [details]
Bug 1746084 - Avoid generating InterpolateMatrix operations if there are no size dependencies. r=hiro,boris,birtles

Approved by 96.0.2

Comment 17

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-release/rev/a1e89a2bb5a0

Comment 18

[Catalin Sasca, Desktop QA [:csasca]]

Verified that the issue is no longer reproducible on Firefox 96.0.2 as well. Tests were performed on macOS 11.6.2, Ubuntu 20.04 and Windows 11.