Closed Bug 1727083 Opened 2 months ago Closed 2 months ago

Flickering doc updates in about:webrtc got markedly worse

Categories

(Core :: Internationalization, defect)

Desktop
macOS
defect

Tracking

()

VERIFIED FIXED
93 Branch
Tracking Status
firefox-esr78 --- unaffected
firefox-esr91 --- unaffected
firefox91 --- unaffected
firefox92 --- verified
firefox93 --- verified

People

(Reporter: jib, Assigned: ng)

References

(Regression)

Details

(Keywords: regression, Whiteboard: [qf])

Attachments

(1 file)

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.

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

Flags: needinfo?(zbraniecki)
Flags: needinfo?(jib)
Whiteboard: [qf]
Blocks: 1727134
No longer blocks: 1727134
Flags: needinfo?(dminor)

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.

Flags: needinfo?(jib)
See Also: → 1727134

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.

Is there a way to await localization?

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?

Flags: needinfo?(zbraniecki)

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

(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:

Flags: needinfo?(dminor)

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.

Assignee: nobody → na-g
Status: NEW → ASSIGNED
Pushed by na-g@nostrum.com:
https://hg.mozilla.org/integration/autoland/rev/d46c7f77fb00
stop about:webrtc from flickering;r=jib
Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 93 Branch

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:
Attachment #9237896 - Flags: approval-mozilla-beta?

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

Approved for 92.0b9.

Attachment #9237896 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

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).

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.