Closed Bug 1484071 Opened 3 years ago Closed 2 years ago
Having a large data url in the Highlights causes terrible reflow performance in the parent process
After using the devtools' "Take a screenshot of the entire page" feature, I ended up with a data: url of the screenshot in the Highlights in about:newtab. The screenshot I had weights 1.1MB. When hovering that box of the highlights section, it takes more than a second for the mouse cursor to change back and forth between the arrow and the hand. Here is a profile of it: https://perfht.ml/2L4BLwk and I'm attaching a screencast. It's not clear to me why the jank is on the parent process rather than on the content process where about:newtab is loaded, but I think the fix is straight forward: when a base64 encoded data url is long, we should only put the beginning of it in the DOM. See bug 1408854 for a similar bug we already fixed.
I forgot to say this is on a current nightly. On release, the behavior I'm seeing matches the screenshot in bug 1459948, and it's the activity stream content process that gets blocked for 2s during the DisplayList step of the refresh driver tick.
Version: 54 Branch → Trunk
According to the profile, the time is spent in the _showDelayed function, which is related to the status panel that displays the hovered link. As seen on the screencast, the problem is that the status panel is taking a long time to render the giant data URL. The data URL is cropped in the middle, so it should be fairly straightforward to just take the first and last N characters to pass to display instead of trying to display the whole URL and ending up with it cropped anyway.
Component: Activity Streams: Newtab → Toolbars and Customization
Priority: -- → P3
Hi Mike, this bug fix seems straight forward and I believe is narrow enough in scope to take on as a newbie. It also seems well defined from comment 1 and an idea of a possible solution is given in comment 2. A fix would make the user experience feel more responsive by eliminating unnecessary lag. Would you say this is a GFB to take on? Thanks for guiding me away from my previous bug I wanted to fix that was too complex, not well defined, and too far reaching for my skill level.
Hi Grant, I think putting a hard limit on the URL going into the status bar makes sense. You're right - this bug seems much better as a good first bug than the last one. Felipe has signed himself up as a mentor on this, so I'll let him guide you through it. Thanks!
Flags: needinfo?(mconley) → needinfo?(felipc)
Hello Grant, I'll guide you on this bug. Have you already figured out things like pulling the source code with Mercurial and building Firefox? If not yet, I recommend that you take a look at something called Artifact builds (https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds) which will make building Firefox very fast (since for this bug, and most of the front-end, we don't need to make C++ changes) If you already have gotten all that ready, here's where to start fixing this bug: according to comment 2, the starting point of the problem is the _showDelayed function, from here: https://searchfox.org/mozilla-central/rev/3d989d097fa35afe19e814c6d5bc2f2bf10867e1/browser/base/content/browser.js#5161 that function calls StatusPanel.update, which is implemented here: https://searchfox.org/mozilla-central/rev/3d989d097fa35afe19e814c6d5bc2f2bf10867e1/browser/base/content/tabbrowser.js#5141 and it's likely the place where you'll need to make the fix. (btw I highly recommend using https://searchfox.org/ to search and navigate the source code)
Assignee: nobody → grant.hj.wong
Status: NEW → ASSIGNED
Hi Felipe, I've pulled and built FireFox so I can readily begin implementing the fix. Thank you for taking on the task of guiding me through this bug and I'll update you when I have a patch ready for review.
Hi Grant, are you still working on this?
Whiteboard: [fxperf:p2] → [fxperf:p3]
Hi Mike, I won't be contributing a fix to this bug.
Hi Grant, okay, thanks for letting us know!
Assignee: grant.hj.wong → nobody
Status: ASSIGNED → NEW
Assignee: nobody → nikkisharpley
Status: NEW → ASSIGNED
Attachment #9047504 - Attachment description: status panel: decrease time to render larger data urls (bug 1484071) → Crop large data URLs before displaying them in the status bar
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/6890499eb5d1 Crop large data URLs before displaying them in the status bar r=Felipe
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/f25fac04788d Crop large data URLs before displaying them in the status bar r=Felipe
You need to log in before you can comment on or make changes to this bug.