Closed Bug 1143154 Opened 9 years ago Closed 9 years ago

Data store for Synced Tabs


(Firefox for iOS :: Home screen, defect)

iOS 8
Not set





(Reporter: nalexander, Assigned: nalexander)




(1 file)

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: iostabspanel
This should blocked the Synced Tabs panel.  I am actively working on this.
Assignee: nobody → nalexander
Blocks: iostabspanel
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)
Depends on: 1143952
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:

rnewman reviewed on GH and IRC.
Attachment #8578346 - Flags: review?(rnewman) → review+
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.