Open Bug 1806276 Opened 2 years ago Updated 1 year ago

Loading spinner SVG on yahoo.mydashboard.oath.com does not draw properly and causes memory leak/crash

Categories

(Web Compatibility :: Desktop, defect, P3)

Firefox 110

Tracking

(firefox113 affected)

Tracking Status
firefox113 --- affected

People

(Reporter: bbhtt.zn0i8, Unassigned)

References

(Depends on 3 open bugs, Blocks 1 open bug, )

Details

(Keywords: crash, hang, memory-leak)

Attachments

(4 files)

Attached video Screencast.webm (deleted) —

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:110.0) Gecko/20100101 Firefox/110.0

Steps to reproduce:

Open https://yahoo.mydashboard.oath.com/ while logged in/logged out

See the spinner on the front page trying to load the privacy summary

Refresh the page and let it load

Interact with toolbar items ie. addon, profiler item hamburger menu etc.

Actual results:

Everything (addon, profiler item hamburger menu etc.) is frozen while that page loads

Expected results:

It should've opened the privacy summary instantly without any delayed spinner and without freeze. On Chromium 108.0.5359.124 the page loads normally without any delayed spinner or freeze.

I suspect the freeze has something to do with the spinner loading.

The issue seems to happen on both latest Fenix and Firefox Nightly. A screencap is attached.

I've attempted to capture a profile but because everything is frozen I can't open the profiler page or refresh/upload it. Firefox doesn't recover from the freeze, after a while system kills it (GNOME's "application not responding" screen appears).

Attached file about:support (deleted) —

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Component: Untriaged → Widget: Gtk
Product: Firefox → Core

Which distro do you run? Can you test with Wayland disabled (run with MOZ_ENABLE_WAYLAND=0).
Thanks.

Blocks: wayland
Flags: needinfo?(bbhtt.zn0i8)
Priority: -- → P3

