1419197 - browser.history.search returns results past endTime

Status: RESOLVED INVALID

(WebExtensions :: General, defect)

Description

[Ian Bicking (:ianbicking)]

When using code like this, you can get results with .lastVisitTime > endTime:

history.search({
  text: "",
  startTime: 0,
  endTime: timestamp
});

An extension (manifest/background.js) demonstrating this is attached. See the Browser Console for its output: when you first hit the browserAction it will return 10 results. The second time it will attempt to fetch 10 older results, but many of the results will be more recent than would be expected given endTime.

Comment 1

[Ian Bicking (:ianbicking)]

Attachment: manifest.json

Comment 2

[Ian Bicking (:ianbicking)]

Attachment: background.js

Comment 3

[Bob Silverberg [:bsilverberg]]

I'll look into this.

Comment 4

[Bob Silverberg [:bsilverberg]]

I was able to reproduce this with one particular history entry. With an end date of 1511815178909, I did get an entry returned to me with a lastVisitTime of 1511815185490. I checked the numbers internally, and the end date of 1511815178909 gets turned into a PRTime of 1511815178909000, which gets passed into PlacesUtils.history.executeQuery(), and that returns a result set which includes a node with a time of 1511815185490693, which is indeed later than 1511815178909000.

Marco, do you know why passing an endTime of 1511815178909000 into PlacesUtils.history.executeQuery() would return an entry with a time of 1511815185490693 (i.e., later than the endTime)?

Comment 5

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

(In reply to Bob Silverberg [:bsilverberg] from comment #4)
> Marco, do you know why passing an endTime of 1511815178909000 into
> PlacesUtils.history.executeQuery() would return an entry with a time of
> 1511815185490693 (i.e., later than the endTime)?

When you query with a start and an end date, the query will return uris that have a visit in that timeframe, but the time property of each uri is the *absolute* last visit date for that uri. Indeed also the Library names that column "Most Recent Visit", it's not the most recent visit in the given timeframe, but the absolute most recent visit. If you look at the Library for "October", and show the Most Recent Visit column, you'll likely see visits with just the time and visits with time in November.

This happens because we unfortunately have views showing thousands of entries, and getting the real last visit time inside the queries timeframe would be one additional subquery repeated thousands of times, or it would require an extremely slow JOIN with the visits table. If in the future we'd move to a better filtering UI without infinitely long lists, then we could add the subquery to fetch a more expected value.

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

It just came to my mind that now (newer sqlite versions) it may be possible to rewrite the query using a WITH clause, so that we first pick all the place_ids with visits in the timeframe. Maybe even a plan join with a filtered visits query?
That needs some investigation (mostly perf measurement between the old and the new query) but sounds like a possibility.

Let me see if I can find a Places bug to link this to, otherwise I'll file a new one.

Comment 7

[Bob Silverberg [:bsilverberg]]

Thanks for the detailed explanation, Marco. While it might make sense in some contexts to change the query to return the last visited date within the query date range, I actually think the current behaviour makes sense and is what is expected from the API. The Chrome documentation [1] says "Searches the history for the last visit time of each page matching the query", which does suggest to me that it would return *pages* based on the query, but that each page would come with its own last visit time, which could be outside of the range of the query.

I tested in Chrome and it has the same behaviour as described in this bug, so we are consistent with Chrome and I think we're both doing what the API is documented to do. I'm therefore going to close this bug as invalid.

[1] https://developer.chrome.com/extensions/history#method-search