Data store for Synced Tabs

RESOLVED FIXED

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

This ticket tracks implementing an SQLite data store for iOS Fennec's representation of Sync's clients and tab records.
rnewman: can you outline v1 of this?  Can it just be a clients table and a tabs table, like in Fennec?  Or do we need to build in a Sync store layer up-front?
Flags: needinfo?(rnewman)
I think this is a two-parter, with the other part being Bug 1134904. That bug has some schema details for tabs itself.

Because clients and tabs are unidirectional (see Bug 1141845 and friends), we don't really need to do anything special here: just a place to put client and tab records when we download them.
Flags: needinfo?(rnewman)
(In reply to Richard Newman [:rnewman] from comment #2)
> I think this is a two-parter, with the other part being Bug 1134904. That
> bug has some schema details for tabs itself.

Here, I'm thinking strictly of storage for Synced Tabs.  I don't care if we store local tabs in the same DB/table but we don't *need* to, and since Sync's tabs format is relatively impoverished, we might want to split this storage entirely.  So yes: two parts.
Blocks: 1141847
No longer blocks: 1130495
This should blocked the Synced Tabs panel.  I am actively working on this.
Assignee: nobody → nalexander
Blocks: 1130495
Status: NEW → ASSIGNED
Created attachment 8578346 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/228

rnewman: st3fan: still some things left to do here, but I thought I'd get feedback early.

I kind of went for a hybrid: two GenericTables but some roll your own SQL in the middle.  I originally intended to do a single LEFT OUTER JOIN like we do on Android, but it got a little... duplicative.  So I went for two queries (clients, then all tabs).

I don't like the callback interface but I don't care to depend on Result and Deferred just yet.
Attachment #8578346 - Flags: review?(rnewman)
Attachment #8578346 - Flags: feedback?(sarentz)
I think this looks good.

My only request is to test with real timestamps. The changes that I made to support long long timestamps are probably ok but it would be good to see some more tests use it.

(Just use 13 digit numbers to test with and see if they come back ok)
Attachment #8578346 - Flags: feedback?(sarentz)
Comment on attachment 8578346 [details] [review]
Link to Github pull-request: https://github.com/mozilla/firefox-ios/pull/228

rnewman reviewed on GH and IRC.
Attachment #8578346 - Flags: review?(rnewman) → review+
https://github.com/mozilla/firefox-ios/commit/c6f9eb3930587093a0f67d421e0ae5e4f3d87ac3
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.