Bug 1527104 - use rkv::OwnedValue instead of internal OwnedValue in kvstore r=nika
authorMyk Melez <myk@mykzilla.org>
Fri, 15 Feb 2019 18:33:30 +0000
changeset 459581 977d3cf4a200f3690222f6423b50a9a3cf276673
parent 459580 7465be2b8821fe89f5bbf1df01e07d2d81f25568
child 459582 052b3184c809f57b826539eff233cd4ad44264f1
push id35563
push userccoroiu@mozilla.com
push dateSat, 16 Feb 2019 09:36:04 +0000
treeherdermozilla-central@1cfd69d05aa1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnika
bugs1527104, 1490496, 1525392
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1527104 - use rkv::OwnedValue instead of internal OwnedValue in kvstore r=nika The kvstore crate that landed in bug 1490496 uses an internal implementation of OwnedValue because the rkv crate's equivalent was insufficient at the time that kvstore was being developed. rkv's OwnedValue has since evolved to support kvstore's use cases, and bug 1525392 is updating mozilla-central to the latest version of rkv; so we should replace kvstore's internal OwnedValue with rkv::OwnedValue. Differential Revision: https://phabricator.services.mozilla.com/D19436
Cargo.lock
third_party/rust/rkv/.cargo-checksum.json
third_party/rust/rkv/Cargo.toml
third_party/rust/rkv/src/env.rs
third_party/rust/rkv/src/value.rs
toolkit/components/kvstore/Cargo.toml
toolkit/components/kvstore/src/error.rs
toolkit/components/kvstore/src/lib.rs
toolkit/components/kvstore/src/owned_value.rs
toolkit/components/kvstore/src/task.rs
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1,8 +1,10 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
 # This file is automatically @generated by Cargo.
 # It is not intended for manual editing.
 [[package]]
 name = "Inflector"
 version = "0.11.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1348,18 +1350,17 @@ dependencies = [
  "crossbeam-utils 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "failure 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.43 (registry+https://github.com/rust-lang/crates.io-index)",
  "lmdb-rkv 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "moz_task 0.1.0",
  "nserror 0.1.0",
  "nsstring 0.1.0",
- "ordered-float 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "rkv 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "rkv 0.9.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "storage_variant 0.1.0",
  "xpcom 0.1.0",
 ]
 
 [[package]]
 name = "lalrpop"
 version = "0.16.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -2179,17 +2180,17 @@ name = "regex-syntax"
 version = "0.6.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "ucd-util 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
 name = "rkv"
-version = "0.9.2"
+version = "0.9.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "arrayref 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "bincode 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "failure 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "lazy_static 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "lmdb-rkv 0.11.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "ordered-float 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -3485,17 +3486,17 @@ dependencies = [
 "checksum rayon-core 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9d24ad214285a7729b174ed6d3bcfcb80177807f959d95fafd5bfc5c4f201ac8"
 "checksum redox_syscall 0.1.32 (registry+https://github.com/rust-lang/crates.io-index)" = "ab105df655884ede59d45b7070c8a65002d921461ee813a024558ca16030eea0"
 "checksum redox_termios 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "7e891cfe48e9100a70a3b6eb652fef28920c117d366339687bd5576160db0f76"
 "checksum redox_users 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "214a97e49be64fd2c86f568dd0cb2c757d2cc53de95b273b6ad0a1c908482f26"
 "checksum regex 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "1731164734096285ec2a5ec7fea5248ae2f5485b3feeb0115af4fda2183b2d1b"
 "checksum regex 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "75ecf88252dce580404a22444fc7d626c01815debba56a7f4f536772a5ff19d3"
 "checksum regex-syntax 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "ad890a5eef7953f55427c50575c680c42841653abd2b028b68cd223d157f62db"
 "checksum regex-syntax 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8f1ac0f60d675cc6cf13a20ec076568254472551051ad5dd050364d70671bf6b"
