1815306 - Tabs Tray can hang when page titles are abnormally long.

Status: VERIFIED FIXED

(Firefox for Android :: Tabs, defect)

Description

[Noah Bond [:007]]

Steps to reproduce

1. Have 30+ tabs open
2. Scroll up and down through the tabs tray multiple times

Expected behavior

Tabs Tray scrolling should be smooth

Actual behavior

Tabs Tray scrolling is sluggish and occasionally crashes the app without notice or the "app has crashed" dialog.

Device information

• Firefox version: Nightly
• Android device model: Pixel 2
• Android OS version: 9

Any additional information?

From reporter:
"I have issues with Firefox Android tabs list that started ~ this weekend. It takes a few seconds for the tabs to be displayed (I have ~30 tabs open), and then scrolling in the list is slow, and may sometimes bring the app down (getting the system notification to kill the App). I updated this morning and this still happens. Is this a known issue?"

Initial investigation from [:007]
"So I was able to reproduce it, but the app closed with no app crash dialog nor any stack trace. I have 32ish tabs open and scrolling became a huge struggle bus for the app (I don’t think it matters between grid vs list tabs, but I reproduced it in the grid view). Interestingly though, it only became a fatal issue when those tabs were all recently opened. If it was a refreshed app kill->re-open, the app’s performance was functional, but still a bit sluggish. I also noticed that the sluggishness in the tray corresponded with the Favicons being loaded into the UI. A hypothesis I’m currently considering is it’s a memory issue of the tabs in-memory + the favicons coupled with some not-super-performant Favicon components all loading simultaneously. I didn’t immediately see anything in Sentry that could help."

Comment 1

[Jonathan Almeida [:jonalmeida]]

[Tracking Requested - why for this release]: this a large perf regression with STR that can lead to a crash as well.

Comment 2

[Donal Meehan [:dmeehan]]

:007 do you know what the regressor is?

Comment 3

[Joe M [:jmahon]]

Note for anyone trying to open a bunch of tabs to test this: the deep link

adb shell am start -a android.intent.action.VIEW -d 'fenix-dev://open?url=https://www.wikipedia.org/'

can help open tabs without having to poke through the UI manually.

Comment 4

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

The bug is marked as blocking firefox111 (nightly). We have limited time to fix this, the soft freeze is in 2 days. However, the bug still isn't assigned.

:cpeterson, could you please find an assignee for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Comment 5

[Donal Meehan [:dmeehan]]

:jonalmedia is this an S1 or should it be an S2?
https://wiki.mozilla.org/BMO/UserGuide/BugFields#bug_severity

Comment 6

[Jonathan Almeida [:jonalmeida]]

It's on the lower end of S1 because it may affect a variety of users with no suitable workarounds. The current assumption is that it is related to long tab title lengths.

Comment 7

[Matt Tighe [:matt-tighe]]

I wrote a small script to open a bunch of tabs easily if anyone finds that helpful:

#!/bin/bash

for x in {1950..1990}
do
    echo $x
    link="https://en.wikipedia.org/wiki/$x"
    adb shell am start -a "android.intent.action.VIEW" -d $link -n "org.mozilla.fenix/org.mozilla.fenix.IntentReceiverActivity"
    sleep 1
done

Additionally, it was later reported that the issue seems to actually stem from URLs with very long titles. I'm attaching the example static html that the original reporter generated to the top-level post.

Comment 8

[Matt Tighe [:matt-tighe]]

Attachment: long-title.html

Comment 9

[Noah Bond [:007]]

After a few hours of triaging with Matt, we can pretty confidently say that this issue is due to tabs with long site titles. We noticed by using the this testing site with a long title the tabs tray would come to a crawl until the tabs tray was closed or that particular tab was killed. This issue also existed in 109 and is not a recent regression. Our recommendation would be to fix this either at the Android Components level (having some sort of "display title" parameter) or truncating the title text in Fenix, and ship the fix either in the next dot release or allow it to follow the trains.

The issue I was finding, where the entire tabs tray would become sluggish with 30+ tabs and soft crash, we feel, is an entirely separate performance issue with the Tabs Tray + some of the debug-only tab list items that were converted to Compose. This was only observed in local debug builds, as the Tabs Tray to Compose work is behind a debug-only feature flag. After doing some memory profiling, we found that the memory usage would skyrocket by simply scrolling up and down in the Tabs Tray, when most of the tabs had their site-preview Favicon loaded. We'll file a separate ticket to investigate that particular issue.

After this investigation, we feel that the severity and priority of this bug can be lowered to a S3 and P2, respectively, and the release can continue as planned.

Comment 10

[Matt Tighe [:matt-tighe]]

Since this is not a regression and will only affect users with page titles that are abnormally long, I am shifting the severity down as mentioned above so that it unblocks the release.

Comment 11

[Jonathan Almeida [:jonalmeida]]

