I noticed that several times I've been left with partial bookmarks or partial history in my main Firefox profile. The only way this can happen is if two things occur: 1. The database is moved/recreated. 2. The Sync timestamps are not reset. This used to be impossible: in the course of initializing the profile we would create a BrowserSyncManager, and it would listen to reset notifications and do the right thing. These days BrowserProfile.init is eager: it eagerly creates BrowserDB (line 243), which tries to create the schema table, which can trigger the database replacement logic. BrowserSyncManager observes onDatabaseWasRecreated… but BrowserSyncManager takes a Profile as an init argument. If the database is moved out of the way during the init of BrowserProfile, there will be no BrowserSyncManager listening for the database recreation logic, and we will be left with partial Sync data. The fix is to somehow ensure that the correct Sync-related code runs if the DB needs to be recovered: perhaps via a flag in Profile, or by rearranging the init of BrowserSyncManager. It's also necessary to detect this state and recover — perhaps by checking for a .bak file _once_ and re-syncing the appropriate database.
Regression from https://github.com/mozilla-mobile/firefox-ios/pull/2419: https://github.com/mozilla-mobile/firefox-ios/commit/dae882fbadd4e467f7a6e4f04007528931fffddc#diff-7a45ce0d70d55bdf297fcacd47e4e874R259
status-fxios-v7.0: --- → ?
status-fxios-v7.1: --- → ?
status-fxios-v7.2: --- → ?
status-fxios-v7.3: --- → ?
status-fxios-v7.4: --- → ?
status-fxios-v7.5: --- → affected
status-fxios-v7.6: --- → affected
status-fxios-v8.0: --- → affected
status-fxios-v8.1: --- → affected
tracking-fxios: ? → 8.0+
Created attachment 8883737 [details] [review] PR https://github.com/mozilla-mobile/firefox-ios/pull/2873
Assignee: nobody → sarentz
Attachment #8883737 - Flags: review?(jdarcangelo)
Unclear what the risk is of landing this in 8.0 so late. Maybe this should move to 8.1?
Priority: -- → P1
(In reply to Stefan Arentz [:st3fan] from comment #3) > Unclear what the risk is of landing this in 8.0 so late. Maybe this should > move to 8.1? The risk of the change itself to everyday users is relatively low. The larger risk, which I have neither examined through inspection nor tested, is whether the reset logic still works -- it's possible that this bug means it has never run since the Swift 3 migration. It's unlikely to be worse than the current situation, which essentially requires signing out of Sync and signing back in to get your data back. The conservative approach is to ship 8.0 and let this ride the trains. My hope is that justindarc is taking a holistic view of this and related bugs.
Attachment #8883737 - Flags: review?(jdarcangelo) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
Uplifted to v8.x
Whiteboard: [MobileCore][needsuplift] → [MobileCore]
You need to log in before you can comment on or make changes to this bug.