-"checksum rkv 0.9.2 (registry+https://github.com/rust-lang/crates.io-index)" = "967f320b7357a64909224619c90c1832f4532c3f19ec24350860bd64c2c7e272"
+"checksum rkv 0.9.3 (registry+https://github.com/rust-lang/crates.io-index)" = "becd7f5278be3b97250a8035455116f9fc63f5fc68cc8293213051d7d751c373"
 "checksum ron 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "da06feaa07f69125ab9ddc769b11de29090122170b402547f64b86fe16ebc399"
 "checksum runloop 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5d79b4b604167921892e84afbbaad9d5ad74e091bf6c511d9dbfb0593f09fabd"
 "checksum rust-ini 0.10.3 (registry+https://github.com/rust-lang/crates.io-index)" = "8a654c5bda722c699be6b0fe4c0d90de218928da5b724c3e467fc48865c37263"
 "checksum rustc-demangle 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "76d7ba1feafada44f2d38eed812bd2489a03c0f5abb975799251518b68848649"
 "checksum rustc_version 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)" = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a"
 "checksum ryu 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)" = "fd0568787116e13c652377b6846f5931454a363a8fdf8ae50463ee40935b278b"
 "checksum same-file 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "cfb6eded0b06a0b512c8ddbcf04089138c9b4362c2f696f3c3d76039d68f3637"
 "checksum scoped-tls 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "f417c22df063e9450888a7561788e9bd46d3bb3c1466435b4eccb903807f147d"
--- a/third_party/rust/rkv/.cargo-checksum.json
+++ b/third_party/rust/rkv/.cargo-checksum.json
@@ -1,1 +1,1 @@
-{"files":{"Cargo.toml":"a8e9c69b953f21a7ff7d1b7dec780f9adcde7f5537b7db4e86db45ff382e9132","LICENSE":"cfc7749b96f63bd31c3c42b5c471bf756814053e847c10f3eb003417bc523d30","README.md":"9dc24375b49fef42f35dec42e316e21827d7337622f9e7cf36243cd28808797a","examples/README.md":"143767fc145bf167ce269a65138cb3f7086cb715b8bc4f73626da82966e646f4","examples/iterator.rs":"ddc3997e394a30ad82d78d2675a48c4617353f88b89bb9a3df5a3804d59b8ef9","examples/simple-store.rs":"9cec5f5a1277edf395775c22a29a27b1d9907ca693a3faa6cbd8e0f0bbff4347","run-all-examples.sh":"7f9d11d01017f77e1c9d26e3e82dfca8c6930deaec85e864458e33a7fa267de0","src/env.rs":"e71264a3ee9d5e87948f56a098099b1394935faa5d77dfcc75aff2c7445634d7","src/error.rs":"46632b8fcb1070a1860247e09a59d39772079ebfba5d3d1bbee03d08e1252275","src/lib.rs":"365cd108bec0e22e8aa010b738a7db2f0da4c6e4cbf1284a1e8ad7e2f1f05736","src/manager.rs":"f06b14ee64f2e58d890a3b37677790b707a02d328242c1af0ce3c74e9028edd8","src/readwrite.rs":"5d5dd64c9b36b7f75b69771e6909c6d48f109ee3725b357f6a9099ddb853e978","src/store.rs":"409d13b1ea0d1254dae947ecbce50e741fb71c3ca118a78803b734336dce6a8f","src/store/integer.rs":"a302c7fb70397b7dca6c116828a309d16c9bc664abe029342d8ebdd730d8b457","src/store/integermulti.rs":"f2c8f9c70d1615757ccb0a56a9642ad6769236fd4c406767f5a71fa84eeeaacf","src/store/multi.rs":"9456f5ff3cec3bf2fc27660b18483e1f0752b5f5f6279b4cfcd1898e236188cb","src/store/single.rs":"09f594b7150cbdad4b8a5dc208d4b0ce4962139b8c856276264dd24c98ac92a4","src/value.rs":"660a8e2fad3d13c3073d15f83f28e16662d9f1a682f6673ee1b885c3c28d44dd","tests/integer-store.rs":"f7e06c71b0dead2323c7c61fc8bcbffbdd3a4796eebf6138db9cce3dbba716a3","tests/manager.rs":"97ec61145dc227f4f5fbcb6449c096bbe5b9a09db4e61ff4491c0443fe9adf26","tests/multi-integer-store.rs":"83295b0135c502321304aa06b05d5a9eeab41b1438ed7ddf2cb1a3613dfef4d9"},"package":"967f320b7357a64909224619c90c1832f4532c3f19ec24350860bd64c2c7e272"}
\ No newline at end of file
+{"files":{"Cargo.toml":"448959dc5f9f13c678fef187c10974637b79aa10e9ff396c78da92a5d923c2a7","LICENSE":"cfc7749b96f63bd31c3c42b5c471bf756814053e847c10f3eb003417bc523d30","README.md":"9dc24375b49fef42f35dec42e316e21827d7337622f9e7cf36243cd28808797a","examples/README.md":"143767fc145bf167ce269a65138cb3f7086cb715b8bc4f73626da82966e646f4","examples/iterator.rs":"ddc3997e394a30ad82d78d2675a48c4617353f88b89bb9a3df5a3804d59b8ef9","examples/simple-store.rs":"9cec5f5a1277edf395775c22a29a27b1d9907ca693a3faa6cbd8e0f0bbff4347","run-all-examples.sh":"7f9d11d01017f77e1c9d26e3e82dfca8c6930deaec85e864458e33a7fa267de0","src/env.rs":"9c633d163274a9f76b30b4ce8439120dddac5b778b5300e02bc8fd22a053c0d1","src/error.rs":"46632b8fcb1070a1860247e09a59d39772079ebfba5d3d1bbee03d08e1252275","src/lib.rs":"365cd108bec0e22e8aa010b738a7db2f0da4c6e4cbf1284a1e8ad7e2f1f05736","src/manager.rs":"f06b14ee64f2e58d890a3b37677790b707a02d328242c1af0ce3c74e9028edd8","src/readwrite.rs":"5d5dd64c9b36b7f75b69771e6909c6d48f109ee3725b357f6a9099ddb853e978","src/store.rs":"409d13b1ea0d1254dae947ecbce50e741fb71c3ca118a78803b734336dce6a8f","src/store/integer.rs":"a302c7fb70397b7dca6c116828a309d16c9bc664abe029342d8ebdd730d8b457","src/store/integermulti.rs":"f2c8f9c70d1615757ccb0a56a9642ad6769236fd4c406767f5a71fa84eeeaacf","src/store/multi.rs":"9456f5ff3cec3bf2fc27660b18483e1f0752b5f5f6279b4cfcd1898e236188cb","src/store/single.rs":"09f594b7150cbdad4b8a5dc208d4b0ce4962139b8c856276264dd24c98ac92a4","src/value.rs":"ad74ba4c9ab0a77f1c4f8ee2650ceeb148e4036b017d804affc35085e97944fb","tests/integer-store.rs":"f7e06c71b0dead2323c7c61fc8bcbffbdd3a4796eebf6138db9cce3dbba716a3","tests/manager.rs":"97ec61145dc227f4f5fbcb6449c096bbe5b9a09db4e61ff4491c0443fe9adf26","tests/multi-integer-store.rs":"83295b0135c502321304aa06b05d5a9eeab41b1438ed7ddf2cb1a3613dfef4d9"},"package":"becd7f5278be3b97250a8035455116f9fc63f5fc68cc8293213051d7d751c373"}
\ No newline at end of file
--- a/third_party/rust/rkv/Cargo.toml
+++ b/third_party/rust/rkv/Cargo.toml
@@ -8,17 +8,17 @@
 # If you believe there's an error in this file please file an
 # issue against the rust-lang/cargo repository. If you're
 # editing this file be aware that the upstream Cargo.toml
 # will likely look very different (and much more reasonable)
 
 [package]
 edition = "2018"
 name = "rkv"
