Closed Bug 1319884 Opened 3 years ago Closed 3 years ago

Address FirefoxAdapter feedback from kinto.js#589


(WebExtensions :: Untriaged, defect)

Not set


(firefox53 fixed)

Tracking Status
firefox53 --- fixed


(Reporter: glasserc, Assigned: glasserc)



(2 files)

:n1k0 in commented on a few changes I made to the FirefoxAdapter storage adapter for Kinto.js. (That PR was the one that allowed FirefoxAdapter to use an externally-managed Sqlite connection rather than creating its own; this was necessary to address feedback from bug 1253740.) :n1k0 observed that this caused two issues for FirefoxAdapter:

- It now exports _init, which is simultaneously an implementation detail AND something that callers are supposed to know that they need to use.

- FirefoxAdapter is now responsible for functioning whether or not a Sqlite connection is present. This adds a certain amount of complexity to the module. :n1k0 suggested I pick one or the other and convert existing uses.

This bug represents the work that fixes these issues.
In doing this work, I looked at the other main client of kinto.js in Gecko is blocklist-clients.js, and it mainly calls and collection.db.close(). In this approach, the Sqlite connection is internal to the Collection object. This does have a certain advantage in that 1. it's impossible to "forget" to _init the Sqlite connection (to create tables and so forth), 2. it makes the "common" case, where you don't care about the lifetime of the Sqlite connection, a little easier to manage.

An ideal API would cover all use cases to exist. So far, I haven't been able to find such an API.
Assignee: nobody → eglassercamp
Comment on attachment 8813810 [details]
Bug 1319884 - Address FirefoxAdapter feedback from kinto.js#589,
Attachment #8813810 - Flags: review?(mgoodwin) → review+
Keywords: checkin-needed
Pushed by
Address FirefoxAdapter feedback from kinto.js#589, r=mgoodwin
Leftover cleanups, r=MattN
Keywords: checkin-needed
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.