Closed Bug 1098501 Opened 10 years ago Closed 4 years ago

Track password changes to enable three-way merges

Categories

(Firefox for Android Graveyard :: Android Sync, defect, P5)

defect

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: rnewman, Unassigned, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [sync:passwords][hard])

This is the Android counterpart of Bug 720592. If we're to sync more fields in records, particularly a shift from "every field is important" to "some fields are critical, others are nice to have", then we need to enable a finer-grained pseudo-three-way-merge on each platform for the affected formats. For passwords, which is the first avenue we're attacking, that'll involve tracking a sequence of before-after pairs for each change. There are a few ways we could go about that: * Writing a changelog to disk from Gecko * Writing a changelog into the PasswordsProvider via the bridge or JNI * Store a shadow copy of the last synced record set each time we finish a sync (the highest fidelity approach -- we really would store the shared ancestor!) * Implement Bug 946857, and have the changelog recorded directly from Java.
(In reply to Richard Newman [:rnewman] from comment #0) > This is the Android counterpart of Bug 720592. > > If we're to sync more fields in records, particularly a shift from "every > field is important" to "some fields are critical, others are nice to have", > then we need to enable a finer-grained pseudo-three-way-merge on each > platform for the affected formats. > > For passwords, which is the first avenue we're attacking, that'll involve > tracking a sequence of before-after pairs for each change. > > There are a few ways we could go about that: > > * Writing a changelog to disk from Gecko > * Writing a changelog into the PasswordsProvider via the bridge or JNI > * Store a shadow copy of the last synced record set each time we finish a > sync (the highest fidelity approach -- we really would store the shared > ancestor!) > * Implement Bug 946857, and have the changelog recorded directly from Java. With regards to Bug 946857, I looked at nsIStorageLogingManager and it looks at least technically possible to implement in Java (and then wrap in C++ or JS). I don't think we get much by doing this. Will we ever care to have multiple storage backends? (Would a GeckoView consumer want to implement this storage layer? I doubt it.) And if we take this approach, we still have to implement a changelog recorder. In general, I'm skeptical about recording changelogs. I worry that the complexity of maintaining a changelog leads to correctness problems. Also, it is a nice property of the current system that we can do stuff to the DB -- including remove it entirely -- without much consideration for whether things are consistent. So my initial reaction is that storing a shadow copy of the last synced record is the best option. Some thoughts: * Pro: Passwords is a small DB -- for most users probably fewer than 100 records. So maintaining a shadow copy is not space prohibitive. * Pro: We can isolate the shadow copy and it's management entirely to the Sync and Background Services sub-systems. This is a big win, IMO. This looks very much like a "second upload" to our local shadow copy. * Con: We push a systemic problem into individual engines. You worried (rightly!) about this in https://ebugzilla.mozilla.org/show_bug.cgi?id=720592#c5.
(In reply to Nick Alexander :nalexander from comment #1) > I don't think we get much by doing this. The motivation for that bug isn't really related to this bug: we'd end up with no separate process, no need to load NSS and do awful things in order to load the same DB, and we'd have some options around master password. But it'd make it a little simpler to have the storage layer in Java when attacking this problem in certain ways.
(In reply to Nick Alexander :nalexander from comment #1) > In general, I'm skeptical about recording changelogs. I worry that the > complexity of maintaining a changelog leads to correctness problems. Also, > it is a nice property of the current system that we can do stuff to the DB -- > including remove it entirely -- without much consideration for whether things > are consistent. This is the approach that's easier if we 'own' LMS on the Java side: each storage operation can generate another link in the chain, and Sync can dip into this.
Assignee: rnewman → nobody
Mentor: rnewman
Status: ASSIGNED → NEW
Depends on: 946857
Whiteboard: [sync:passwords] → [sync:passwords][hard]
Priority: -- → P3
Product: Android Background Services → Firefox for Android
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195 Needinfo :susheel if you think this bug should be re-triaged.
Priority: P3 → P5
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.