Closed Bug 1468972 Opened 2 years ago Closed 2 years ago

Move page annotations to the async History API


(Toolkit :: Places, enhancement, P3)






(Reporter: mak, Unassigned)


(Blocks 1 open bug)


(Whiteboard: [fxsearch])

I made a plan for the conversion of current page annotation consumers.
Currently we have 2 consumers:
* charset: one important thing to do here is store a charset only if it's not UTF-8. Since we default to UTF-8 it doesn't make sense to store it.
* downloads info: in the future we could move this to a new store, for now we can retain it as-is, but we need to fix bug 1387446. We also need to change the code adding the annotation (through nsIDownloadHistory) to js (TBD).

The plan involves maintaining the current tables structure: moz_anno_attributes and moz_annos. Though, we'll write and read them with queries from History.jsm

In particular, we'll empower update() and fetch() to fetch annotations:
update(..., annotations: [{ key: value }]. Setting a null/empty value will remove the anno.
fetch can grow a new includeAnnotations option and will return an array of annotations.

For notifications, we can use onPageChanged and add a new attribute for annotations changes. There is probably only one usage of the page annotations observer for history views (downloads) that can be easily converted.

This should allow us to completely remove the page annotations XPCOM APIs.
Summary: Moe page annotations to the History async API → Move page annotations to the async History API
I'm looking with Paolo at ways to convert the current nsIDownloadHistory.
It has 2 APIs, addDownload and removeAllDownloads. The latter is only used by js, so we can add it to History.jsm.
The former is used by nsExternalHelperAppService.cpp, but that is unnecessary, indeed DownloadCore.js is checking "aAlreadyAddedToHistory". By removing those historical-reasons checks, we can remove the cpp call.

Finally, there's a second implementation of nsIDownloadHistory in the docshell, but that is apparently unused, indeed both Thunderbird and Seamonkey use Places, and Android has its own thing. It can be removed.
Depends on: 1468980
No longer blocks: 1462135
Depends on: 1462135
Depends on: 1479722
Depends on: 1480049
This has been fixed by all the dependent bugs.
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.