Bugfix FXIOS-9667 Reduce latency loading favicons

[ih-codes]

:scroll: Tickets

Jira ticket 1 Github issue 1

Jira ticket 2 Github issue 2

:bulb: Description

Two tickets linked, because the favicon rework solves the bug in ticket 1.

Context for the favicon rework: On the very first launch of the app, the home page top sites (in the pinned area) show the 5 DefaultSuggestedSites, the Google tile, and any sponsored tiles by default.

While the app bundled favicons with the intention of reducing load times of fetching and showing favicons for each of these top sites, they were not being used as expected. (bug) This meant the first launch was slow to show the favicons in the home page tiles. Moreover, several of the bundled favicons were very outdated anyway.

The consequence is that the app would fire off a network request for each website to scrape the faviconURL, which would then be used to make an image request.

The app caches the URLs and images to reduce load times on future launches.

Main changes:

• Removed all the bundled favicon assets, many of which were outdated
• Removed code and tests for loading these bundled assets (no longer necessary)
• Hard coded DefaultSuggestedSites faviconURLs to reduce latency looking up the top sites favicon URLs on first app launch

The bugfix ensures that the hard coded favicon URL is used for the top suggested sites. This means requests do not have to be made to each of the DefaultSuggestedSites to scrape the favicon URLs from the metadata (these requests took around ~400ms on my internet).

:pencil: Checklist

You have to check all boxes before merging

• [x] Filled in the above information (tickets numbers and description of your work)
• [x] 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)

[ih-codes]

@OrlaM I just wanted to make a quick draft PR since you'll be away on PTO next week.

I think most of the changes are here, but I'm still doing some testing.

I also am talking to @mattreaganmozilla about the tab metadata providing the wrong faviconURL in some cases (e.g. user goes to gmail.com, which redirects to accounts.google.com/v3/signin/identifier?..., and a bad faviconURL is returned from the JavaScript evaluation). Whereas the fetchFaviconURL method comes up with a correct guess to the default favicon.ico link. Still investigating, but might be a race condition which wins out. Seems like this didn't occur before because of the bundled assets.

[ih-codes]

I also am talking to @mattreaganmozilla about the tab metadata providing the wrong faviconURL in some cases (e.g. user goes to gmail.com, which redirects to accounts.google.com/v3/signin/identifier?..., and a bad faviconURL is returned from the JavaScript evaluation). Whereas the fetchFaviconURL method comes up with a correct guess to the default favicon.ico link. Still investigating, but might be a race condition which wins out. Seems like this didn't occur before because of the bundled assets.

Matt created a ticket for this.

[github-actions[bot]]

This PR has been automatically marked as stale. Please leave any comment to keep this PR opened. It will be closed automatically if no further update occurs in the next 7 days. Thank you for your contributions!

[ih-codes]

I also am talking to @mattreaganmozilla about the tab metadata providing the wrong faviconURL in some cases (e.g. user goes to gmail.com, which redirects to accounts.google.com/v3/signin/identifier?..., and a bad faviconURL is returned from the JavaScript evaluation). Whereas the fetchFaviconURL method comes up with a correct guess to the default favicon.ico link. Still investigating, but might be a race condition which wins out. Seems like this didn't occur before because of the bundled assets.

Matt created a ticket for this.

I'm hesitant to merge this until this issue has been addressed, because it could make the problem look worse.

[mobiletest-ci-bot]

	Warnings
:warning:	Pull Request size seems relatively large. If this Pull Request contains multiple changes, please split each into separate PR will helps faster, easier review. Consider using epic branches for work that would affect main.

	Messages
:book:	Project coverage: 31.65%
:book:	Edited 14 files
:book:	Created 0 files

Client.app: Coverage: 30.36

File	Coverage
PinnedSite.swift	100.0%	✅
TopSitesProvider.swift	98.03%	✅
Tab.swift	38.15%	⚠️
GoogleTopSiteManager.swift	83.05%	✅
TopSiteItemCell.swift	0.0%	⚠️

libStorage.a: Coverage: 56.46

File	Coverage
SuggestedSites.swift	93.55%	✅

Generated by :no_entry_sign: Danger Swift against 958e3898c68b449625379d7523e2f6a18e2fa053

[mattreaganmozilla]

I also am talking to @mattreaganmozilla about the tab metadata providing the wrong faviconURL in some cases (e.g. user goes to gmail.com, which redirects to accounts.google.com/v3/signin/identifier?..., and a bad faviconURL is returned from the JavaScript evaluation). Whereas the fetchFaviconURL method comes up with a correct guess to the default favicon.ico link. Still investigating, but might be a race condition which wins out. Seems like this didn't occur before because of the bundled assets. Matt created a ticket for this.

I'm hesitant to merge this until this issue has been addressed, because it could make the problem look worse.

Let me know if you have a minute to sync on this. I still need to revisit that ticket, it's on my radar but I just haven't had time yet to get back to it but I can try to prioritize it if it's blocking this work. It's possible that we might just need to apply some kind of bandaid temporarily (like directly examining the URL the JS spits out to see if the .ico is in a subdirectory incorrectly).

[ih-codes]

Ok, the aforementioned ticket that was holding this up has been "resolved". Thanks @mattreaganmozilla !

@OrlaM You reviewed this already before mozweek, but just FYI there are two comments awaiting your input (no rush though, I know you're still catching up on a bunch of stuff).

• https://github.com/mozilla-mobile/firefox-ios/pull/21405/files#r1727507576
• https://github.com/mozilla-mobile/firefox-ios/pull/21405/files#r1702088030