-version = "0.9.2"
+version = "0.9.3"
 authors = ["Richard Newman <rnewman@twinql.com>", "Nan Jiang <najiang@mozilla.com>", "Myk Melez <myk@mykzilla.org>"]
 description = "a simple, humane, typed Rust interface to LMDB"
 homepage = "https://github.com/mozilla/rkv"
 readme = "README.md"
 keywords = ["lmdb", "database", "storage"]
 categories = ["database"]
 license = "Apache-2.0"
 repository = "https://github.com/mozilla/rkv"
--- a/third_party/rust/rkv/src/env.rs
+++ b/third_party/rust/rkv/src/env.rs
@@ -1022,9 +1022,92 @@ mod tests {
             }));
         }
 
         // Sum the values returned from the threads and confirm that they're
         // equal to the sum of values written to the threads.
         let thread_sum: u64 = read_handles.into_iter().map(|handle| handle.join().expect("value")).sum();
         assert_eq!(thread_sum, (0..num_threads).sum());
     }
+
+    #[test]
+    fn test_use_value_as_key() {
+        let root = Builder::new().prefix("test_use_value_as_key").tempdir().expect("tempdir");
+        let rkv = Rkv::new(root.path()).expect("new succeeded");
+        let store = rkv.open_single("store", StoreOptions::create()).expect("opened");
+
+        {
+            let mut writer = rkv.write().expect("writer");
+            store.put(&mut writer, "foo", &Value::Str("bar")).expect("wrote");
+            store.put(&mut writer, "bar", &Value::Str("baz")).expect("wrote");
+            writer.commit().expect("committed");
+        }
+
+        // It's possible to retrieve a value with a Reader and then use it
+        // as a key with a Writer.
+        {
+            let reader = &rkv.read().unwrap();
+            if let Some(Value::Str(key)) = store.get(reader, "foo").expect("read") {
+                let mut writer = rkv.write().expect("writer");
+                store.delete(&mut writer, key).expect("deleted");
+                writer.commit().expect("committed");
+            }
+        }
+
+        {
+            let mut writer = rkv.write().expect("writer");
+            store.put(&mut writer, "bar", &Value::Str("baz")).expect("wrote");
+            writer.commit().expect("committed");
+        }
+
+        // You can also retrieve a Value with a Writer and then use it as a key
+        // with the same Writer if you copy the value to an owned type
+        // so the Writer isn't still being borrowed by the retrieved value
+        // when you try to borrow the Writer again to modify that value.
+        {
+            let mut writer = rkv.write().expect("writer");
+            if let Some(Value::Str(value)) = store.get(&writer, "foo").expect("read") {
+                let key = value.to_owned();
+                store.delete(&mut writer, key).expect("deleted");
+                writer.commit().expect("committed");
+            }
+        }
+
+        {
+            let name1 = rkv.open_single("name1", StoreOptions::create()).expect("opened");
+            let name2 = rkv.open_single("name2", StoreOptions::create()).expect("opened");
+            let mut writer = rkv.write().expect("writer");
+            name1.put(&mut writer, "key1", &Value::Str("bar")).expect("wrote");
+            name1.put(&mut writer, "bar", &Value::Str("baz")).expect("wrote");
+            name2.put(&mut writer, "key2", &Value::Str("bar")).expect("wrote");
+            name2.put(&mut writer, "bar", &Value::Str("baz")).expect("wrote");
+            writer.commit().expect("committed");
+        }
+
+        // You can also iterate (store, key) pairs to retrieve foreign keys,
+        // then iterate those foreign keys to modify/delete them.
+        //
+        // You need to open the stores in advance, since opening a store
+        // uses a write transaction internally, so opening them while a writer
+        // is extant will hang.
+        //
+        // And you need to copy the values to an owned type so the Writer isn't
+        // still being borrowed by a retrieved value when you try to borrow
+        // the Writer again to modify another value.
+        let fields = vec![
+            (rkv.open_single("name1", StoreOptions::create()).expect("opened"), "key1"),
+            (rkv.open_single("name2", StoreOptions::create()).expect("opened"), "key2"),
+        ];
+        {
+            let mut foreignkeys = Vec::new();
+            let mut writer = rkv.write().expect("writer");
+            for (store, key) in fields.iter() {
+                if let Some(Value::Str(value)) = store.get(&writer, key).expect("read") {
+                    foreignkeys.push((store, value.to_owned()));
+                }
+            }
+            for (store, key) in foreignkeys.iter() {
+                store.delete(&mut writer, key).expect("deleted");
+            }
+            writer.commit().expect("committed");
+        }
+    }
 }
