Get the Places database working without using DataStore

RESOLVED FIXED

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: benfrancis, Assigned: benfrancis)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Transition])

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
As part of the B2G OS Transition Project we're removing the DataStore API. We'll need to get the Places database working without using that API. This bug is to track that work.
(Assignee)

Updated

3 years ago
Blocks: 1260447
No longer blocks: 1252143
(Assignee)

Updated

3 years ago
Blocks: 1261027
(Assignee)

Updated

3 years ago
Component: Gaia::Shared → Gaia::System
(Assignee)

Updated

3 years ago
Component: Gaia::System → Gaia::System::Browser Chrome
Hey Ben, any progress here? DO you need any help?
Flags: needinfo?(bfrancis)
(Assignee)

Comment 2

3 years ago
Hi Alberto, yes I made some progress on this yesterday. Let me work on it a bit more today and try to get a WIP patch up and you can see what you think about the direction.
Flags: needinfo?(bfrancis)
(Assignee)

Comment 3

3 years ago
Posted file WIP places database (obsolete) —
Hi Alberto,

As the Places and Bookmarks data stores are now useless, I'm thinking of replacing them both with a new places IndexedDB database which has stores for sites, pages, visits and icons.

I have a WIP patch with the first piece of code which just allows you to pin a site in the database.

Do you think this is the right direction?
Attachment #8741879 - Flags: feedback?(apastor)
Comment on attachment 8741879 [details] [review]
WIP places database

Great! I think using indexedDB is the way to go, but shouldn't we keep all the places.js logic in place, and just replace the store.get/put with the indexedDB equivalent? That way everything should work as it was working before, isn't it?
Attachment #8741879 - Flags: feedback?(apastor) → feedback+
(Assignee)

Updated

3 years ago
Depends on: 1266125
(Assignee)

Comment 6

3 years ago
Comment on attachment 8743292 [details] [review]
[gaia] albertopq:places-replacement > mozilla-b2g:kanikani

Oh, I totally missed this when I added the patch on bug 1266125.


My plan was to have the following stores in the Places database:
* sites - to store pinned sites (and possibly all visited sites in future) and and replace the bookmarks DataStore
* pages - to store all visited and pinned pages and replace the places store
* visits - possibly store visits separately to make it easier to generate the history list?
* icons - a central icon cache for sites and pages keyed by icon URL

Your patch uses the "sites" store to store "pages".

Because the changes are fairly major and the current code is quite tied to DataStore I thought it was easier to start from fresh and file follow-up bugs for all of the other functionality, but we can try to patch up the existing code if you'd prefer.
(Assignee)

Updated

3 years ago
Attachment #8741879 - Attachment is obsolete: true
(Assignee)

Updated

3 years ago
Attachment #8743292 - Attachment is obsolete: true
(Assignee)

Comment 8

3 years ago
Comment on attachment 8743923 [details] [review]
[gaia] benfrancis:1257824 > mozilla-b2g:kanikani

Hi Alberto, I've combined your patch with mine to get the "pages" functionality working using the existing places code, and provide some basic "sites" functionality to replace the Bookmarks Data Store.

I've created a first pass on a getPinnedSites method which just resolves a promise with all the pinned sites in one go, we can improve this with something more iterable but hopefully this should be enough to get started on the homescreen.

Some things like clearHistory and screenshotting are still not working and icons probably need some work.

Let me know what you think.
Attachment #8743923 - Flags: review?(apastor)
(Assignee)

Updated

3 years ago
Duplicate of this bug: 1266125

Comment 10

3 years ago
Hi Ben, with this patch could be the start point to replace of the features from the webapp

https://github.com/mozilla-b2g/gaia/tree/kanikani/dead_apps/bookmark

to the system app with the new architecture, what do you think?

I start the review of dead_apps for try to colaborate in revive some of this apps.
Comment on attachment 8743923 [details] [review]
[gaia] benfrancis:1257824 > mozilla-b2g:kanikani

Looks good overall! Made a comment on GH. Can we file a follow up for removing all the BookmarksDatabase code in the System app, and use these services instead? Thanks!
Attachment #8743923 - Flags: review?(apastor) → review+
(Assignee)

Comment 12

3 years ago
Thanks Alberto, addressed review comment and merged. Will file follow-ups.

https://github.com/mozilla-b2g/gaia/commit/f0a173338e064b5f3134c425d09c80d7adb903b9
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.