1801057 - Remove any code referring to moz_items_annos or the "sync/parent" annotation

Status: RESOLVED FIXED

(Toolkit :: Places, task, P5)

Description

[Marco Bonardo [:mak]]

I noticed the "sync/parent" item annotation is no more used by any "live" code, and also references to moz_items_annos are useless. We should keep the empty table around though, just in case of a downgrade.

Comment 1

[G Jeevika]

Hey! I am an Outreachy applicant and I would like to work on this bug. Can someone guide me?
Thank you :)

Comment 2

[Marco Bonardo [:mak]]

Sure. First I'd suggest to fix this in small pieces, possibly one changeset per each source+test pair.
For example changing Bookmarks.sys.mjs, and then running the tests and modify the affected ones. This is one changeset.
In a separate changeset do PlacesDBUtils.sys.mjs, and similarly any affected tests.
And so on. By splitting into small parts we can do this incrementally and land parts that are ready.

All the references can be found here:
https://searchfox.org/mozilla-central/search?q=moz_items_anno
https://searchfox.org/mozilla-central/search?q=sync%252Fparent

The scope here is to remove any code that is managing entries in the moz_items_annos table, because we don't use those anymore, though there are a few exceptions:

• the code in Database.cpp, nsPlacesTables.h, nsPlacesIndexes.h should remain for now, because we must take care of downgrade paths, so we'll handle it apart
• some code related to the "mobile root" may be complicate to remove

Overall I'd concentrate on: Bookmarks.sys.mjs, PlacesDBUtils.sys.mjs, PlacesSyncUtils.sys.mjs, store.rs
Most tests can be executed using ./mach xpcshell-test toolkit/components/places
For all the changes you can use an artifact build, that is quite fast.

There's documentation on how to build Firefox in https://firefox-source-docs.mozilla.org/setup/index.html

Comment 3

[G Jeevika]

Hey!
I looked into the code and I have some general doubts regarding the tests and the moz_items_annos table. Can you please suggest which is the best place to get them cleared - here or the IRC channels?
Thank you!

Comment 4

[G Jeevika]

Attachment: Bug 1801057 - removed code as needed in Bookmarks.sys.mjs and PlacesDBUtils.sys.mjs and ran xpcshell tests. r?mak

All 250 tests were passed.

Comment 5

[G Jeevika]

Attachment: Bug 1801057 - removed code referencing moz_items_annos. r?mak

Comment 6

[G Jeevika]

Attachment: Bug 1801057 - removed code referencing moz_items_annos in store.rs. r?mak

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/171283fee687
removed code as needed in Bookmarks.sys.mjs and PlacesDBUtils.sys.mjs and ran xpcshell tests. r=mak
https://hg.mozilla.org/integration/autoland/rev/b91a76e31e8e
removed code referencing moz_items_annos. r=mak
https://hg.mozilla.org/integration/autoland/rev/5484bfb806a1
removed code referencing moz_items_annos in store.rs. r=mak,markh

Comment 8

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/171283fee687
https://hg.mozilla.org/mozilla-central/rev/b91a76e31e8e
https://hg.mozilla.org/mozilla-central/rev/5484bfb806a1