--- a/third_party/rust/rkv/src/value.rs
+++ b/third_party/rust/rkv/src/value.rs
@@ -163,40 +163,56 @@ impl<'s> Value<'s> {
         .map_err(|e| DataError::DecodingError {
             value_type: t,
             err: e,
         })
     }
 
     pub fn to_bytes(&self) -> Result<Vec<u8>, DataError> {
         match self {
-            Value::Bool(ref v) => serialize(&(Type::Bool.to_tag(), *v)),
-            Value::U64(ref v) => serialize(&(Type::U64.to_tag(), *v)),
-            Value::I64(ref v) => serialize(&(Type::I64.to_tag(), *v)),
-            Value::F64(ref v) => serialize(&(Type::F64.to_tag(), v.0)),
-            Value::Instant(ref v) => serialize(&(Type::Instant.to_tag(), *v)),
-            Value::Str(ref v) => serialize(&(Type::Str.to_tag(), v)),
-            Value::Json(ref v) => serialize(&(Type::Json.to_tag(), v)),
-            Value::Blob(ref v) => serialize(&(Type::Blob.to_tag(), v)),
-            Value::Uuid(ref v) => {
+            Value::Bool(v) => serialize(&(Type::Bool.to_tag(), *v)),
+            Value::U64(v) => serialize(&(Type::U64.to_tag(), *v)),
+            Value::I64(v) => serialize(&(Type::I64.to_tag(), *v)),
+            Value::F64(v) => serialize(&(Type::F64.to_tag(), v.0)),
+            Value::Instant(v) => serialize(&(Type::Instant.to_tag(), *v)),
+            Value::Str(v) => serialize(&(Type::Str.to_tag(), v)),
+            Value::Json(v) => serialize(&(Type::Json.to_tag(), v)),
+            Value::Blob(v) => serialize(&(Type::Blob.to_tag(), v)),
+            Value::Uuid(v) => {
                 // Processed above to avoid verbose duplication of error transforms.
                 serialize(&(Type::Uuid.to_tag(), v))
             },
         }
         .map_err(DataError::EncodingError)
     }
 }
 
 impl<'s> From<&'s Value<'s>> for OwnedValue {
     fn from(value: &Value) -> OwnedValue {
         match value {
-            Value::Bool(ref v) => OwnedValue::Bool(*v),
-            Value::U64(ref v) => OwnedValue::U64(*v),
-            Value::I64(ref v) => OwnedValue::I64(*v),
-            Value::F64(ref v) => OwnedValue::F64(**v),
-            Value::Instant(ref v) => OwnedValue::Instant(*v),
-            Value::Uuid(ref v) => OwnedValue::Uuid(Uuid::from_bytes(**v)),
-            Value::Str(ref v) => OwnedValue::Str(v.to_string()),
-            Value::Json(ref v) => OwnedValue::Json(v.to_string()),
-            Value::Blob(ref v) => OwnedValue::Blob(v.to_vec()),
+            Value::Bool(v) => OwnedValue::Bool(*v),
+            Value::U64(v) => OwnedValue::U64(*v),
+            Value::I64(v) => OwnedValue::I64(*v),
+            Value::F64(v) => OwnedValue::F64(**v),
+            Value::Instant(v) => OwnedValue::Instant(*v),
+            Value::Uuid(v) => OwnedValue::Uuid(Uuid::from_bytes(**v)),
+            Value::Str(v) => OwnedValue::Str(v.to_string()),
+            Value::Json(v) => OwnedValue::Json(v.to_string()),
+            Value::Blob(v) => OwnedValue::Blob(v.to_vec()),
         }
     }
 }
