1673020 - Window recording keyboard shortcuts conflict with Firefox Multi-Account Containers

Status: REOPENED

(Firefox :: General, defect, P3)

Description

[Joe Cardenas]

Attachment: memory-reportVERYBAD.json.gz

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

Steps to reproduce:

I cannot yet find a pattern. I will continue to test.

Actual results:

I was experiencing a lot of OOM crashes, and noticed FF was using much more memory than before. When examining which processes were using the most memory, I found that sometimes the GPU subprocess would explode in size to about 3-4GB. After examining about:memory I noticed that over 95% of the GPU subprocess's memory was heap_unclassified. One time, the about:memory showed about 6GB of memory allocated to the GPU subprocess when task manager only showed about 4. about:memory attached.

Expected results:

I would like the GPU subprocess to not explode in size, crashing itself and causing other OOM issues.

This is one of my first bugs so if there is more info you need then be sure to ask and I will try to do what I can!

Comment 1

[Joe Cardenas]

Workaround: ending the GPU subprocess temporarily breaks FF but then returns memory usage back to normal.

Comment 2

[Joe Cardenas]

Also to note: I only have 8GB memory and integrated graphics. Having this much RAM dedicated to the GPU wouldn't make any sense.

Comment 3

[Clara Guerrero ( Need Info Brindusa Tot Please)]

Hi Joe,
Can you please provide your about:support information?
Thanks in advance!

