Closed Bug 946778 Opened 11 years ago Closed 11 years ago

Create Places Database for System App

Categories

(Firefox OS Graveyard :: Gaia::System, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
1.3 C2/1.4 S2(17jan)

People

(Reporter: benfrancis, Assigned: daleharvey)

References

Details

(Whiteboard: system-browser [systemsfe])

Attachments

(2 files, 3 obsolete files)

Like the places database in the browser app, but in the system app.
Some thoughts... I'm currently wondering whether the bookmarks object store is actually necessary for the system app, perhaps this should just be managed by the homescreen app. We won't need the settings object store. I'd like to experiment with some refactoring to remove one layer of abstraction from the BrowserDB.js code as I think it's a bit overcomplicated at the moment.
Component: Gaia::System → Gaia::System::Browser Chrome
Attached image Places Database Diagram (deleted) —
Trying to get my head around which operations which apps need to perform on which data... One challenge is that a "get" in this diagram means "get by URL" but DataStore only allows integers as keys so we might need to create an IndexedDB object store with the sole purpose of mapping URLs to DataStore IDs. I've filed bug 946316 to allow strings as IDs which would make this much simpler.
Moving this back into the system component as the Places system itself isn't part of browser chrome, browser chrome just talks to it.
Component: Gaia::System::Browser Chrome → Gaia::System
Depends on: 949491, 948014
Depends on: 938406
Blocks: 949469
Attached file https://github.com/mozilla-b2g/gaia/pull/14663 (obsolete) (deleted) —
Attachment #8347381 - Flags: review?(dale)
Attached file https://github.com/mozilla-b2g/gaia/pull/14702 (obsolete) (deleted) —
Hmm, a rebase of the rocketbar2 branch broke all downstream branches. Sending a pull request against master instead as that's less volatile and these changes are safe to land independently of that branch. The rocketbar branch can get them in the next merge from master.
Attachment #8347381 - Attachment is obsolete: true
Attachment #8347381 - Flags: review?(dale)
Attachment #8347983 - Flags: review?(dale)
There would be a few issues with merging this, as .init should signal when the store is ready, the test stuff should be split out / fixed, 946316 is almost landed so in the meantime we should probably just turn urls into ints, we should do this with the rocketbar branch to get it merged and tested asap Cancelling review and stealing as Ben is on PTO
Assignee: bfrancis → dale
Attachment #8347983 - Flags: review?(dale)
Attached file Add initial places store (obsolete) (deleted) —
Attachment #8347983 - Attachment is obsolete: true
Attachment #8349362 - Flags: review?(kgrandon)
Comment on attachment 8349362 [details] Add initial places store R+ing for now with the caveat that we do not land yet. There is concern around the upgrade path, and local map to store urls -> ids. Let's have some more discussion before landing. Thanks!
Attachment #8349362 - Flags: review?(kgrandon) → review+
I split out the 'places_init' file since the tests need to manually instantiate it (to get the callback), if theres a nice way then can edit Needs a notification of url changes, there was an unused callback already in app_window.js, not sure if thats the best way Alive?
Attachment #8349362 - Attachment is obsolete: true
Attachment #8356737 - Flags: review?(kgrandon)
Attachment #8356737 - Flags: review?(alive)
Attachment #8356737 - Flags: feedback?(bfrancis)
Comment on attachment 8356737 [details] https://github.com/mozilla-b2g/gaia/pull/15081 Generally looks good :) I've left some comments on the pull request though, we need to figure out what to do with app:// URLs for packaged apps and also http:// URLs corresponding to installed hosted apps when the user selects a Rocketbar result. Do we launch the app, or do we create an AppWindow with the src set to the URL and with the mozapp attribute set? How do we know which app and therefore which app manifest a URL corresponds to?
Attachment #8356737 - Flags: feedback?(bfrancis) → feedback+
Comment on attachment 8356737 [details] https://github.com/mozilla-b2g/gaia/pull/15081 I would just like to move the init() code into the bootstrap.js file. There are a few other init() calls in there.
Attachment #8356737 - Flags: review?(kgrandon) → review+
Oh, and looks like a travis test failed, "navigator.getDataStores is not a function" so would be good to fix that, and land when green :)
Moved the init call, checking out the travis failure
Whiteboard: system-browser → system-browser [systemsfe]
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
Updated PR to address nits, it removes the unit tests as the current unit test infrastructure isnt able to test datastore, carrying r+ as agreed, will land when I see green
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: