901440 - Add a maintenance task to remove binary annotations

Status: RESOLVED FIXED

(Toolkit :: Places, defect, P2)

Description

[Marco Bonardo [:mak]]

we should remove all existing binary annotations in a maintenance task.

Comment 1

[Marco Bonardo [:mak]]

The idea is to add a task to PlacesDBUtils.jsm that removes any existing binary annotation, and add such task to the idle maintenance tasks

Comment 2

[MikeLing]

Hi~ I would like to work on this issue, but I only have some basic knowledge about JavaScript. Could you tell me more about this bug? Thank you very much! :)

Comment 3

[Marco Bonardo [:mak]]

Hi.
If you didn't yet, I suggest taking a look at these docs:
https://developer.mozilla.org/en-US/docs/Introduction
https://developer.mozilla.org/en-US/docs/Simple_Firefox_build
https://developer.mozilla.org/en-US/docs/Mercurial_FAQ
https://developer.mozilla.org/en-US/docs/Mercurial_Queues

Now, let's look at what happens here.
Basically some time ago we removed an API that allowed to add binary data to a page in places.sqlite database. Even if we removed the API, we never removed the data eventually created by it.

The code we use to do maintenance on the database lives in PlacesDBUtils.jsm, it runs a series of "tasks" to cleanup a database.
you can look into _getBoundCoherenceStatements method where there is a list of SQL statements executed to do those cleanups.
Here, before "A.3" (see the code comments) we need to add a statement that will remove the binary annotations from moz_annos and moz_items_annos tables.
These annotations can be recognized cause they have type = 4 (writing a SQL for this removal should be trivial, but feel free to ask if you have doubts, you can use SQLITE Manager add-on to test queries, just don't do on your default profile!)

Then we need to write a test for this. The tests are in toolkit/components/places/tests/unit/test_preventive_maintenance.js you can copy the sub test A.2  and do the appropriate modifications, that is: in setup() create a fake binary annotation (type = 4) in both moz_annos and moz_items_annos(), finally in check() verify it has been removed.

Comment 4

[MikeLing]

(In reply to Marco Bonardo [::mak] from comment #3)

Thank you for your trust! :)

Last time when I rebuild mozilla source code I meet gcc-version problem. The system I use is Ubuntu 12.04 and gcc on it is default 4.6.3 but build mozilla need 4.8.2 or above :(

I will try it again after I find way to update my gcc and I comment to let you know I had work on this issue. :)

Comment 5

[Marco Bonardo [:mak]]

You might look here for some useful information https://developer.mozilla.org/en-US/docs/Simple_Firefox_build/Linux_and_MacOS_build_preparation

You might also ask on irc.mozilla.org on the channels #introduction or #build

Comment 6

[MikeLing]

(In reply to Marco Bonardo [::mak] from comment #5)

Sorry for the late reply, I must say my computer is too old to compiler Mozilla source code, I had try a lot of times these days but still have problem :(

So, I decide to leave this bug to other people who capable to fix that. Sorry :(

Comment 7

[MikeLing]

Oops.....I try it again and it works. Please forget I comment in comment #6 and I will commit my first ASAP :-)

Comment 8

[MikeLing]

(In reply to Marco Bonardo [::mak] from comment #3)

> Here, before "A.3" (see the code comments) we need to add a statement that
> will remove the binary annotations from moz_annos and moz_items_annos tables.
> These annotations can be recognized cause they have type = 4 (writing a SQL
> for this removal should be trivial, but feel free to ask if you have doubts,
> you can use SQLITE Manager add-on to test queries, just don't do on your
> default profile!)

Hi~ I don't know exactly type = 4 means. In my point of view, I should add some lines in deleteObsoleteAnnos and deleteObsoleteItemsAnnos functions to delete type 4 annotations. Could you tell me some more informations about how to pick up type 4 annotations? Thanks :)

Comment 9

[Marco Bonardo [:mak]]

If you open places.sqlite database using Sqlite Manager add-on, and you look at the moz_items_annos table, you see there's a type column with values (1, 2, 3 and 4). This is what type = 4 means.
The query to add should be something like:
"DELETE FROM moz_items_annos WHERE type = 4"

You should add this query after deleteObsoleteItemsAnnos, the code will look similar.

Comment 10

[MikeLing]

Hi Marco,

There are a bunch of things on my schedule and I'm afraid I can't finish this issue. Sorry about that :( 
Please unassigned me from this bug. Thank you very much!

Comment 11

[Marco Bonardo [:mak]]

no worries, thank you for the notice.
The bug now has also more information for other contributors :)

Comment 12

[Kartikey Agrawal[:kartikey0303]]

Attachment: Added the lines to delete type 4

I tried to fix the bug. This is my first bug hopefully this helps.

Comment 13

[Marco Bonardo [:mak]]

Comment on attachment 8697873 [details] [diff] [review]
Added the lines to delete type 4

Review of attachment 8697873 [details] [diff] [review]:
-----------------------------------------------------------------

yep, it looks good, thanks

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/dadcbb0c8f3e

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/dadcbb0c8f3e