webext-storage: Add explicit method to close the database

[mhammond]

While the DB will be closed when the store is dropped, we might as well expose an explicit close method - it will also let us capture any errors during a close(), which could be useful for diagnostics.

┆Issue is synchronized with this Jira Task ┆Story Points: 2 ┆Sprint: SYNC - end 2020-05-08

[mhammond]

So .close() consumes the connection, which (iiuc), pretty much means it will also need to consume the store etc too (which TBH makes the most sense from an API perspective too). However, I don't quite see how this would map to the bridge in m-c, other than by swapping the OnceCell for a regular cell - @linacambridge WDYT?

[mhammond]

Actually, the problem is closer to home than m-c - Arc<Mutex<Connection>> needs to be consumed and I'm not sure that's possible without a wrapping Option<> (or similar)?

[linabutler]

Thanks very much, @mhammond!

I can do this tomorrow, since I've been poking around in there anyway. You're right that we need a RefCell<Option<...>> in there, but that can live on the m-c side of the bridge. Its teardown method already takes ownership of the store, so all we have to do is add the call to our close method.

Thinking about it even more, I also don't think we need an Arc<Mutex<Connection>>, since our StorageSyncArea wrapper already holds an Arc<LazyStore>. So we can have it be just Mutex<Connection>. This also means we don't have to use Arc::try_unwrap to get at the Connection, since we don't have to worry about other owning references.

But wait, there's more! 😆 Our ConcurrentHandleMap for the FFI already wraps its contents in a mutex. So, in our FFI code, when we write static ref STORES: ConcurrentHandleMap<Store> = ConcurrentHandleMap::new(), it's actually defining a RwLock<HandleMap<Mutex<Store>>>. That means we don't need our StoreDb to have a mutex, either, because the handle map already makes one!

We'll need to change Desktop's LazyStore slightly to hold on to a OnceCell<Mutex<Store>>, and change its get method to return a MutexGuard<'_, Store> instead of a &Store...but that's a four-line change! 😄

So, in conclusion, we can:

1. Change StorageDb to hold a writer: Connection. We don't need the Arc<Mutex<...>> wrapping.
2. Add a stub ffi crate, with a ConcurrentHandleMap<Store>, to verify that the above still compiles. (I've verified this works locally, but it would be good to check in as a test).
3. Since we got rid of the Mutex from StorageDb (and Store holds a StorageDb), we need to change Desktop's LazyStore to hold a OnceCell<Mutex<Store>> instead of a OnceCell<Store>. This is an easy change, too.
4. Add a StorageDb::close method that consumes the StorageDb, and a Store::close method that consumes the Store.
5. Change Desktop's LazyStore::teardown to call Store::close. LazyStore::teardown already consumes the LazyStore, so this isn't a problem.

TL;DR: We can have our consuming close method and simplify a bunch of stuff in the process. 🎉 I'll get a patch up tomorrow, it might make more sense seeing it in code than written out like this.