Closed
Bug 753175
Opened 12 years ago
Closed 12 years ago
Race condition in Sync Migration / Preference:Get
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox14 fixed, firefox15 fixed, blocking-fennec1.0 beta+)
RESOLVED
FIXED
Firefox 15
People
(Reporter: gcp, Assigned: Margaret)
Details
(Keywords: dataloss)
Attachments
(1 file)
4.11 KB,
patch
|
gcp
:
review+
johnath
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
https://bugzilla.mozilla.org/show_bug.cgi?id=752907#c9 Profile Migrator should not unregister its Preferences event listener until it actually received the preferences it asked for. It will currently get confused if someone else requests preferences at the same time. D/ProfileMigrator( 1410): Received event: Preferences:Data W/ProfileMigrator( 1410): Could not recover setting for = ui.zooming.animation_frames
Reporter | ||
Updated•12 years ago
|
Assignee: nobody → gpascutto
Comment 1•12 years ago
|
||
Nominating for blocking because this can prevent profile migration from finishing successfully.
blocking-fennec1.0: --- → ?
Keywords: dataloss
Assignee | ||
Comment 2•12 years ago
|
||
Assignee: gpascutto → margaret.leibovic
Attachment #622221 -
Flags: review?(gpascutto)
Assignee | ||
Comment 3•12 years ago
|
||
I tested this patch with the STR from bug 752907 comment 0, and it indeed fixes the sync migration problem.
Reporter | ||
Comment 4•12 years ago
|
||
Comment on attachment 622221 [details] [diff] [review] patch Review of attachment 622221 [details] [diff] [review]: ----------------------------------------------------------------- Our Preferences:Get design won't hold up in the long run. If you would fire a request for only one of the preferences elsewhere, we'd still misfire and think this was a reply to our request. We'll have to keep this in mind for the longer term. ::: mobile/android/base/ProfileMigrator.java @@ +456,5 @@ > JSONObject jPref = jsonPrefs.getJSONObject(i); > final String prefName = jPref.getString("name"); > + > + // Check to make sure we're working with preferences we requested. > + if (!Arrays.asList(SYNC_SETTINGS_LIST).contains(prefName)) I think this is guaranteed to be in mSyncSettingsList already.
Attachment #622221 -
Flags: review?(gpascutto) → review+
Comment 5•12 years ago
|
||
> Our Preferences:Get design won't hold up in the long run. If you would fire > a request for only one of the preferences elsewhere, we'd still misfire and > think this was a reply to our request. We'll have to keep this in mind for > the longer term. Yeah, I ran into this problem when adding prefs to GeckoLayerClient and PanZoomController. I've filed bug 753312 for it.
Reporter | ||
Comment 6•12 years ago
|
||
Comment on attachment 622221 [details] [diff] [review] patch [Approval Request Comment] User impact if declined: Sync will not migrate successfully dependent on timing.
Attachment #622221 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 7•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/228971745c99
Target Milestone: --- → Firefox 15
Updated•12 years ago
|
Attachment #622221 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 8•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/1ad62534690f
status-firefox14:
--- → fixed
status-firefox15:
--- → fixed
Updated•12 years ago
|
blocking-fennec1.0: ? → beta+
Comment 9•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/228971745c99
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•