(In reply to Martin Stránský [:stransky] (ni? me) from comment #3)

Which distro do you run? Can you test with Wayland disabled (run with MOZ_ENABLE_WAYLAND=0).
Thanks.

Arch Linux GNOME 44 Wayland session.

Happens with MOZ_ENABLE_WAYLAND=0 as well

Tested with Nightly 113.0a1 (2023-03-29)

Thanks.

Flags: needinfo?(bbhtt.zn0i8)

Please try to find a regression range (you get a pushlog url at the end):
$ pip3 install mozregression
$ ~/.local/bin/mozregression --good 2022-10-01 --bad 2023-03-31 -a https://yahoo.mydashboard.oath.com/

Keywords: hang

STR:

  1. On Ubuntu 22.10 visit https://yahoo.mydashboard.oath.com/.
  2. If a blue loading spinner does not appear, create a new tab and visit again.

Gets stuck on a broken loading animation. CPU is high, memory climbs in main process (about:memory shows leak in heap-unclassified) and browser becomes unresponsive. Sometimes it recovers but it can continue until crash.

Does not occur with svg.disabled = true or when .loading-holder is blocked. Saving a local copy of the loading SVG does not reproduce the issue but the animation is still broken, looks correct in Chrome.

SVG is only part of DOM during loading. Manually adding SVG after finished loading causes same issue. Manually adding SVG to a different site (eg wikipedia.org) does not reproduce issue.

Does not occur on Windows unless using debugger to pause while SVG is displayed or manually adding the SVG.

Not a recent regression, reproducible with 2020-01-01 build.

No longer blocks: wayland
Status: UNCONFIRMED → NEW
Component: Widget: Gtk → Graphics
Ever confirmed: true
Keywords: crash, memory-leak
Summary: Yahoo Privacy Dashboard - yahoo.mydashboard.oath.com freezes browser → Loading spinner SVG on yahoo.mydashboard.oath.com does not draw properly and causes memory leak/crash
Attached video Broken loading animation SVG (deleted) —
Attached image spinner-leak.svg (deleted) —

Performance profile: https://share.firefox.dev/3Zzfqgt

Component: Graphics → SVG

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

For more information, please visit auto_nag documentation.

Flags: needinfo?(aethanyc)

From the profile in comment 9, I see almost every callstack containing CSPReportSenderRunnable. Maybe this bug is related to CSP?

Component: SVG → DOM: Security
Flags: needinfo?(aethanyc)

Tom, can you investigate a bit? We can reproduce but it's not fully clear what our options are this being an issue with the SVG <animate> feature.
Can you take a look and explore this a bit, before we make a decision how to move forward?

Flags: needinfo?(tschuster)

This is a report only policy so we are not really blocking anything. The problem purely seems to be the totally massive amount of CSP reports that we are trying to send. We had a very similar bug 1804871 before and bug 1657519, which shows that Chrome coalesces reports together.

I think for this bug we should simply stop sending reports after we reached a fixed number of reports for a specific context. I am not sure what a good number would be. The Yahoo page seems to produce about 200 reports and for example stopping after 20 reports makes everything load.

Edit: A higher number 100 also seems to work. The important part seems to be that we stop at some point so what we can actually make progress and finish loading the page.

Flags: needinfo?(tschuster)

We could consider it a dupe of our "coalesce CSP reports" bug, but does Chrome issue reports for this, or block things if it's not a report-only policy? At first glance I didn't see any literal style attribute setting: is it the svg transforms that are being interpreted as inline style? Is that per spec, or just an implementation detail of how we mapped SVG transforms into CSS transforms internally?

re-adding the needinfo for Tom since comment 14 was more what Freddy was asking. We know the reporter's perf problem/hang is from the massive # of reports (a known issue, as you point out) but weren't sure if these are valid violations to begin with.

If the reports are legitimate we should make this "depend on" one of the existing bugs and move this specific bug over to Webcompat to see if we can reach out to Yahoo about changing their page or CSP to work around the problem, or maybe shim it with the Webcompat extension.

If we do move this back to webcompat it would be good to know what we could even do to shim this, short of altering their CSP header in some way (and would we want to do that)? We could try to reach out, but odds are that it would be best to match whatever Chrome is doing here, or we'll probably just run into it again some time (sites apparently are using CSP and other reporting APIs more and more).

The difference between Chrome and Firefox in regards to blocking inline styles caused by <animate> via CSP is covered by bug 1459872.

Flags: needinfo?(tschuster)

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

For more information, please visit BugBot documentation.

Flags: needinfo?(fbraun)
Severity: -- → S3
Flags: needinfo?(fbraun)
Depends on: 1459872
Depends on: 1657519

If you save the SVG to a local file and view it without ANY content-security-policy involved at all it still looks terrible compared to webkit and blink browsers. Made my "file://" child process burn 5% CPU just trying to animate it badly. (On a mac).

It appears marginally more like it's supposed to if you load it as an <IMG> but it's still not right. I can at least see dots forming the entire circle, but the animation doesn't go around. To test, copy the following and load from a new window. No CSP involved:
data:text/html,<img src=https://bug1806276.bmoattachments.org/attachment.cgi?id=9326270>

We have the other bugs for "CSP related to SVG" and "Limit excessive CSP violation reports". I'm sending this over to layout as an SVG correctness issue.

Component: DOM: Security → SVG

The file is invalid because begin attributes that are < 1 and > -1 must begin with 0 or -0. Check out the console, it's full of errors.

Chrome's parsing is invalid per the SVG and SMIL specifications. I raised https://github.com/w3c/svgwg/issues/394 but it's had no traction. While it talks about dur, begin is just the same for parsing: https://www.w3.org/TR/SVG11/animate.html#ClockValueSyntax

So the display by Firefox is correct per the current specifications, although I would like to see the specification changed to allow this.

Do you want this to be a Web Compatibility bug? Or.a wayland memory leak bug? Or a CSP bug?

Whatever it is, it's not strictly an SVG bug.

Flags: needinfo?(dveditz)

(In reply to Robert Longson [:longsonr] from comment #20)

So the display by Firefox is correct per the current specifications, although I would like to see the specification changed to allow this.

Thanks, Robert. Looking at the spec issue, it sounds like nobody is opposed to the proposal, but it's just a question of defining the grammar in spec terms.

Given that, and given that we're the only ones with our current behavior, I'd suggest we should just go ahead and make the leading zero optional, independent of the spec status, to get the compat win and make sites like this render as-intended. The spec text can catch up when the svg spec editors are ready to update it, but we don't need to block our users' experience on the spec change being finalized, unless there's some substantial ambiguity around particular edge cases that we need to wait on clarification for or something like that.

Do you want this to be a Web Compatibility bug? Or.a wayland memory leak bug? Or a CSP bug?

I suspect doing outreach to get the SVG file updated might be more trouble than it's worth. It looks like we have bug 1459872 and bug 1657519 already filed for the CSP issues here, and I think the memory issues are related to that (from us aggressively trying to report CSP violations); that's not wayland-specific, per comment 4.

Having said that: the SVG leading-0 parsing issue is entirely unrelated to the more-concerning freeze/etc. that the user reported in comment 0, so probably we should refocus this bug on that issue that the user reported, and spin off a new bug for the SVG parsing issue.

So the issue with this bug here (the badness described in comment 0) is essentially some blocking happening per bug 1459872, which then results in zillions of reports which we fail to coalesce per bug 1657519. Presumably we could fix this issue by fixing either of those bugs?

Component: SVG → DOM: Security
Flags: needinfo?(dveditz)

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

I'd suggest we should just go ahead and make the leading zero optional,
[...] probably we should refocus this bug on that issue that the user reported, and spin off a new bug for the SVG parsing issue.

I spun off bug 1834892 for that parsing issue resulting in the visual glitch in the animation that dveditz described.

(In reply to Robert Longson [:longsonr] from comment #20)

The file is invalid because begin attributes that are < 1 and > -1 must begin with 0 or -0. Check out the console, it's full of errors.

Chrome's parsing is invalid per the SVG and SMIL specifications. I raised [....] So the display by Firefox is correct per the current specifications, although I would like to see the specification changed to allow this.

Safari handles the file the same way as Chrome and all the other chromium browsers out there. "technically correct" is not helping us when the spec doesn't match reality :-(

Do you want this to be a Web Compatibility bug? Or.a wayland memory leak bug? Or a CSP bug?

It doesn't make sense as a CSP bug since the work would still be in the other two identified CSP bugs. I thought comment 4 ruled out wayland? Fixing the SVG parsing won't change the overwhelm of CSP animation errors so I guess it should be a web compat bug that depends on the code changes we'd like to make.

Whatever it is, it's not strictly an SVG bug.

True, but there was some kind of SVG webcompat issue involved in the display -- but now Daniel has filed bug 1834892 for that so we don't need to use this bug for that. So Webcompat it is.

Component: DOM: Security → Desktop
Depends on: 1834892
Product: Core → Web Compatibility
Depends on: 1839165
Depends on: 1848303
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: