[Regression] Background/Foreground the app quickly will result in no top sites, history, or bookmarks

RESOLVED FIXED

Status

()

Firefox for iOS
General
P1
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: sleroux, Assigned: sleroux)

Tracking

unspecified
Other
iOS

Firefox Tracking Flags

(fxios6.0+)

Details

(Whiteboard: [MobileCore])

Attachments

(2 attachments)

(Assignee)

Description

a year ago
Created attachment 8818891 [details]
IMG_3751.JPG

STR:

1. Launch Firefox
2. Sign into a FxA account with history/bookmarks
3. Open the HistoryPanel
4. Quickly background/foreground the app a couple of times
5. Navigate between the home panels

Expected:
All data should still appear (Top Sites/Bookmarks/History)

Actual:
Views are all blank and all the data is missing

-----

As a result of force closing the database on app background and reopening it on foreground, there seems to be a race condition where a DB close will happen after it tries to reopen causing our connection to the database to be nil which results in no data. I've attached a (crummy) drawing I made to help explain what is probably happening.

Essentially since we kick off a sync within a long running background task ID, if we open the app while this occurs before the sync finishes, the reopen command we schedule on the sharedConnectionQueue will be slotted behind the sync end's forceClose call.
(Assignee)

Comment 1

a year ago
Note: The diagram shows that the sync completion block occurs on the main thread which currently is not true. Even if we were to move this invocation to the main thread it wouldn't solve the issue.
(Assignee)

Comment 2

a year ago
So we actually already have code that checks to make sure that we only call shutdown if the application is in the foreground [1] . What appears to be happening is that we can run into the situation where the app becomes foregrounded then immediately after the upon block from the background sync is scheduled on the main thread on a cycle after reopen has already happened.

Since we're enqueuing the shutdown on the main thread, we need to be mindful of it potentially being called after appDidForeground - which makes the foreground check invalid.

[1] https://github.com/mozilla-mobile/firefox-ios/blob/master/Client/Application/AppDelegate.swift#L409
Assignee: nobody → sleroux
(Assignee)

Comment 3

a year ago
Created attachment 8818926 [details]
Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2282#attch-to-bugzilla
Attachment #8818926 - Flags: review?(bnicholson)
(Assignee)

Updated

a year ago
Status: NEW → ASSIGNED
Comment on attachment 8818926 [details]
Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/2282#attch-to-bugzilla

Makes sense to me.
Attachment #8818926 - Flags: review?(bnicholson) → review+
(Assignee)

Comment 5

a year ago
master https://github.com/mozilla-mobile/firefox-ios/commit/9dc8a9656f60ac318e9be33ddce7359103553ebb
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Whiteboard: [MobileCore] → [MobileCore][needsuplift]
(Assignee)

Comment 6

a year ago
v6.x 19f760b828c1493ba8457abad5c17221d35f80aa
Whiteboard: [MobileCore][needsuplift] → [MobileCore]
You need to log in before you can comment on or make changes to this bug.