860479 - mozIAsyncHistory: usage pattern for getting all visited or not visited links out of a collection of links

Status: RESOLVED WONTFIX

(Firefox :: Bookmarks & History, enhancement)

Description

[u130342]

I have a private addin which can do things for a set of links (from a document, clipboard, all tabs, all links of all open document links).
These links/URI's can be filtered for being visited or not visited and then performing an action per filtered link.
On common use case is to open links that weren't visited before, or to copy them as text to the clipboard.

With nsIGlobalHistory.isVisited this is a simple loop over the objects containing the uri and create a new list for the filtered objects.

According to FF21 docs (https://developer.mozilla.org/en-US/docs/Firefox_21_for_developers) this function will be removed.

Therefore I was looking at mozIAsyncHistory.isURIVisited but it makes things really complicated.

Processing can start only after all links are inspected, but there is only one callback per link, therefore I would need to have a decreasing counter, care for threading issues if (e.g.) this.countdown-- isn't threadsafe or atomic.
If any callback doesn't arrive, the result is that nothing at all will happen.
Also I only have a backlink to the object containing the uri if I implement isVisited:function(aURI, aVisitedStatus) there, making counting down more difficult.

BTW: isVisited and isURIVisited are not good names, but thats my personal opinion ;). isVisited -> visitedStateCallback, isURIVisited -> queryURIVisited 

My suggestion's:

A function which is similar to updatePlaces .. first parameter is a list of objects implementing mozIVisitedStatusCallback2 (follows), and a second parameter for allProcessedCallback (e.g nsIObserver  or like mozIVisitInfoCallback.handleCompletion.

mozIVisitedStatusCallback2:
  readonly attribute nsIURI URI;  // the uri to check
  void updateVisitedState(boolean aVisitedStatus); // function to update visit state
  // attribute bool boolean isVisited;   // alternative an attribute or getter/setter

Not choosing isVisited would allow to have a property or getter which reveals the visied state in allProcessedCallback, isVisited is the most natural name for this IMO.

Comment 1

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

(In reply to Harald from comment #0)
> Processing can start only after all links are inspected, but there is only
> one callback per link, therefore I would need to have a decreasing counter,
> care for threading issues if (e.g.) this.countdown-- isn't threadsafe or
> atomic.

Callbacks are on main-thread, everything coming from APIs are on main-thread, in JS you can hardly have threadsafety issues, at a maximum you may have ordering issues if you use workers.

> If any callback doesn't arrive, the result is that nothing at all will
> happen.

All callbacks are supposed to arrive, otherwise there's a bug to be fixed.

> A function which is similar to updatePlaces .. 

We currently creating GetPlacesInfo API in bug 834539, I suppose we may add visited information to the results.

Btw, as a side note, you should look at the various libraries existing for JS to simplify and streamline async behavior, for example using Task.jsm and promises you could easily wrap isURIVisited and keep working with a common for loop.

Confirming as anh request for now, we'll evaluate the use-case.

Comment 2

[Mark Banner (:standard8)]

Legacy add-ons are no longer supported, and the APIs have been changing lots. If something like this is still required, please file a WebExtension API request.