Setting a component for this in order to get the dev team involved.
(If the team feels it's an incorrect one please feel free to change it to a more appropriate one.)
Regards,
Clara

Comment 4

[Joe Cardenas]

Attachment: about:support info

Comment 5

[Joe Cardenas]

I attatched my about:support.

Comment 6

[Jeff Muizelaar [:jrmuizel]]

Joe, can you get a new memory report with a Nightly with a build id of 20201215213427 or newer?

Comment 7

[Joe Cardenas]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #6)

Joe, can you get a new memory report with a Nightly with a build id of 20201215213427 or newer?

I have just experienced it again on the 2021-01-22 build of Nightly.

I have attached it to the bug as "2021-01-22.json.gz"

Comment 8

[Joe Cardenas]

Attachment: 2021-01-22 memory ballooning

Comment 9

[Joe Cardenas]

By the way, sorry for not replying for so long as my email notifications were not working. I have now fixed them.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Ok yeah, there's 25 GB of gpu-committed and 25 GB of heap unclassified. Something very wrong is obviously happening.

Comment 11

[Joe Cardenas]

Attachment: about:support machine 2 (2021-01-22)

By the way, the initial crashes/memory issues were occurring on a different machine than the one I just took. I will be attaching about:support for the second machine now.

Comment 12

[Joe Cardenas]

By the way, the initial crashes/memory issues were occurring on a different machine than the one I just took. I will be attaching about:support for the second machine now.

Comment 13

[Joe Cardenas]

Because I do not experience these issues on Firefox Developer, (even though the 6 week release cycle has passed) I assume the problem must lie in something specific to the Nightly browser. Over the next while, I'm going to try to figure out if it is any of the "Nightly Experiments", and if so, figure out which it is.

Comment 14

[Jeff Muizelaar [:jrmuizel]]

Might also be worth checking whether Fission is being enabled.

Comment 15

[Joe Cardenas]

Yes, Fission was enabled. I am going to use it on default settings for a while to see if Fission or any of the other things I had enabled makes a difference.

Comment 16

[Joe Cardenas]

I was able to reproduce the error using the default experiments plus Fission. SameSite=Lax default, SameSite=None requires secure, Schemeful SameSite cookies, Fission, and Print Preview Redesign were the only experiments I had enabled. I am not trying the same group enabled, but without Fission.
BTW, sorry for the long wait. I was away with no access to my main Workstation.

Comment 17

[Joe Cardenas]

I have much more information:
The bug seems to be from the "Firefox Multi-Account Containers" add-on. The bug occurs when using a shortcut to open a new tab in a container. As far as I can remember, I was only able to reproduce the issue using the Ctrl + Shift + 4 shortcut, which seems weird. It is definitely possible that I have not done enough testing.

I was able to reproduce the bug on a clean profile of Firefox Nightly (only Multi-Account Containers installed).
I was not able to reproduce the bug on Firefox 86.

If you need any more info, let me know. I can do further testing or provide additional information as needed!

Comment 18

[Jeff Muizelaar [:jrmuizel]]

It turns out that Ctrl + Shift + 4 is actually the short cut that we were using for a multi-frame WebRender capture. You should be able to disable it with gfx.webrender.enable-capture=false. We're also going to change the default so that other people don't run into this. Thanks for sticking with it and figuring out what was going on.

Comment 19

[Joe Cardenas]

I can confirm that on build 20210308213435, gfx.webrender.enable-capture is false by default on Nightly. However, I can also still reproduce the bug. Is there any other obscure functionality tied to Ctrl + Shift + 4 ?

Also, I'd like to ask if you were able to reproduce the bug. If so, did setting gfx.webrender.enable-capture to false fix the problem?

By the way, regardless of the issues I am still having, I would like to thank you for sticking with this incredibly obscure bug. I truly thank you for that.

Comment 20

[Jeff Muizelaar [:jrmuizel]]

Oh weird. If you disable "Firefox Multi-Account Containers" but still press Ctrl + Shift + 4 a bunch can you reproduce the problem?

Comment 21

[Joe Cardenas]

I tried restarting nightly into Safe/Troubleshooting mode, and I was able to reproduce the issue. Also, when I checked about:memory, I found no GPU process (I assume WebRender was disabled for Safe Mode?) and instead found the heap_unclassified listed under the main process.

Comment 22

[Jeff Muizelaar [:jrmuizel]]

Looks like it's actually caused by the "WindowRecording" feature.

Bas, can you avoid leaking or turn this off or hide it behind a pref?

Comment 23

[Bas Schouten (:bas.schouten)]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #22)

Looks like it's actually caused by the "WindowRecording" feature.

Bas, can you avoid leaking or turn this off or hide it behind a pref?

Heh, it doesn't really leak. It simply stores the recording in memory until the recording is ended with Ctrl+Shift+5, (we could make it stop recording after it recorded say, a couple of gig?). Maybe we need to pick a more obscure keyboard shortcut I suppose? Ctrl+Shift+<number> seemed safe because we're already using those kind of keyboard shortcuts to drive the profiler.

Comment 24

[Joe Cardenas]

Personally, picking a more obscure shortcut doesn’t really seem like a solution. Those who use that feature would probably be confused (especially considering there’s little to no reference to these shortcuts)

I would suggest two things:

1. As Jeff says, it would probably be good to hide this shortcut (or many shortcuts, or even all/most of the profiler shortcuts) behind a setting in Preferences
2. Considering these shortcuts are quite obscure, there should be a way to see, configure, and/or manage them, so you could change shortcuts to fit your workflow and reduce duplicated shortcuts. I think this is the best solution. You could even, for example, leave the feature enabled but just give it no default shortcut.

I’m not at my computer, so I’m not sure if I’m correct, but IIRC if you configure a shortcut to one that is already being used (ex: ctrl + t, ctrl + shift + 4) Firefox will warn you that shortcut is already being used. Is there some way we can do this for the shortcuts built in to the profiler?

Of course, this seems like a pretty complex solution to an incredibly obscure bug. I’d rather this bug get fixed than stay in limbo for years waiting on major UI/UX changes, so if you don’t think these changes are worth implementing then I’d be totally okay with that. Also, I’ve never programmed in the FF repository before but I’d be eager to learn so if any additional manpower is needed I would be happy to provide it.

Comment 25

[Jim Mathies [:jimm]]

We fixed our shortcut related to this. The window recording feature should use some other shortcut, or be disabled completely through a pref. I'm not sure who would own that, hoping front-end can get this to the right location.

Comment 26

[Joe Cardenas]

Thanks for the meta changes, Jim. Hopefully, we can get someone on this!

Comment 27

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

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

Comment 28

[Joe Cardenas]

This is not a WebRender issue, so I will be reverting the Bugbug change.