Vendor webext-storage from applications-services into mozilla-central
Categories
(WebExtensions :: Storage, enhancement, P2)
Tracking
(firefox77 fixed)
Tracking | Status | |
---|---|---|
firefox77 | --- | fixed |
People
(Reporter: markh, Assigned: lina)
References
Details
(Whiteboard: SACI)
Attachments
(2 files)
The implementation of chrome.storage.sync
which syncs with the usual Sync servers is being written in Rust and lives in the application-services repo. This will need to be vendored into mozilla-central before it can be used.
Assignee | ||
Updated•4 years ago
|
Comment 1•4 years ago
|
||
Hi Mark,
we are setting the priority to P2 mainly to filter it out by our weekly bug triaging, feel free to bump its priority to P1 once it is ready to be assigned.
Assignee | ||
Comment 2•4 years ago
|
||
Hooray, our first Application Services Rust component! This is a
mechanical run of mach vendor rust
, split out into its own commit
to make reviewing the Firefox bindings easier.
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
The webext_storage_bridge
crate introduced in this commit bridges the
mozIExtensionStorageArea
XPCOM interface to the webext_storage
Rust
component from Application Services.
This commit factors out the following parts from bug 1623245, so that
we can land them piecemeal:
- The
mozIExtensionStorageArea
interfaces, which implement all the
methods needed to support thestorage.sync
API. - A Rust implementation of the above, in
StorageSyncArea
. - A
StorageTask
type, for dispatching storage operations to a
background task queue. - A
LazyStore
, which wraps the Rust component'sStore
and lazily
initializes it on the background queue the first time it's used. - A
StorageSyncService
, which is our singleton. It holds on to a
configuredStorageSyncArea
, and hands out references to it via
getInterface
. Eventually, we'll extend this to support syncing,
too.
Depends on D71895
Assignee | ||
Comment 4•4 years ago
|
||
On Phabricator, Luca asked:
I'd like to know more about this comment and the rationale behind it, especially because I may be missing or misreading something related to it ;-)
If this part is used for the storage.sync implementation part that takes care of storing the data locally (in the new
storage-sync2.sqlite
sqlite > file) as I assume from the quick look a gave to this part, I would expect that this would be in the GeckoView builds too, and then the part related > to actually sync the data from/to the sync backend to be the part integrated at the "Android Components" level.is that right or am I missing something?
Since Phabricator likes to munch comments, and Mark and I were also chatting about Android earlier, I wanted to capture this here! 😁
[So, that's almost right! On Desktop, as you saw, we link our Rust component into libxul
, and call into it directly. But, on Android, our Rust components are "peer dependencies" of GeckoView: our Android Components libraries depend on both, and glue them together. For our other components, like bookmarks, we have our Rust component expose a C-compatible FFI (on Desktop, XPCOM fulfills this role), and then a Kotlin API on top of that for Android Components to call. For bookmarks, it's easier, because GV doesn't need to access bookmarks directly: it's only told "go load this URL", so it doesn't need to talk to the Rust component at all. But, storage.sync
(and history, and password autofill), is special, because we need to expose data that's managed by our Rust component to WebExtensions running in GV.
Splitting it up the way you suggested—including webext_storage
in GV, and the Sync parts in Android Components—is a little tricky, because both the storage and syncing APIs need to write to the same database. That would mean having two writer connections—one inside GV, the other in our Rust component embedded into Android Components—accessing the same database file concurrently, which we've learned is super unreliable. It would also mean linking the Rust component twice: once into the libxul
that ships with GV, and the second time into our Rust "megazord" (a library containing all our Rust components, packaged this way to reduce the size footprint of our Rust code) that we provide for Android Components.
The way we've solved this problem for other data types, like history, is to have GV provide a delegate for the API. In our case, the GV delegate will look a lot like the mozIExtensionStorageArea
interface, maybe something like this:
class WebExtensionStorageAreaChange {
@Nullable public String key;
@Nullable public String oldValue;
@Nullable public String newValue;
};
interface WebExtensionStorageAreaDelegate {
GeckoResult<String> get(String extensionId, String key);
GeckoResult<WebExtensionStorageAreaChange> set(String extensionId,
String key,
String newValue);
GeckoResult<WebExtensionStorageAreaChange> remove(String extensionId,
String key);
GeckoResult<List<WebExtensionStorageAreaChange>> clear(String extensionId);
};
Then, in GV, we'll include a JS implementation of mozIExtensionStorageArea
that calls into the Java delegate (using GV's EventDispatcher
), and passes the results back. And Android Components will implement this delegate in Java/Kotlin, and wire it up to our Rust component. That way, only the Rust component has a connection to the storage DB, it integrates well with the other Sync code that's already there, and we only link one copy of webext_storage
into our megazord.
Web Push is actually an even better example of this, because it follows the same pattern I described above! There's an nsIPushService
interface, which GeckoView implements to pass data back and forth between web content and a WebPushDelegate
. Android Components provides an implementation of this delegate that calls into the Kotlin API for our Push Rust component.
That's quite the tiramisu! 😂 But that's the suuuper long answer of why we don't build webext_storage
on Android—we expect it to provide a different implementation that calls into a GV delegate, instead of managing storage directly!
Pushed by kcambridge@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/00b40d247500 Vendor the `webext_storage` component. r=markh https://hg.mozilla.org/integration/autoland/rev/f3deedfe235d Add a binding for the new extension storage Rust component. r=markh
Assignee | ||
Comment 6•4 years ago
|
||
Comment 7•4 years ago
|
||
Backed out 2 changesets for causing bustages.
Backout link: https://hg.mozilla.org/integration/autoland/rev/612772266861a8e4864ae15e609a284d9ffda0e8
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=299515517&repo=autoland&lineNumber=49512
Assignee | ||
Comment 8•4 years ago
|
||
Sorry about that! It would help if I landed what I actually pushed to Try, wouldn't it? 😂
Pushed by kcambridge@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/42cadb78e630 Vendor the `webext_storage` component. r=markh https://hg.mozilla.org/integration/autoland/rev/63dacb1f4104 Add a binding for the new extension storage Rust component. r=markh
Comment 10•4 years ago
|
||
Of course! :D
Comment 11•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/42cadb78e630
https://hg.mozilla.org/mozilla-central/rev/63dacb1f4104
Comment 12•4 years ago
|
||
(In reply to :Lina Cambridge from comment #4)
On Phabricator, Luca asked:
I'd like to know more about this comment and the rationale behind it, especially because I may be missing or misreading something related to it ;-)
If this part is used for the storage.sync implementation part that takes care of storing the data locally (in the new
storage-sync2.sqlite
sqlite > file) as I assume from the quick look a gave to this part, I would expect that this would be in the GeckoView builds too, and then the part related > to actually sync the data from/to the sync backend to be the part integrated at the "Android Components" level.is that right or am I missing something?
Since Phabricator likes to munch comments, and Mark and I were also chatting about Android earlier, I wanted to capture this here! 😁
...
That's quite the tiramisu! 😂 But that's the suuuper long answer of why we don't buildwebext_storage
on Android—we expect it to provide a different implementation that calls into a GV delegate, instead of managing storage directly!
Thanks a lot Lina! Personally I would define the answer as "as detailed as actually needed" :-)
I did really appreciate the level of detail you provided, that was exactly what I needed to know
(and I'm really glad I asked, keeping these detail in mind is going to be invaluable for the reviews on Bug 1623245 that will come next).
Updated•7 months ago
|
Description
•