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

ASSIGNED
Assigned to

Status

()

P5
normal
ASSIGNED
2 years ago
7 months ago

People

(Reporter: Grisha, Assigned: Grisha)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

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

1492637568420 vs 1492637568419
Comment hidden (mozreview-request)
(Assignee)

Updated

2 years ago
Assignee: nobody → gkruglov
Status: NEW → ASSIGNED
(Assignee)

Updated

2 years ago
Attachment #8861217 - Flags: review?(rnewman) → review?(s.kaspari)

Comment 2

2 years ago
mozreview-review
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

2 years ago
mozreview-review
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. :)
Attachment #8861217 - Flags: review?(s.kaspari) → review+
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.