Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix FXIOS-9859 Don’t call apply theme for every tab #22746

Merged
merged 1 commit into from
Oct 24, 2024

Conversation

OrlaM
Copy link
Contributor

@OrlaM OrlaM commented Oct 24, 2024

📜 Tickets

Jira ticket
Github issue

💡 Description

There was an increase in hang rate some time around release v129. Since the problem area in calling apply theme my top suspect is this PR #20667.
The hang rate report from apple pre-dates 129 but it gets much worse in 129. Many of the logs accompanying the metric kit event for our beta builds in Sentry indicate the users experiencing this issue have a large number of tabs (> 100).
I think it's likely something in the refactor above is causing the apply theme to fire more often and since apply theme is triggered on every tab this has the potential to bog things down.

This PR triggers apply theme on just the selected PR as that is the only place it's really needed. The background of blank tabs get updated when they are selected.

📝 Checklist

You have to check all boxes before merging

  • Filled in the above information (tickets numbers and description of your work)
  • Updated the PR name to follow our PR naming guidelines
  • Wrote unit tests and/or ensured the tests suite is passing
  • When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
  • If needed, I updated documentation / comments for complex code and public methods
  • If needed, added a backport comment (example @Mergifyio backport release/v120)

@OrlaM OrlaM requested a review from a team as a code owner October 24, 2024 16:51
@mobiletest-ci-bot
Copy link

Messages
📖 Project coverage: 32.66%
📖 Edited 2 files
📖 Created 0 files

Client.app: Coverage: 30.51

File Coverage
BrowserViewController.swift 4.89% ⚠️
Tab.swift 44.7% ⚠️

Generated by 🚫 Danger Swift against ac92395

@OrlaM
Copy link
Contributor Author

OrlaM commented Oct 24, 2024

@Mergifyio backport release/v132

@OrlaM OrlaM merged commit 1193861 into main Oct 24, 2024
10 checks passed
@OrlaM OrlaM deleted the om/FXIOS-9859-apply-theme-hang branch October 24, 2024 19:51
Copy link
Contributor

mergify bot commented Oct 24, 2024

backport release/v132

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Oct 24, 2024
Don’t call apply theme for every tab

(cherry picked from commit 1193861)
rvandermeulen pushed a commit that referenced this pull request Nov 4, 2024
…) (#22750)

Bugfix FXIOS-9859 Don’t call apply theme for every tab (#22746)

Don’t call apply theme for every tab

(cherry picked from commit 1193861)

Co-authored-by: OrlaM <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants