WIP: test logins update behaviour bumps usage stamps

CHANGELOG.md

@@ -1,5 +1,11 @@
 # v153.0 (In progress)
 
+## 🔧 What's Fixed 🔧
+
+### Logins
+
+- `update()` no longer bumps `times_used` or `time_last_used`, this is done only via `touch()`. Verified that both Firefox Android and iOS already track password use via `touch()` and call `update()` only for explicit edits. ([bug 2045032](https://bugzilla.mozilla.org/show_bug.cgi?id=2045032))
+
 ### Nimbus
 
 - Enrollment change events (visible to the UDL) now include the feature IDs of the features involved when possible. ([#7391](https://github.com/mozilla/application-services/pull/7391/))

components/logins/src/db.rs

@@ -552,6 +552,7 @@ impl LoginDb {
 
     fn update_existing_login(&self, login: &EncryptedLogin) -> Result<()> {
         // assumes the "local overlay" exists, so the guid must too.
+        let now_ms = util::system_time_ms_i64(SystemTime::now());
         let sql = format!(
             "UPDATE loginsL
              SET local_modified                           = :now_millis,
@@ -583,8 +584,7 @@ impl LoginDb {
                 ":time_password_changed": login.meta.time_password_changed,
                 ":sec_fields": login.sec_fields,
                 ":guid": &login.meta.id,
-                // time_last_used has been set to now.
-                ":now_millis": login.meta.time_last_used,
+                ":now_millis": now_ms,
             },
         )?;
         Ok(())
@@ -755,8 +755,9 @@ impl LoginDb {
                 id: existing.id,
                 time_created: existing.time_created,
                 time_password_changed,
-                time_last_used: now_ms,
-                times_used: existing.times_used + 1,
+                // An edit is not a use (see bug 2045032)
+                time_last_used: existing.time_last_used,
+                times_used: existing.times_used,
                 time_last_breach_alert_dismissed: None,
             },
             fields: LoginFields {
@@ -1956,6 +1957,46 @@ mod tests {
         assert_eq!(login2.meta.times_used, login.meta.times_used + 1);
     }
 
+    #[test]
+    fn test_update_does_not_count_as_use() {
+        // A plain update is not a password use.
+        // It must not bump `times_used` or `time_last_used`. Only `touch()` is
+        // allowed to do that.
+        ensure_initialized();
+        let db = LoginDb::open_in_memory();
+        let login = db
+            .add(
+                LoginEntry {
+                    origin: "https://www.example.com".into(),
+                    http_realm: Some("https://www.example.com".into()),
+                    username: "user1".into(),
+                    password: "password1".into(),
+                    ..Default::default()
+                },
+                &*TEST_ENCDEC,
+            )
+            .unwrap();
+        // Make sure the "now" an update would use differs from the add time.
+        thread::sleep(time::Duration::from_millis(50));
+        db.update(
+            &login.meta.id,
+            LoginEntry {
+                origin: "https://www.example.com".into(),
+                http_realm: Some("https://www.example.com".into()),
+                username: "user1".into(),
+                password: "password2".into(),
+                ..Default::default()
+            },
+            &*TEST_ENCDEC,
+        )
+        .unwrap();
+        let updated = db.get_by_id(&login.meta.id).unwrap().unwrap();
+        // An edit is not a use: times_used must stay unchanged.
+        assert_eq!(updated.meta.times_used, login.meta.times_used);
+        // An edit is not a use: time_last_used must stay unchanged.
+        assert_eq!(updated.meta.time_last_used, login.meta.time_last_used);
+    }
+
     #[test]
     fn test_breach_alert_dismissal() {
         ensure_initialized();

components/logins/src/store.rs

@@ -499,9 +499,9 @@ mod tests {
         assert!(b_after_update.time_created >= start_us);
         assert!(b_after_update.time_created <= now_us);
         assert!(b_after_update.time_password_changed >= now_us);
-        assert!(b_after_update.time_last_used >= now_us);
-        // Should be two even though we updated twice
-        assert_eq!(b_after_update.times_used, 2);
+        // An edit is not a use: usage stats are unchanged by update().
+        assert_eq!(b_after_update.time_last_used, b_from_db.time_last_used);
+        assert_eq!(b_after_update.times_used, 1);
     }
 
     #[test]