(In reply to Noah Bond [:007] from comment #9)

Our recommendation would be to fix this either at the Android Components level (having some sort of "display title" parameter) or truncating the title text in Fenix, and ship the fix either in the next dot release or allow it to follow the trains.

The fix has to be in Fenix because the UI for the tabs tray exists there. We have multiple examples of truncating the URL like we do in the tabs tray code itself here for these exact kind of bugs. This fix we need to add is a few lines above this example.

I'm going to fix this now because:

1. The fix is a (safe) one-liner win - we have multiple examples of fix in our code base.
2. We can avoid wasting the round trip time and resources of a dot release.
3. This bug can be used to DDOS our users, so leaving a known bug like this can be abused.
4. You are already working on a Compose version of the tabs tray where the other perf issues are limited to. Our users are only seeing the ones related to the long title from what I can read in your investigation.

The issue I was finding, where the entire tabs tray would become sluggish with 30+ tabs and soft crash, we feel, is an entirely separate performance issue with the Tabs Tray + some of the debug-only tab list items that were converted to Compose. This was only observed in local debug builds, as the Tabs Tray to Compose work is behind a debug-only feature flag. After doing some memory profiling, we found that the memory usage would skyrocket by simply scrolling up and down in the Tabs Tray, when most of the tabs had their site-preview Favicon loaded. We'll file a separate ticket to investigate that particular issue.

I filed bug 1815620 for this because its a perm-crash for me (and would probably be for other users) and the fix there is similar to the one that would go here.

Comment 12

[BMO Automation]

Attachment: [mozilla-mobile/fenix] Bug 1815306 - Truncate tabs tray title to mitigate scroll performance (#28792)

Comment 13

[Matt Tighe [:matt-tighe]]

(In reply to Jonathan Almeida [:jonalmeida] from comment #11)

Thanks for working on a fix Jonathan! We had assumed that since this was not a regression and we had already missed the RC build that it would be fine to handle these during subsequent normal work cycles. I'd be curious to learn more about a couple things so that I can better know how to handle prioritizations like these in the future.

2. We can avoid wasting the round trip time and resources of a dot release.

I'm guessing that this generally means respinning the RC is a less burdensome process and that is your plan for including a fix in 110. Is that correct?

3. This bug can be used to DDOS our users, so leaving a known bug like this can be abused.

Since it only impacts tabs tray performance, I'm not aware of the issues that this could cause that don't have a low effort/reward ratio. I would guess we probably want to take specifics offline, but I'd love to learn more about what the vector would look like here and any historical examples driving our caution around it.

Comment 14

[Jonathan Almeida [:jonalmeida]]

(In reply to Matt Tighe [:matt-tighe] from comment #13)

(In reply to Jonathan Almeida [:jonalmeida] from comment #11)

Thanks for working on a fix Jonathan! We had assumed that since this was not a regression and we had already missed the RC build that it would be fine to handle these during subsequent normal work cycles. I'd be curious to learn more about a couple things so that I can better know how to handle prioritizations like these in the future.

2. We can avoid wasting the round trip time and resources of a dot release.

I'm guessing that this generally means respinning the RC is a less burdensome process and that is your plan for including a fix in 110. Is that correct?

No, the intention was not to uplift anything. It was to ensure that the fix didn't need to land in 112 when you both did the hard work of debugging it already. 🙂

Sorry if that was unclear. I understood dot release to mean, beta dot release.

---

This has landed and is in nightly 111 be verified.

Comment 15

[Oana Horvath [:ohorvath - out of office]]

Hi, is this ready to be marked as Resolved Fixed? I want to make sure it gets on the QA's radar for verification.

Comment 16

[Noah Bond [:007]]

(In reply to Oana Horvath [:ohorvath] from comment #15)

Hi, is this ready to be marked as Resolved Fixed? I want to make sure it gets on the QA's radar for verification.

Hey, yes! I'll update the status of the ticket now

Comment 17

[Jonathan Almeida [:jonalmeida]]

Clearing NI.

Thanks Oana - I was leaving the bug open until qe-verify+ was done, but I've gotten my process wrong. :)

Comment 18

[Cristi Nechita]

Hi,
We've tested this issue on multiple devices and this issue it is no longer reproducible on the following devices:
-Samsung Galaxy A13 (SM-A135SF/DSN) & Android A12
-RealMe GT Master Edition & Android 11
-OnePlus 5 (A5000) & Android 10
-Motorola g6 (XT1925-5) & Android 8
-Huawei MediaPad T5 (AGS2-W09) & Android 8
-Huawei P9 Lite & Android 7
-Samsung Galaxy Tab S3 & Android 9
-Xiaomi MI 11 Lite & Android 11
-Samsung Galaxy Tab A7 & Android 12
-Oppo Find X5 & Android 13
-Google Pixel 7 & Android 13

Tested with latest build of Nightly: 111.0a1 (2023-02-13).
Marking this issue as Verified Fixed.
Thank you!

Comment 19

[Lorand Janos]

Verified on the latest Beta 111.0b2 build.
Devices used:

• OnePlus 5 (A5000) (Android 10).
• RealMe GT Master Edition (Android 11).
• Google Pixel 7 (Android 13).