Closed Bug 1396923 Opened 2 years ago Closed 2 years ago

Race condition in FaviconFetcher

Categories

(Firefox for iOS :: Favicons, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
Iteration:
1.29
Tracking Status
fxios 10.0 ---

People

(Reporter: eoger, Assigned: farhan)

Details

(Whiteboard: [MobileCore])

Attachments

(1 file)

I believe that this is the reason some automated tests are failing, preventing us to merge some PRs (see #2911).

That test fails:
https://github.com/mozilla-mobile/firefox-ios/blob/79100c7730b84a0564453de0c435b62643856c21/ClientTests/TestFavicons.swift#L52

Because it is calling getDefaultIconForURL, which does a lookup in multiRegionDomains:
https://github.com/mozilla-mobile/firefox-ios/blob/98a9f16eab863896fc39ddd113d0671bd1b4524b/Client/Utils/FaviconFetcher.swift#L50

multiRegionDomains is populated by getDefaultIcons(), which is never called in that test case.
I guess the test passed before, because we ran tests by executing the app, so it was probably called at some point before the tests executed.
getDefaultIcons() is only called by defaultIcons (lazy var), which is referenced in getDefaultIconForURL().
That means the first lookup of a favicon, if it is for a "multi region domain", will always fail, not only in tests but also in the app.
Attached file Pull Request
Attachment #8904679 - Flags: review?(eoger)
Assignee: nobody → fpatel
Priority: -- → P1
master https://github.com/mozilla-mobile/firefox-ios/commit/3b029e4b9f71029f7dc4e024da9b6996fa8bd168
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Iteration: --- → 1.29
Whiteboard: [MobileCore]
Attachment #8904679 - Flags: review?(eoger) → review+
You need to log in before you can comment on or make changes to this bug.