Closed Bug 1377646 Opened 7 years ago Closed 7 years ago

Database recreation listener logic is wrong

Categories

(Firefox for iOS :: Data Storage, enhancement, P1)

All
iOS
enhancement

Tracking

()

RESOLVED FIXED
Tracking Status
fxios-v7.0 --- ?
fxios-v7.1 --- ?
fxios-v7.2 --- ?
fxios-v7.3 --- ?
fxios-v7.4 --- ?
fxios-v7.5 --- affected
fxios-v7.6 --- affected
fxios 8.0+ ---
fxios-v8.0 --- affected
fxios-v8.1 --- affected
fxios-v9.0 --- fixed

People

(Reporter: rnewman, Assigned: rnewman)

References

Details

(Whiteboard: [MobileCore])

Attachments

(1 file)

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.
See Also: → 1368719
Whiteboard: [MobileCore][needsuplift]
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?
Flags: needinfo?(rnewman)
Priority: -- → P1
Assignee: sarentz → rnewman
Status: NEW → ASSIGNED
(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.
Flags: needinfo?(rnewman)
Attachment #8883737 - Flags: review?(jdarcangelo) → review+
https://github.com/mozilla-mobile/firefox-ios/commit/64d03672a6364d30ac4daa5d6d56bb3d41cb09bf
Status: ASSIGNED → RESOLVED
Closed: 7 years 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.

Attachment

General

Created:
Updated:
Size: