1098501 - Track password changes to enable three-way merges

Status: RESOLVED INCOMPLETE

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

Description

[Richard Newman [:rnewman]]

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.

Comment 1

[Nick Alexander :nalexander [he/him]]

(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.

Comment 2

[Richard Newman [:rnewman]]

(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.

Comment 3

[Richard Newman [:rnewman]]

(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.

Comment 4

[Firefox Bug Husbandry Bot]

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.

Comment 5

[BMO Automation]

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.