1867290 - Missing newlines in errors breaks glxtest

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[David Turner]

Attachment: support_bad

Steps to reproduce:

• Running on Raspberry Pi OS Bookworm, based on Debian Bookworm, using the Wayfire wayland compositor.
• Upgraded from Firefox 119 to 120

Actual results:

Upgrading from Firefox 119 to 120 breaks DMAbufs, which drastically reduces performance on WebGL. I did a git bisect and the problem is a regression in from https://phabricator.services.mozilla.com/D189920 from Bug 1856582.

Digging into it, the problem is because that commit changes record_warning and record_error and removes the newline after the content of the warning/error. Since the warning/error messages themselves don't contain newlines, this means that the next line in glxtest output gets appended on the same line as the warning/error. In my case this means the DRM_RENDERDEVICE line gets glued onto the preceding warning and so the parser doesn't see it and disables DMAbufs.

I've attached my about:support from the commit before and after the breakage, the problem can be seen in the gfx failure log.

Comment 1

[David Turner]

Attachment: support_good

Comment 2

[David Turner]

@stransky If you agree with my assessment I can get a diff up for this. The fix might be worth uplifting if it's going to break random gfx features any time there is a warning from glxtest

Comment 3

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

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

Comment 4

[David Turner]

Attachment: Bug 1867290 - Add back newline in record_warning and record_error r?stransky

As part of other changes, bug 1856582 removed newlines at the end of
record_warning() and record_error() messages. But most uses of these
macros don't put newlines in themselves. This meant that warnings and
errors in glxtest output were missing newlines and so the following line
would get appended to the error/warning message. This could break
various things depending what the next line after the error/warning was
meant to be.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/92ebd26049f6
Add back newline in record_warning and record_error r=stransky

Comment 6

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/92ebd26049f6

Comment 7

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

Set release status flags based on info from the regressing bug 1856582

Comment 8

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

The patch landed in nightly and beta is affected.
:david.turner, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox121 to wontfix.

For more information, please visit BugBot documentation.

Comment 9

[David Turner]

Attachment: Bug 1867290 - Add back newline in record_warning and record_error

As part of other changes, bug 1856582 removed newlines at the end of
record_warning() and record_error() messages. But most uses of these
macros don't put newlines in themselves. This meant that warnings and
errors in glxtest output were missing newlines and so the following line
would get appended to the error/warning message. This could break
various things depending what the next line after the error/warning was
meant to be.

Original Revision: https://phabricator.services.mozilla.com/D194997

Comment 10

[Phabricator Automation]

Uplift Approval Request

• Fix verified in Nightly: yes
• Steps to reproduce for manual QE testing: Check that gfx acceleration features in about:support are correct for the platform
• Is Android affected?: no
• String changes made/needed: I don't think these strings are user facing / localised
• Explanation of risk level: Small change to a particular area which just restores previous behaviour
• Code covered by automated testing: no
• Risk associated with taking this patch: Low
• User impact if declined: gfx hardware acceleration features may not work at random
• Needs manual QE test: no

Comment 11

[Ryan VanderMeulen (PTO, back July 27)]

Comment on attachment 9367241 [details]
Bug 1867290 - Add back newline in record_warning and record_error

Approved for 121.0b9.

Comment 12

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/8071f36fd325