+
+impl<'s> From<&'s OwnedValue> for Value<'s> {
+    fn from(value: &OwnedValue) -> Value {
+        match value {
+            OwnedValue::Bool(v) => Value::Bool(*v),
+            OwnedValue::U64(v) => Value::U64(*v),
+            OwnedValue::I64(v) => Value::I64(*v),
+            OwnedValue::F64(v) => Value::F64(OrderedFloat::from(*v)),
+            OwnedValue::Instant(v) => Value::Instant(*v),
+            OwnedValue::Uuid(v) => Value::Uuid(v.as_bytes()),
+            OwnedValue::Str(v) => Value::Str(v),
+            OwnedValue::Json(v) => Value::Json(v),
+            OwnedValue::Blob(v) => Value::Blob(v),
+        }
+    }
+}
--- a/toolkit/components/kvstore/Cargo.toml
+++ b/toolkit/components/kvstore/Cargo.toml
@@ -7,18 +7,17 @@ authors = ["Myk Melez <myk@mykzilla.org>
 atomic_refcell = "0.1"
 crossbeam-utils = "0.6.3"
 libc = "0.2"
 lmdb-rkv = "0.11.2"
 log = "0.4"
 moz_task = { path = "../../../xpcom/rust/moz_task" }
 nserror = { path = "../../../xpcom/rust/nserror" }
 nsstring = { path = "../../../xpcom/rust/nsstring" }
-ordered-float = "1"
-rkv = "0.9.2"
+rkv = "0.9.3"
 storage_variant = { path = "../../../storage/variant" }
 xpcom = { path = "../../../xpcom/rust/xpcom" }
 
 # Get rid of failure's dependency on backtrace. Eventually
 # backtrace will move into Rust core, but we don't need it here.
 [dependencies.failure]
 version = "0.1"
 default_features = false
--- a/toolkit/components/kvstore/src/error.rs
+++ b/toolkit/components/kvstore/src/error.rs
@@ -35,21 +35,24 @@ pub enum KeyValueError {
     PoisonError,
 
     #[fail(display = "error reading key/value pair")]
     Read,
 
     #[fail(display = "store error: {:?}", _0)]
     StoreError(StoreError),
 
-    #[fail(display = "unsupported type: {}", _0)]
-    UnsupportedType(uint16_t),
+    #[fail(display = "unsupported owned value type")]
+    UnsupportedOwned,
 
     #[fail(display = "unexpected value")]
     UnexpectedValue,
+
+    #[fail(display = "unsupported variant type: {}", _0)]
+    UnsupportedVariant(uint16_t),
 }
 
 impl From<nsresult> for KeyValueError {
     fn from(result: nsresult) -> KeyValueError {
         KeyValueError::Nsresult(result.error_name(), result)
     }
 }
 
@@ -59,18 +62,19 @@ impl From<KeyValueError> for nsresult {
             KeyValueError::ConvertBytes(_) => NS_ERROR_FAILURE,
             KeyValueError::ConvertString(_) => NS_ERROR_FAILURE,
             KeyValueError::NoInterface(_) => NS_ERROR_NO_INTERFACE,
             KeyValueError::Nsresult(_, result) => result,
             KeyValueError::NullPointer => NS_ERROR_NULL_POINTER,
             KeyValueError::PoisonError => NS_ERROR_UNEXPECTED,
             KeyValueError::Read => NS_ERROR_FAILURE,
             KeyValueError::StoreError(_) => NS_ERROR_FAILURE,
-            KeyValueError::UnsupportedType(_) => NS_ERROR_NOT_IMPLEMENTED,
+            KeyValueError::UnsupportedOwned => NS_ERROR_NOT_IMPLEMENTED,
             KeyValueError::UnexpectedValue => NS_ERROR_UNEXPECTED,
+            KeyValueError::UnsupportedVariant(_) => NS_ERROR_NOT_IMPLEMENTED,
         }
     }
 }
 
 impl From<StoreError> for KeyValueError {
     fn from(err: StoreError) -> KeyValueError {
         KeyValueError::StoreError(err)
     }
--- a/toolkit/components/kvstore/src/lib.rs
+++ b/toolkit/components/kvstore/src/lib.rs
@@ -7,34 +7,33 @@ extern crate crossbeam_utils;
 #[macro_use]
 extern crate failure;
 extern crate libc;
 extern crate lmdb;
 extern crate log;
 extern crate moz_task;
 extern crate nserror;
 extern crate nsstring;
-extern crate ordered_float;
 extern crate rkv;
 extern crate storage_variant;
 #[macro_use]
 extern crate xpcom;
 
 mod error;
 mod owned_value;
 mod task;
 
 use atomic_refcell::AtomicRefCell;
 use error::KeyValueError;
 use libc::c_void;
 use moz_task::{create_thread, TaskRunnable};
 use nserror::{nsresult, NS_ERROR_FAILURE, NS_ERROR_NO_AGGREGATION, NS_OK};
 use nsstring::{nsACString, nsCString};
-use owned_value::{owned_to_variant, variant_to_owned, OwnedValue};
-use rkv::{Rkv, SingleStore};
+use owned_value::{owned_to_variant, variant_to_owned};
+use rkv::{OwnedValue, Rkv, SingleStore};
 use std::{
     ptr,
     sync::{Arc, RwLock},
     vec::IntoIter,
 };
 use task::{DeleteTask, EnumerateTask, GetOrCreateTask, GetTask, HasTask, PutTask};
 use xpcom::{
     interfaces::{
@@ -323,11 +322,11 @@ impl KeyValuePair {
     xpcom_method!(get_key => GetKey() -> nsACString);
     xpcom_method!(get_value => GetValue() -> *const nsIVariant);
 
     fn get_key(&self) -> Result<nsCString, KeyValueError> {
         Ok(nsCString::from(&self.key))
     }
 
     fn get_value(&self) -> Result<RefPtr<nsIVariant>, KeyValueError> {
-        Ok(owned_to_variant(self.value.clone()))
+        Ok(owned_to_variant(self.value.clone())?)
     }
 }
--- a/toolkit/components/kvstore/src/owned_value.rs
+++ b/toolkit/components/kvstore/src/owned_value.rs
@@ -1,75 +1,62 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use error::KeyValueError;
 use libc::int32_t;
 use nserror::NsresultExt;
 use nsstring::nsString;
-use ordered_float::OrderedFloat;
-use rkv::Value;
+use rkv::OwnedValue;
 use storage_variant::{
     GetDataType, VariantType, DATA_TYPE_BOOL, DATA_TYPE_DOUBLE, DATA_TYPE_EMPTY, DATA_TYPE_INT32,
     DATA_TYPE_VOID, DATA_TYPE_WSTRING,
 };
 use xpcom::{interfaces::nsIVariant, RefPtr};
 
-// This is implemented in rkv but is incomplete there.  We implement a subset
-// to give KeyValuePair ownership over its value, so it can #[derive(xpcom)].
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub enum OwnedValue {
-    Bool(bool),
-    I64(i64),
-    F64(OrderedFloat<f64>),
-    Str(String),
-}
+pub fn owned_to_variant(owned: OwnedValue) -> Result<RefPtr<nsIVariant>, KeyValueError> {
+    match owned {
+        OwnedValue::Bool(val) => Ok(val.into_variant()),
+        OwnedValue::I64(val) => Ok(val.into_variant()),
+        OwnedValue::F64(val) => Ok(val.into_variant()),
+        OwnedValue::Str(ref val) => Ok(nsString::from(val).into_variant()),
 
-pub fn value_to_owned(value: Option<Value>) -> Result<OwnedValue, KeyValueError> {
-    match value {
-        Some(Value::Bool(val)) => Ok(OwnedValue::Bool(val)),
-        Some(Value::I64(val)) => Ok(OwnedValue::I64(val)),
-        Some(Value::F64(val)) => Ok(OwnedValue::F64(val)),
-        Some(Value::Str(val)) => Ok(OwnedValue::Str(val.to_owned())),
-        Some(_value) => Err(KeyValueError::UnexpectedValue),
-        None => Err(KeyValueError::UnexpectedValue),
-    }
-}
-
-pub fn owned_to_variant(owned: OwnedValue) -> RefPtr<nsIVariant> {
-    match owned {
-        OwnedValue::Bool(val) => val.into_variant(),
-        OwnedValue::I64(val) => val.into_variant(),
-        OwnedValue::F64(OrderedFloat(val)) => val.into_variant(),
-        OwnedValue::Str(ref val) => nsString::from(val).into_variant(),
+        // kvstore doesn't (yet?) support these types of OwnedValue,
+        // and we should never encounter them, but we need to exhaust
+        // all possible variants of the OwnedValue enum.
+        OwnedValue::Instant(_) => Err(KeyValueError::UnsupportedOwned),
+        OwnedValue::Json(_) => Err(KeyValueError::UnsupportedOwned),
+        OwnedValue::U64(_) => Err(KeyValueError::UnsupportedOwned),
+        OwnedValue::Uuid(_) => Err(KeyValueError::UnsupportedOwned),
+        OwnedValue::Blob(_) => Err(KeyValueError::UnsupportedOwned),
     }
 }
 
 pub fn variant_to_owned(variant: &nsIVariant) -> Result<Option<OwnedValue>, KeyValueError> {
     let data_type = variant.get_data_type();
 
     match data_type {
         DATA_TYPE_INT32 => {
             let mut val: int32_t = 0;
             unsafe { variant.GetAsInt32(&mut val) }.to_result()?;
             Ok(Some(OwnedValue::I64(val.into())))
         }
         DATA_TYPE_DOUBLE => {
             let mut val: f64 = 0.0;
             unsafe { variant.GetAsDouble(&mut val) }.to_result()?;
-            Ok(Some(OwnedValue::F64(val.into())))
+            Ok(Some(OwnedValue::F64(val)))
         }
         DATA_TYPE_WSTRING => {
             let mut val: nsString = nsString::new();
             unsafe { variant.GetAsAString(&mut *val) }.to_result()?;
             let str = String::from_utf16(&val)?;
             Ok(Some(OwnedValue::Str(str)))
         }
         DATA_TYPE_BOOL => {
             let mut val: bool = false;
             unsafe { variant.GetAsBool(&mut val) }.to_result()?;
             Ok(Some(OwnedValue::Bool(val)))
         }
         DATA_TYPE_EMPTY | DATA_TYPE_VOID => Ok(None),
-        unsupported_type => Err(KeyValueError::UnsupportedType(unsupported_type)),
+        unsupported_type => Err(KeyValueError::UnsupportedVariant(unsupported_type)),
     }
 }
--- a/toolkit/components/kvstore/src/task.rs
+++ b/toolkit/components/kvstore/src/task.rs
@@ -3,19 +3,19 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 extern crate xpcom;
 
 use crossbeam_utils::atomic::AtomicCell;
 use error::KeyValueError;
 use moz_task::Task;
 use nserror::{nsresult, NsresultExt, NS_ERROR_FAILURE};
-use nsstring::{nsCString, nsString};
-use owned_value::{value_to_owned, OwnedValue};
-use rkv::{Manager, Rkv, SingleStore, StoreError, StoreOptions, Value};
+use nsstring::nsCString;
+use owned_value::owned_to_variant;
+use rkv::{Manager, OwnedValue, Rkv, SingleStore, StoreError, StoreOptions, Value};
 use std::{
     path::Path,
     str,
     sync::{Arc, RwLock},
 };
 use storage_variant::VariantType;
 use xpcom::{
     interfaces::{
@@ -165,24 +165,18 @@ impl Task for PutTask {
     fn run(&self) {
         // We do the work within a closure that returns a Result so we can
         // use the ? operator to simplify the implementation.
         self.result.store(Some(|| -> Result<(), KeyValueError> {
             let key = str::from_utf8(&self.key)?;
             let env = self.rkv.read()?;
             let mut writer = env.write()?;
 
-            let value = match self.value {
-                OwnedValue::Bool(val) => Value::Bool(val),
-                OwnedValue::I64(val) => Value::I64(val),
-                OwnedValue::F64(val) => Value::F64(val),
-                OwnedValue::Str(ref val) => Value::Str(&val),
-            };
-
-            self.store.put(&mut writer, key, &value)?;
+            self.store
+                .put(&mut writer, key, &Value::from(&self.value))?;
             writer.commit()?;
 
             Ok(())
         }()));
     }
 
     task_done!(void);
 }
@@ -210,45 +204,36 @@ impl GetTask {
             store,
             key,
             default_value,
             result: AtomicCell::default(),
         }
     }
 
     fn convert(&self, result: Option<OwnedValue>) -> Result<RefPtr<nsIVariant>, KeyValueError> {
-        // TODO: refactor with owned_to_variant in owned_value.rs.
         Ok(match result {
-            Some(OwnedValue::Bool(val)) => val.into_variant(),
-            Some(OwnedValue::I64(val)) => val.into_variant(),
-            Some(OwnedValue::F64(val)) => val.into_variant(),
-            Some(OwnedValue::Str(ref val)) => nsString::from(val).into_variant(),
+            Some(val) => owned_to_variant(val)?,
             None => ().into_variant(),
         })
     }
 }
 
 impl Task for GetTask {
     fn run(&self) {
         // We do the work within a closure that returns a Result so we can
         // use the ? operator to simplify the implementation.
         self.result
             .store(Some(|| -> Result<Option<OwnedValue>, KeyValueError> {
                 let key = str::from_utf8(&self.key)?;
                 let env = self.rkv.read()?;
                 let reader = env.read()?;
                 let value = self.store.get(&reader, key)?;
 
-                // TODO: refactor with value_to_owned in owned_value.rs.
                 Ok(match value {
-                    Some(Value::Bool(val)) => Some(OwnedValue::Bool(val)),
-                    Some(Value::I64(val)) => Some(OwnedValue::I64(val)),
-                    Some(Value::F64(val)) => Some(OwnedValue::F64(val)),
-                    Some(Value::Str(val)) => Some(OwnedValue::Str(val.to_owned())),
-                    Some(_value) => return Err(KeyValueError::UnexpectedValue),
+                    Some(value) => Some(OwnedValue::from(&value)),
                     None => match self.default_value {
                         Some(ref val) => Some(val.clone()),
                         None => None,
                     },
                 })
             }()));
     }
 
@@ -436,20 +421,20 @@ impl Task for EnumerateTask {
                                     Err(_err) => true,
                                 }
                             }
                         }
                         Err(_) => true,
                     })
                     // Convert the key/value pair to owned.
                     .map(|result| match result {
-                        Ok((key, val)) => match (key, value_to_owned(val)) {
-                            (Ok(key), Ok(val)) => Ok((key.to_owned(), val)),
+                        Ok((key, val)) => match (key, val) {
+                            (Ok(key), Some(val)) => Ok((key.to_owned(), OwnedValue::from(&val))),
                             (Err(err), _) => Err(err.into()),
-                            (_, Err(err)) => Err(err),
+                            (_, None) => Err(KeyValueError::UnexpectedValue),
                         },
                         Err(err) => Err(KeyValueError::StoreError(err)),
                     })
                     .collect();
 
                 Ok(pairs)
             }(),
         ));