1143154 - Data store for Synced Tabs

Status: RESOLVED FIXED

(Firefox for iOS :: Home screen, defect)

Description

[Nick Alexander :nalexander [he/him]]

This ticket tracks implementing an SQLite data store for iOS Fennec's representation of Sync's clients and tab records.

Comment 1

[Nick Alexander :nalexander [he/him]]

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?

Comment 2

[Richard Newman [: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.

Comment 3

[Nick Alexander :nalexander [he/him]]

(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.

Comment 4

[Nick Alexander :nalexander [he/him]]

This should blocked the Synced Tabs panel.  I am actively working on this.

Comment 5

[Nick Alexander :nalexander [he/him]]

Attachment: 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.

Comment 6

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

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)

Comment 7

[Nick Alexander :nalexander [he/him]]

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.

Comment 8

[Nick Alexander :nalexander [he/him]]

https://github.com/mozilla/firefox-ios/commit/c6f9eb3930587093a0f67d421e0ae5e4f3d87ac3