435320 - Add tracking support to PasswordEngine

Status: RESOLVED DUPLICATE of bug 468697

(Cloud Services :: General, defect, P2)

Description

[Anant Narayanan [:anant]]

PasswordEngine needs tracking support. See weave/modules/trackers.js.

Comment 1

[Anant Narayanan [:anant]]

Attachment: Add tracking support for PasswordEngine

We query the loginManager for the number of logins stored, and record every change as worth 15 points.

Comment 2

[Dan Mills [:thunder]]

Comment on attachment 322279 [details] [diff] [review]
Add tracking support for PasswordEngine


>+   * FIXME!: Buggy, because changes in a NSLoginInfo doesn't
>+   * result in an increment of our score.

We can't know that w/o a master password, right?  Is there a way to know whether a) the db has been unlocked, or b) there is no master password set at all? (that is, if we would cause the master password prompt to appear).  If either of those are true, we could dig a little deeper.

>+    this._score = 0;

Do you actually need to set this._score?  You just generate it in score() and return it immediately anyway, it seems superfluous to set it to an instance property and clear it in _init/reset.

r- pending tracking of edits.

Comment 3

[Justin Dolske [:Dolske]]

Checking for a master password isn't quite the right thing to do, because there could be different backends which don't use the MP. It might be better to use the shim from bug 433762 instead. That can give you explicit notifications of when logins are added/removed/modified.

Comment 4

[Dan Mills [:thunder]]

The password engine has a tracker now.  Should this bug be closed?

Comment 5

[Anant Narayanan [:anant]]

Does it? I don't see it in modules/engines/passwords.js...

Comment 6

[Anant Narayanan [:anant]]

As a compromise we can push the patch in attachment #322279 [details] [diff] [review] in, and punt the tracking of password edits (using the shim mentioned in comment #3) to 0.3.

Comment 7

[Dan Mills [:thunder]]

Let's add the tracker in the attachment, with one mod:  have it artificially add to the score (say, +30), to ensure edits will sync at some point.

Comment 8

[Dan Mills [:thunder]]

Plus the removal of this._score, per comment #2.

Comment 9

[Anant Narayanan [:anant]]

Checked-in http://hg.mozilla.org/labs/weave/index.cgi/rev/423809ba3ef9

Let's keep the bug open, but change the milestone to 0.3, so we eventually get the shim to track password edits as well.

Comment 10

[Dan Mills [:thunder]]

Small bug: you should add 30 before normalizing to 100.  The committed version could return up to 129.

Comment 11

[Anant Narayanan [:anant]]

Thanks for the catch, fixed http://hg.mozilla.org/labs/weave/index.cgi/rev/ab462bd0b41f

Comment 12

[Dan Mills [:thunder]]

These bugs need to be triaged, removing 0.3 milestone setting.

Comment 13

[Anant Narayanan [:anant]]

We're tracking this in 468697