Bug 1635487 Comment 0 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Once we have our new extension storage engine, we'll want to wire it up to the Desktop Sync logging machinery. For Reasons, this is a bit more complicated than it has to be, and I wanted to capture some of those sticking points in this ticket. This will involve changes to both a-s and m-c.

The standard for Rust logging is the `log` crate, which we use extensively in a-s. Unfortunately, `log` only lets you set a [global logger](https://docs.rs/log/0.4.6/log/fn.set_boxed_logger.html), and only once. That means calling `set_boxed_logger` will set it for all of Firefox—I think one is already set, anyway, so that will fail with an error. But, even if it did work, we wouldn't want that, because other in-tree crates also use `log`. We don't want, say, WebRender and Necko trace logs getting written into `about:sync-log`!

This is why Dogear basically [reimplements the `log` macros](https://github.com/mozilla/dogear/blob/41b498c6fa9ad6270cf7268b5399e45eb57e8a69/src/driver.rs#L124-L212, but with a "driver" as the first argument. The driver has a `logger`, which, [by default](https://github.com/mozilla/dogear/blob/41b498c6fa9ad6270cf7268b5399e45eb57e8a69/src/driver.rs#L100-L109), uses the `log` crate's global logger. But Desktop overrides this to return [its own logger](https://searchfox.org/mozilla-central/rev/7908ce29657cfd623993046bd8e38664e1c0b28e/toolkit/components/places/bookmark_sync/src/driver.rs#88-91), which calls into the Sync logger. There's some complicated machinery around dispatching log messages to the main thread, which we need to do because the logger is in JS. All of that is factored out into [`golden_gate::LogSink`](https://searchfox.org/mozilla-central/rev/7908ce29657cfd623993046bd8e38664e1c0b28e/services/sync/golden_gate/src/log.rs#27-98). On the JS side, we have another tiny [helper class](https://searchfox.org/mozilla-central/rev/7908ce29657cfd623993046bd8e38664e1c0b28e/toolkit/components/places/SyncedBookmarksMirror.jsm#91-129) that adapts a `Log.jsm` logger to a `mozIServicesLogger` that `golden_gate::LogSink` expects.

If we want to copy what Dogear does, on the a-s side, we can add a new support crate (`log-support`?) with our reimplemented macros, and have them take a `&dyn Log`. We can store the logger in a field on `webext_storage::Store`, and pass it to any method that needs it, and thread that through to the callback.

It's kind of unfortunate that this means passing an extra `log: &dyn Log` as an argument to any function that wants to do logging, but it's one approach. Let's brainstorm others in this ticket, too!

Whatever approach we decide on here, we'll probably want to copy for our other Desktop integrations.
Once we have our new extension storage engine, we'll want to wire it up to the Desktop Sync logging machinery. For Reasons, this is a bit more complicated than it has to be, and I wanted to capture some of those sticking points in this ticket. This will involve changes to both a-s and m-c.

The standard for Rust logging is the `log` crate, which we use extensively in a-s. Unfortunately, `log` only lets you set a [global logger](https://docs.rs/log/0.4.6/log/fn.set_boxed_logger.html), and only once. That means calling `set_boxed_logger` will set it for all of Firefox—I think one is already set, anyway, so that will fail with an error. But, even if it did work, we wouldn't want that, because other in-tree crates also use `log`. We don't want, say, WebRender and Necko trace logs getting written into `about:sync-log`!

This is why Dogear basically [reimplements the `log` macros](https://github.com/mozilla/dogear/blob/41b498c6fa9ad6270cf7268b5399e45eb57e8a69/src/driver.rs#L124-L212), but with a "driver" as the first argument. The driver has a `logger`, which, [by default](https://github.com/mozilla/dogear/blob/41b498c6fa9ad6270cf7268b5399e45eb57e8a69/src/driver.rs#L100-L109), uses the `log` crate's global logger. But Desktop overrides this to return [its own logger](https://searchfox.org/mozilla-central/rev/7908ce29657cfd623993046bd8e38664e1c0b28e/toolkit/components/places/bookmark_sync/src/driver.rs#88-91), which calls into the Sync logger. There's some complicated machinery around dispatching log messages to the main thread, which we need to do because the logger is in JS. All of that is factored out into [`golden_gate::LogSink`](https://searchfox.org/mozilla-central/rev/7908ce29657cfd623993046bd8e38664e1c0b28e/services/sync/golden_gate/src/log.rs#27-98). On the JS side, we have another tiny [helper class](https://searchfox.org/mozilla-central/rev/7908ce29657cfd623993046bd8e38664e1c0b28e/toolkit/components/places/SyncedBookmarksMirror.jsm#91-129) that adapts a `Log.jsm` logger to a `mozIServicesLogger` that `golden_gate::LogSink` expects.

If we want to copy what Dogear does, on the a-s side, we can add a new support crate (`log-support`?) with our reimplemented macros, and have them take a `&dyn Log`. We can store the logger in a field on `webext_storage::Store`, and pass it to any method that needs it, and thread that through to the callback.

It's kind of unfortunate that this means passing an extra `log: &dyn Log` as an argument to any function that wants to do logging, but it's one approach. Let's brainstorm others in this ticket, too!

Whatever approach we decide on here, we'll probably want to copy for our other Desktop integrations.

Back to Bug 1635487 Comment 0