1357896 - Bookmark's `modified` timestamp is older than its `created` timestamp

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: Data Providers, defect, P5)

Description

[:Grisha Kruglov]

Right after adding a bookmark, and looking at the db record, I see that modified is slightly older:

1492637568420 vs 1492637568419

Comment 1

[:Grisha Kruglov]

Attachment: Bug 1357896 - Let BrowserProvider handle bookmark timestamps on insertion

Otherwise we end up with modified<created, since we're generating 'modified' just before
BrowserProvider generates 'created'. BrowserProvider will handle these timestamps missing
from the bookmark ContentValues during an insertion.

Review commit: https://reviewboard.mozilla.org/r/133180/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/133180/

Comment 2

[Richard Newman [:rnewman]]

Comment on attachment 8861217 [details]
Bug 1357896 - Let BrowserProvider handle bookmark timestamps on insertion

https://reviewboard.mozilla.org/r/133180/#review136344

::: mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
(Diff revision 1)
>          }
>  
> +        // Timestamps (modified and created) will be set by BrowserProvider.
>          values.put(Bookmarks.URL, uri);
>          values.put(Bookmarks.PARENT, folderId);
> -        values.put(Bookmarks.DATE_MODIFIED, now);

Isn't a simpler solution to add one line?

```
values.put(Bookmarks.DATE_CREATED, now);
```

::: mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java:1183
(Diff revision 1)
>          debug("Bumping parent modified time for addition to: " + folderId);
>          final String where  = Bookmarks._ID + " = ?";
>          final String[] args = new String[] { String.valueOf(folderId) };
>  
>          ContentValues bumped = new ContentValues();
> -        bumped.put(Bookmarks.DATE_MODIFIED, now);
> +        bumped.put(Bookmarks.DATE_MODIFIED, System.currentTimeMillis());

The idea of using `now` was to have the modified parent and the child have the same modified time. That seems like a nice property to have: it ensures that if clock drift occurs then either both or neither of the two records will be retrieved from the database.

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8861217 [details]
Bug 1357896 - Let BrowserProvider handle bookmark timestamps on insertion

https://reviewboard.mozilla.org/r/133180/#review138388

Seems like Richard reviewd this already. Apart from that LGTM. :)

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.