1727083 - Flickering doc updates in about:webrtc got markedly worse

Status: VERIFIED FIXED

(Core :: Internationalization, defect)

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

STR 1 (simplest steps to reproduce flicker):

1. Prerequisite: make sure to have added a media-related pref to about:config, like media.getusermedia.audiocapture.enabled.
2. Open about:webrtc and wait 10 seconds

Expected result:

• No blinking (though if I look closely, the User Set WebRTC Preferences section blinks once every ~10 seconds or so).

Actual result:

• The User Set WebRTC Preferences blinks every 0.5 seconds.

STR2 (larger areas and entire tables flicker):

1. First open https://jsfiddle.net/jib1/yxbLvjm6/ and allow camera+mic
2. Run STR 1

In this case, when I click Show details, I see entire tables of data (like Ice Stats and Estimated Bandwidth) flicker every 0.5 seconds.

I'm marking this as a regression since the symptoms went from acceptable to unacceptable.

There's likely a preexisting issue here with how about:webrtc updates itself on a timer, so feel free to lob this over to WebRTC, after you're satisfied that bug 1613705 isn't introducing anything that might affect other web pages similarly.

Any theories on why the symptoms got a lot worse, and advice on how to live-update documents properly if anyone has them, would also be welcome.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

zibi - any expectation that bug 1613705 would have perf impacts, here or elsewhere?
jib - can you get profiles of this?

Comment 2

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

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

Comment 3

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Here's a profile of STR1 https://share.firefox.dev/2WhbxBS. Again, lobbing this over to WebRTC is perfectly reasonable, just wanted to get eyes on the significant regression first before fixing about:webrtc, in case there were other side effects.

Comment 4

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Here's a profile of STR2 https://share.firefox.dev/3B8Edwc. This one's a bit more revealing from the Screenshots:

At 6.16s we see the "Ice Stats" table rendered without any headings. The localized headings are missing but the rest of the non-localized data is rendered, causing the table to jump noticeably, which registers as flickering.

This is surprising since all document manipulation in aboutWebrtc.js appears synchronous. Is there some async delay inherent in localization? Or is rendering triggered here when it shouldn't be?

The workaround seems obvious, and should be undertaken for performance alone I think: Cache the localized strings, since they're not likely to change every 500 ms.

I'd nuke the updating of the prefs section for the same reason. It doesn't make sense that it live-updates when users have to hit the ↻ refresh button to see new peer connections.

Comment 5

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

Is there a way to await localization?

Comment 6

[Zibi Braniecki [:zbraniecki][:gandalf]]

zibi - any expectation that bug 1613705 would have perf impacts, here or elsewhere?

The "impact" is in that we changed how microtasks work between JS runtime and localization frame from a microtask to another. If the code was assuming same microtask for execution, then this change pushed it to reflow/relayout.

The perf impact is generally positive and we reduced the length of the l10n frame by 70%, but it may interact with other async code in a way that ultimately affects performance negatively if not planned for frames carefully. I suspect that's what is happening here.

The workaround seems obvious, and should be undertaken for performance alone I think: Cache the localized strings, since they're not likely to change every 500 ms.

That shouldn't be needed and it adds inherently complexity to the code (every cache needs invalidation ;)).

What I'd suggest for such cases as frequent re-rendering is to bundle DOM updates with l10n frames, and only insert into DOM when fully localized:

async function refreshDOM() {
  root.clearChildrenNodes();

  let newFragment = document.createDocumentFragment();
  populateDOM(newFragment); // this populated all `data-l10n-id/data-l10n-args`
  await document.l10n.translateFragment(newFragment);
  root.appendChild(newFragment);
}

this way you'll get localization once per refreshDOM and insert translated DOM.

Would that work for your use case?

Comment 7

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

That should work. I'm surprised this didn't break more stuff. Are docs available on the l10n functions?

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #7)

That should work. I'm surprised this didn't break more stuff. Are docs available on the l10n functions?

https://firefox-source-docs.mozilla.org/l10n/fluent/tutorial.html

and :nordzilla is working on improved DOM L10n docs

For now:

• https://searchfox.org/mozilla-central/source/dom/webidl/Localization.webidl
• https://searchfox.org/mozilla-central/source/dom/webidl/DOMLocalization.webidl
• https://searchfox.org/mozilla-central/source/dom/chrome-webidl/DocumentL10n.webidl

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

if you want to be really specific, you probably will event want to do:

await document.l10n.translateFragment(frag);
document.l10n.pauseObserving();
root.appendChild(frag);
document.l10n.resumeObserving();

the observer shouldn't cause reflow, but it will retranslate when you inject so pausing observing will be prudent.

Comment 10

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Attachment: Bug 1727083 - stop about:webrtc from flickering;r?jib

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d46c7f77fb00
stop about:webrtc from flickering;r=jib

Comment 12

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/d46c7f77fb00

Comment 13

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 9237896 [details]
Bug 1727083 - stop about:webrtc from flickering;r?jib

Beta/Release Uplift Approval Request

• User impact if declined: about:webrtc will be largely unusable due to flickering of contents every 1/2 second
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It is a change that only effects the otherwise broken about:webrtc.
• String changes made/needed:

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9237896 [details]
Bug 1727083 - stop about:webrtc from flickering;r?jib

Approved for 92.0b9.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3cdaa771ee85

Comment 16

[Bogdan Maris, Desktop QA]

Reproduced the flicker issue inside about:webrtc using old Nightly build from 2021-08-23, verified that this is now fixed using latest Beta 92.0b9 and latest Nightly 93.0a1 across platforms (macOS 11.5, Ubuntu 18.04 and Windows 10 64bit).