Bug 1552596 - Revert dynamic line clamping in favor of performant static line clamping

[Mardak]

Changed approach. See #5044 (comment)

---

Original comment:
r?@ScottDowne or @gvn Looks like there was a significant performance regression with #5025 because every time a card mounts, it immediately queries scrollHeight forcing a synchronous reflow for layout to return the computed size, and this happens for every card back to back. Even worse is that once we get the size information, we then set a clamp style invalidating the sizing information just calculated resulting in yet another reflow when handling the next child.

The fix here introduces a frame delay so that layout is settled and sizes are cached, and after all the line sizing computation is done, on the next frame update the clamp for all children at the same time. This does add a delay that happens to avoid the missing styles issue with separatePrivilegedContentProcess=true and results in some jank when a later frame fixes up the sizing, but in the common case, a new tab will be preloaded with the final sizing ready by the time the page becomes visible.

Before:
553ms for applying clamp when reloading the page 10 times
https://profiler.firefox.com/public/a94a92c96bafc55825371dc1ded49db85a5b7fc5/calltree/?implementation=js&search=clamptotal&thread=2&v=3

After:
47ms for applying clamp when reloading the page 10 times
https://profiler.firefox.com/public/7842f2721e6d29b4cb9a50b67d2672e041a3fd6b/calltree/?implementation=js&search=clamptotal&thread=2&v=3

[Mardak]

Not sure why DSImage coverage changed to result in test failures, but I added a commit to get to 100% coverage to make it happy…

[ScottDowne]

My current thoughts on this are we back out #5025 and uplift that backout, and fix this above bug along with 5025 in nightly.

Assuming I'm fully understanding all the pieces (if not I'm happy to be corrected on any of the following :))

My reasoning is:

1. I can live without line clamping for beta (68).
2. It's exposing an image rendering bug that I'm not sure about NOBUG - More aggressive measurement checking #5051
3. I'm not sure if I'm ok with the flash of un clamped lines before the clamping happens.
4. Performance issues make me nervous, and I want a bit more time (a nightly cycle) to work through that.
5. We need to do at least 1 uplift anyway, either 1 uplift to back it out, or two uplifts to fix the performance and another to fix the image bug.
6. "in the common case, a new tab will be preloaded with the final sizing ready by the time the page becomes visible" I'm not sure if I'm ok with that. It's a bit of a perceived performance issue on first load, and first load performance is still important enough to block releasing this because of a regression.

If we back out 5025 we can fix everything in nightly.

[gvn]

I agree with Scott. It doesn't seem worth the risk of the perf regression (either perceptual visual jank and/or slowdown to "settled" state) to keep this in beta.

Honestly, I think we need to have a discussion of how fundamentally crucial the dynamic clamping really is with product and design. It feels like a "nice-to-have" IMO.

Any time we're adding JS to adjust layout post-render is going to be a perf risk. Also, once we get into implementing a responsive layout we'll need to retrigger post-render JS adjustments every time the window is scaled, which is not ideal.

[punamdahiya]

We should be backing out #5041 with #5025

[punamdahiya]

Two backouts above will have title and description text to static line clamping as implemented #5014

[Mardak]

@ScottDowne Updated PR with removal of clamp-total-lines and related data-{total-lines,clamp} attributes.

Before:

After:

Using this to test longer stuff:

diff --git a/content-src/lib/selectLayoutRender.js b/content-src/lib/selectLayoutRender.js
--- a/content-src/lib/selectLayoutRender.js
+++ b/content-src/lib/selectLayoutRender.js
@@ -109,8 +109,11 @@ export const selectLayoutRender = (state, prefs, rickRollCache) => {
     for (let i = 0; i < items; i++) {
       data.recommendations[i] = {
         ...data.recommendations[i],
         pos: positions[component.type]++,
+        title: `${data.recommendations[i].title} `.repeat(3),
+        excerpt: `${data.recommendations[i].excerpt} `.repeat(3),
+        domain: data.recommendations[i].domain.repeat(3),
       };
     }
 
     return {...component, data};

[gvn]

Tested locally and this is working properly. Backout looks correct. 👍 🗜