From bce2c33963bafe696f8159dca995ea9c48d01fee Mon Sep 17 00:00:00 2001 From: lougeniac64 Date: Sat, 27 Jun 2020 06:26:22 +0000 Subject: [PATCH] (Bug 1635487) Wired up sync logging for extension pref storage r=lina,markh Differential Revision: https://phabricator.services.mozilla.com/D80975 --- Cargo.lock | 14 ++++ security/manager/ssl/cert_storage/src/lib.rs | 5 +- .../AppServicesLoggerComponents.h | 38 ++++++++++ .../common/app_services_logger/Cargo.toml | 15 ++++ .../app_services_logger/components.conf | 15 ++++ .../common/app_services_logger/src/lib.rs | 71 +++++++++++++++++++ services/common/moz.build | 8 +++ services/interfaces/moz.build | 3 +- services/interfaces/mozIAppServicesLogger.idl | 11 +++ services/interfaces/mozIBridgedSyncEngine.idl | 4 +- ...icesLogger.idl => mozIServicesLogSink.idl} | 8 ++- services/sync/golden_gate/src/log.rs | 29 ++++---- services/sync/modules/bridged_engine.js | 15 ++-- .../sync/modules/engines/extension-storage.js | 7 ++ .../storage/webext_storage_bridge/src/area.rs | 10 +-- .../places/SyncedBookmarksMirror.jsm | 12 ++-- .../places/bookmark_sync/src/driver.rs | 8 +-- .../places/bookmark_sync/src/merger.rs | 26 +++---- .../places/mozISyncedBookmarksMirror.idl | 4 +- toolkit/library/rust/shared/Cargo.toml | 1 + toolkit/library/rust/shared/lib.rs | 16 ++++- .../test_register_app_services_logger.js | 26 +++++++ toolkit/profile/xpcshell/xpcshell.ini | 1 + xpcom/base/nsDebugImpl.cpp | 9 +++ xpcom/base/nsIDebug2.idl | 9 +++ xpcom/rust/gecko_logger/Cargo.toml | 1 + xpcom/rust/gecko_logger/src/lib.rs | 17 ++++- xpcom/rust/moz_task/src/lib.rs | 1 - 28 files changed, 318 insertions(+), 66 deletions(-) create mode 100644 services/common/app_services_logger/AppServicesLoggerComponents.h create mode 100644 services/common/app_services_logger/Cargo.toml create mode 100644 services/common/app_services_logger/components.conf create mode 100644 services/common/app_services_logger/src/lib.rs create mode 100644 services/interfaces/mozIAppServicesLogger.idl rename services/interfaces/{mozIServicesLogger.idl => mozIServicesLogSink.idl} (81%) create mode 100644 toolkit/profile/xpcshell/test_register_app_services_logger.js diff --git a/Cargo.lock b/Cargo.lock index bc58f119c88d..5a39529e02cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -27,6 +27,18 @@ version = "1.0.30" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2494382e9ba43995f3c56359e518641f450f5c36feeb4632a75cde2ec297c867" +[[package]] +name = "app_services_logger" +version = "0.1.0" +dependencies = [ + "golden_gate", + "log", + "nserror", + "nsstring", + "once_cell", + "xpcom", +] + [[package]] name = "app_units" version = "0.7.0" @@ -1612,6 +1624,7 @@ dependencies = [ name = "gecko_logger" version = "0.1.0" dependencies = [ + "app_services_logger", "env_logger", "lazy_static", "log", @@ -1852,6 +1865,7 @@ dependencies = [ name = "gkrust-shared" version = "0.1.0" dependencies = [ + "app_services_logger", "audio_thread_priority", "audioipc-client", "audioipc-server", diff --git a/security/manager/ssl/cert_storage/src/lib.rs b/security/manager/ssl/cert_storage/src/lib.rs index 4d1c645a56b0..b9067bf42e29 100644 --- a/security/manager/ssl/cert_storage/src/lib.rs +++ b/security/manager/ssl/cert_storage/src/lib.rs @@ -1268,10 +1268,7 @@ impl CertStorage { NS_OK } - unsafe fn GetRemainingOperationCount( - &self, - state: *mut i32, - ) -> nserror::nsresult { + unsafe fn GetRemainingOperationCount(&self, state: *mut i32) -> nserror::nsresult { if !is_main_thread() { return NS_ERROR_NOT_SAME_THREAD; } diff --git a/services/common/app_services_logger/AppServicesLoggerComponents.h b/services/common/app_services_logger/AppServicesLoggerComponents.h new file mode 100644 index 000000000000..015eae230b6a --- /dev/null +++ b/services/common/app_services_logger/AppServicesLoggerComponents.h @@ -0,0 +1,38 @@ +/* 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/. */ + +#ifndef mozilla_services_AppServicesLoggerComponents_h_ +#define mozilla_services_AppServicesLoggerComponents_h_ + +#include "mozIAppServicesLogger.h" +#include "nsCOMPtr.h" + +extern "C" { + +// Implemented in Rust, in the `app_services_logger` crate. +nsresult NS_NewAppServicesLogger(mozIAppServicesLogger** aResult); + +} // extern "C" + +namespace mozilla { +namespace appservices { + +// The C++ constructor for a `services.appServicesLogger` service. This wrapper +// exists because `components.conf` requires a component class constructor to +// return an `already_AddRefed`, but Rust doesn't have such a type. So we +// call the Rust constructor using a `nsCOMPtr` (which is compatible with Rust's +// `xpcom::RefPtr`) out param, and return that. +already_AddRefed NewLogService() { + nsCOMPtr logger; + nsresult rv = NS_NewAppServicesLogger(getter_AddRefs(logger)); + if (NS_WARN_IF(NS_FAILED(rv))) { + return nullptr; + } + return logger.forget(); +} + +} // namespace appservices +} // namespace mozilla + +#endif // mozilla_services_AppServicesLoggerComponents_h_ diff --git a/services/common/app_services_logger/Cargo.toml b/services/common/app_services_logger/Cargo.toml new file mode 100644 index 000000000000..5e3fb897d88b --- /dev/null +++ b/services/common/app_services_logger/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "app_services_logger" +version = "0.1.0" +authors = ["lougeniac64 "] +edition = "2018" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +golden_gate = { path = "../../../services/sync/golden_gate" } +log = "0.4" +once_cell = "1.4.0" +nserror = { path = "../../../xpcom/rust/nserror" } +nsstring = { path = "../../../xpcom/rust/nsstring" } +xpcom = { path = "../../../xpcom/rust/xpcom" } diff --git a/services/common/app_services_logger/components.conf b/services/common/app_services_logger/components.conf new file mode 100644 index 000000000000..cdea60a04bcc --- /dev/null +++ b/services/common/app_services_logger/components.conf @@ -0,0 +1,15 @@ +# -*- Mode: python; indent-tabs-mode: nil; tab-width: 40 -*- +# vim: set filetype=python: +# 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/. + +Classes = [ + { + 'cid': '{d2716568-f5fa-4989-91dd-e11599e932a1}', + 'contract_ids': ['@mozilla.org/appservices/logger;1'], + 'type': 'mozIAppServicesLogger', + 'headers': ['mozilla/appservices/AppServicesLoggerComponents.h'], + 'constructor': 'mozilla::appservices::NewLogService', + }, +] diff --git a/services/common/app_services_logger/src/lib.rs b/services/common/app_services_logger/src/lib.rs new file mode 100644 index 000000000000..36e3cf96351f --- /dev/null +++ b/services/common/app_services_logger/src/lib.rs @@ -0,0 +1,71 @@ +/* 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 https://mozilla.org/MPL/2.0/. */ + +//! This provides a XPCOM service to send app services logs to the desktop + +#[macro_use] +extern crate xpcom; + +use golden_gate::log::LogSink; +use log; +use nserror::{nsresult, NS_OK}; +use nsstring::nsAString; +use once_cell::sync::Lazy; +use std::{cmp, collections::HashMap, sync::RwLock}; +use xpcom::{ + interfaces::{mozIAppServicesLogger, mozIServicesLogSink}, + RefPtr, +}; + +#[derive(xpcom)] +#[xpimplements(mozIAppServicesLogger)] +#[refcnt = "nonatomic"] +pub struct InitAppServicesLogger {} + +pub static LOGGERS_BY_TARGET: Lazy>> = Lazy::new(|| { + let h: HashMap = HashMap::new(); + let m = RwLock::new(h); + m +}); + +impl AppServicesLogger { + xpcom_method!(register => Register(target: *const nsAString, logger: *const mozIServicesLogSink)); + fn register(&self, target: &nsAString, logger: &mozIServicesLogSink) -> Result<(), nsresult> { + let log_sink_logger = LogSink::with_logger(Some(logger))?; + let max_level = cmp::max(log::max_level(), log_sink_logger.max_level); + + // Note: This will only work if the max_level is lower than the compile-time + // max_level_* filter. + log::set_max_level(max_level); + + LOGGERS_BY_TARGET + .write() + .unwrap() + .insert(target.to_string(), log_sink_logger); + Ok(()) + } + + pub fn is_app_services_logger_registered(target: String) -> bool { + match LOGGERS_BY_TARGET.read() { + Ok(loggers_by_target) => loggers_by_target.contains_key(&target), + Err(_e) => false, + } + } +} + +/// The constructor for an `AppServicesLogger` service. This uses C linkage so that it +/// can be called from C++. See `AppServicesLoggerComponents.h` for the C++ +/// constructor that's passed to the component manager. +/// +/// # Safety +/// +/// This function is unsafe because it dereferences `result`. +#[no_mangle] +pub unsafe extern "C" fn NS_NewAppServicesLogger( + result: *mut *const mozIAppServicesLogger, +) -> nsresult { + let logger = AppServicesLogger::allocate(InitAppServicesLogger {}); + RefPtr::new(logger.coerce::()).forget(&mut *result); + NS_OK +} diff --git a/services/common/moz.build b/services/common/moz.build index a5671d42dac5..720d43f33bef 100644 --- a/services/common/moz.build +++ b/services/common/moz.build @@ -9,6 +9,10 @@ with Files('**'): TEST_DIRS += ['tests'] +EXPORTS.mozilla.appservices += [ + 'app_services_logger/AppServicesLoggerComponents.h', +] + EXTRA_COMPONENTS += [ 'servicesComponents.manifest', ] @@ -37,3 +41,7 @@ TESTING_JS_MODULES.services.common += [ ] SPHINX_TREES['services'] = 'docs' + +XPCOM_MANIFESTS += [ + 'app_services_logger/components.conf', +] diff --git a/services/interfaces/moz.build b/services/interfaces/moz.build index bce96642c53e..babb706a042a 100644 --- a/services/interfaces/moz.build +++ b/services/interfaces/moz.build @@ -12,7 +12,8 @@ with Files('**'): XPIDL_MODULE = 'services' XPIDL_SOURCES += [ + 'mozIAppServicesLogger.idl', 'mozIBridgedSyncEngine.idl', 'mozIInterruptible.idl', - 'mozIServicesLogger.idl', + 'mozIServicesLogSink.idl', ] diff --git a/services/interfaces/mozIAppServicesLogger.idl b/services/interfaces/mozIAppServicesLogger.idl new file mode 100644 index 000000000000..17d8c87c3af9 --- /dev/null +++ b/services/interfaces/mozIAppServicesLogger.idl @@ -0,0 +1,11 @@ +/* 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/. */ + +#include "nsISupports.idl" +#include "mozIServicesLogSink.idl" + +[scriptable, uuid(446dd837-fbb0-41e4-8221-f740f672b20d)] +interface mozIAppServicesLogger : nsISupports { + void register(in AString target, in mozIServicesLogSink logger); +}; diff --git a/services/interfaces/mozIBridgedSyncEngine.idl b/services/interfaces/mozIBridgedSyncEngine.idl index 5af4890821da..72683abf2211 100644 --- a/services/interfaces/mozIBridgedSyncEngine.idl +++ b/services/interfaces/mozIBridgedSyncEngine.idl @@ -2,7 +2,7 @@ * 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/. */ -#include "mozIServicesLogger.idl" +#include "mozIServicesLogSink.idl" #include "nsISupports.idl" interface nsIVariant; @@ -63,7 +63,7 @@ interface mozIBridgedSyncEngine : nsISupports { // Wires up the Sync logging machinery to the bridged engine. This can be // `null`, in which case any logs from the engine will be discarded. - attribute mozIServicesLogger logger; + attribute mozIServicesLogSink logger; // Returns the last sync time, in milliseconds, for this engine's // collection. This is used to build the collection URL for fetching diff --git a/services/interfaces/mozIServicesLogger.idl b/services/interfaces/mozIServicesLogSink.idl similarity index 81% rename from services/interfaces/mozIServicesLogger.idl rename to services/interfaces/mozIServicesLogSink.idl index 54db045830c0..cbe22545eae3 100644 --- a/services/interfaces/mozIServicesLogger.idl +++ b/services/interfaces/mozIServicesLogSink.idl @@ -8,12 +8,13 @@ // The synced bookmarks mirror and bridged engines implement this interface // to hook in to the services `LogManager` infrastructure. [scriptable, uuid(c92bfe0d-50b7-4a7f-9686-fe5335a696b9)] -interface mozIServicesLogger : nsISupports { +interface mozIServicesLogSink : nsISupports { const short LEVEL_OFF = 0; const short LEVEL_ERROR = 1; const short LEVEL_WARN = 2; - const short LEVEL_DEBUG = 3; - const short LEVEL_TRACE = 4; + const short LEVEL_INFO = 3; + const short LEVEL_DEBUG = 4; + const short LEVEL_TRACE = 5; attribute short maxLevel; @@ -21,4 +22,5 @@ interface mozIServicesLogger : nsISupports { void warn(in AString message); void debug(in AString message); void trace(in AString message); + void info(in AString message); }; diff --git a/services/sync/golden_gate/src/log.rs b/services/sync/golden_gate/src/log.rs index 6373da377770..937f3001e530 100644 --- a/services/sync/golden_gate/src/log.rs +++ b/services/sync/golden_gate/src/log.rs @@ -8,11 +8,11 @@ use log::{Level, LevelFilter, Log, Metadata, Record}; use moz_task::{Task, TaskRunnable, ThreadPtrHandle, ThreadPtrHolder}; use nserror::nsresult; use nsstring::nsString; -use xpcom::{interfaces::mozIServicesLogger, RefPtr}; +use xpcom::{interfaces::mozIServicesLogSink, RefPtr}; pub struct LogSink { pub max_level: LevelFilter, - logger: Option>, + logger: Option>, } impl Default for LogSink { @@ -32,7 +32,7 @@ impl LogSink { /// these, but, for now, we've just duplicated it to make prototyping /// easier. #[inline] - pub fn new(max_level: LevelFilter, logger: ThreadPtrHandle) -> LogSink { + pub fn new(max_level: LevelFilter, logger: ThreadPtrHandle) -> LogSink { LogSink { max_level, logger: Some(logger), @@ -43,7 +43,7 @@ impl LogSink { /// underlying implementation. The `logger` will always be called /// asynchronously on its owning thread; it doesn't need to be /// thread-safe. - pub fn with_logger(logger: Option<&mozIServicesLogger>) -> Result { + pub fn with_logger(logger: Option<&mozIServicesLogSink>) -> Result { Ok(if let Some(logger) = logger { // Fetch the maximum log level while we're on the main thread, so // that `LogSink::enabled()` can check it while on the background @@ -54,10 +54,11 @@ impl LogSink { let rv = unsafe { logger.GetMaxLevel(&mut raw_max_level) }; let max_level = if rv.succeeded() { match raw_max_level as i64 { - mozIServicesLogger::LEVEL_ERROR => LevelFilter::Error, - mozIServicesLogger::LEVEL_WARN => LevelFilter::Warn, - mozIServicesLogger::LEVEL_DEBUG => LevelFilter::Debug, - mozIServicesLogger::LEVEL_TRACE => LevelFilter::Trace, + mozIServicesLogSink::LEVEL_ERROR => LevelFilter::Error, + mozIServicesLogSink::LEVEL_WARN => LevelFilter::Warn, + mozIServicesLogSink::LEVEL_DEBUG => LevelFilter::Debug, + mozIServicesLogSink::LEVEL_TRACE => LevelFilter::Trace, + mozIServicesLogSink::LEVEL_INFO => LevelFilter::Info, _ => LevelFilter::Off, } } else { @@ -65,15 +66,15 @@ impl LogSink { }; LogSink::new( max_level, - ThreadPtrHolder::new(cstr!("mozIServicesLogger"), RefPtr::new(logger))?, + ThreadPtrHolder::new(cstr!("mozIServicesLogSink"), RefPtr::new(logger))?, ) } else { LogSink::default() }) } - /// Returns a reference to the underlying `mozIServicesLogger`. - pub fn logger(&self) -> Option<&mozIServicesLogger> { + /// Returns a reference to the underlying `mozIServicesLogSink`. + pub fn logger(&self) -> Option<&mozIServicesLogSink> { self.logger.as_ref().and_then(|l| l.get()) } @@ -131,7 +132,7 @@ impl Log for LogSink { /// Logs a message to the mirror logger. This task is created on the background /// thread queue, and dispatched to the main thread. struct LogTask { - logger: ThreadPtrHandle, + logger: ThreadPtrHandle, level: Level, message: nsString, } @@ -152,7 +153,9 @@ impl Task for LogTask { Level::Trace => unsafe { logger.Trace(&*self.message); }, - _ => {} + Level::Info => unsafe { + logger.Info(&*self.message); + }, } } diff --git a/services/sync/modules/bridged_engine.js b/services/sync/modules/bridged_engine.js index cc328d8f9047..c317f37b3680 100644 --- a/services/sync/modules/bridged_engine.js +++ b/services/sync/modules/bridged_engine.js @@ -30,7 +30,7 @@ XPCOMUtils.defineLazyModuleGetters(this, { PlacesUtils: "resource://gre/modules/PlacesUtils.jsm", }); -var EXPORTED_SYMBOLS = ["BridgedEngine"]; +var EXPORTED_SYMBOLS = ["BridgedEngine", "LogAdapter"]; /** * A stub store that converts between raw decrypted incoming records and @@ -151,7 +151,7 @@ class InterruptedError extends Error { } /** - * Adapts a `Log.jsm` logger to a `mozIServicesLogger`. This class is copied + * Adapts a `Log.jsm` logger to a `mozIServicesLogSink`. This class is copied * from `SyncedBookmarksMirror.jsm`. */ class LogAdapter { @@ -162,18 +162,18 @@ class LogAdapter { get maxLevel() { let level = this.log.level; if (level <= Log.Level.All) { - return Ci.mozIServicesLogger.LEVEL_TRACE; + return Ci.mozIServicesLogSink.LEVEL_TRACE; } if (level <= Log.Level.Info) { - return Ci.mozIServicesLogger.LEVEL_DEBUG; + return Ci.mozIServicesLogSink.LEVEL_DEBUG; } if (level <= Log.Level.Warn) { - return Ci.mozIServicesLogger.LEVEL_WARN; + return Ci.mozIServicesLogSink.LEVEL_WARN; } if (level <= Log.Level.Error) { - return Ci.mozIServicesLogger.LEVEL_ERROR; + return Ci.mozIServicesLogSink.LEVEL_ERROR; } - return Ci.mozIServicesLogger.LEVEL_OFF; + return Ci.mozIServicesLogSink.LEVEL_OFF; } trace(message) { @@ -214,7 +214,6 @@ function BridgedEngine(bridge, name, service) { SyncEngine.call(this, name, service); this._bridge = bridge; - this._bridge.logger = new LogAdapter(this._log); } BridgedEngine.prototype = { diff --git a/services/sync/modules/engines/extension-storage.js b/services/sync/modules/engines/extension-storage.js index af7b9f7c8a4b..159d83bd1fe4 100644 --- a/services/sync/modules/engines/extension-storage.js +++ b/services/sync/modules/engines/extension-storage.js @@ -15,6 +15,7 @@ const { XPCOMUtils } = ChromeUtils.import( XPCOMUtils.defineLazyModuleGetters(this, { BridgedEngine: "resource://services-sync/bridged_engine.js", + LogAdapter: "resource://services-sync/bridged_engine.js", extensionStorageSync: "resource://gre/modules/ExtensionStorageSync.jsm", Observers: "resource://services-common/observers.js", Svc: "resource://services-sync/util.js", @@ -73,6 +74,12 @@ function setEngineEnabled(enabled) { function ExtensionStorageEngineBridge(service) { let bridge = StorageSyncService.getInterface(Ci.mozIBridgedSyncEngine); BridgedEngine.call(this, bridge, "Extension-Storage", service); + + let app_services_logger = Cc["@mozilla.org/appservices/logger;1"].getService( + Ci.mozIAppServicesLogger + ); + let logger_target = "app-services:webext_storage:sync"; + app_services_logger.register(logger_target, new LogAdapter(this._log)); } ExtensionStorageEngineBridge.prototype = { diff --git a/toolkit/components/extensions/storage/webext_storage_bridge/src/area.rs b/toolkit/components/extensions/storage/webext_storage_bridge/src/area.rs index 909e75dc13e5..ceb87aff9dca 100644 --- a/toolkit/components/extensions/storage/webext_storage_bridge/src/area.rs +++ b/toolkit/components/extensions/storage/webext_storage_bridge/src/area.rs @@ -21,7 +21,7 @@ use webext_storage::STORAGE_VERSION; use xpcom::{ interfaces::{ mozIBridgedSyncEngineApplyCallback, mozIBridgedSyncEngineCallback, - mozIExtensionStorageCallback, mozIServicesLogger, nsIFile, nsISerialEventTarget, + mozIExtensionStorageCallback, mozIServicesLogSink, nsIFile, nsISerialEventTarget, }, RefPtr, }; @@ -311,13 +311,13 @@ impl StorageSyncArea { /// `mozIBridgedSyncEngine` implementation. impl StorageSyncArea { - xpcom_method!(get_logger => GetLogger() -> *const mozIServicesLogger); - fn get_logger(&self) -> Result> { + xpcom_method!(get_logger => GetLogger() -> *const mozIServicesLogSink); + fn get_logger(&self) -> Result> { Err(NS_OK)? } - xpcom_method!(set_logger => SetLogger(logger: *const mozIServicesLogger)); - fn set_logger(&self, _logger: Option<&mozIServicesLogger>) -> Result<()> { + xpcom_method!(set_logger => SetLogger(logger: *const mozIServicesLogSink)); + fn set_logger(&self, _logger: Option<&mozIServicesLogSink>) -> Result<()> { Ok(()) } diff --git a/toolkit/components/places/SyncedBookmarksMirror.jsm b/toolkit/components/places/SyncedBookmarksMirror.jsm index 633253f03472..2ca8e2ada71c 100644 --- a/toolkit/components/places/SyncedBookmarksMirror.jsm +++ b/toolkit/components/places/SyncedBookmarksMirror.jsm @@ -88,7 +88,7 @@ const DEFAULT_MAX_FRECENCIES_TO_RECALCULATE = 400; // Use a shared jankYielder in these functions XPCOMUtils.defineLazyGetter(this, "yieldState", () => Async.yieldState()); -/** Adapts a `Log.jsm` logger to a `mozIServicesLogger`. */ +/** Adapts a `Log.jsm` logger to a `mozIServicesLogSink`. */ class LogAdapter { constructor(log) { this.log = log; @@ -97,18 +97,18 @@ class LogAdapter { get maxLevel() { let level = this.log.level; if (level <= Log.Level.All) { - return Ci.mozIServicesLogger.LEVEL_TRACE; + return Ci.mozIServicesLogSink.LEVEL_TRACE; } if (level <= Log.Level.Info) { - return Ci.mozIServicesLogger.LEVEL_DEBUG; + return Ci.mozIServicesLogSink.LEVEL_DEBUG; } if (level <= Log.Level.Warn) { - return Ci.mozIServicesLogger.LEVEL_WARN; + return Ci.mozIServicesLogSink.LEVEL_WARN; } if (level <= Log.Level.Error) { - return Ci.mozIServicesLogger.LEVEL_ERROR; + return Ci.mozIServicesLogSink.LEVEL_ERROR; } - return Ci.mozIServicesLogger.LEVEL_OFF; + return Ci.mozIServicesLogSink.LEVEL_OFF; } trace(message) { diff --git a/toolkit/components/places/bookmark_sync/src/driver.rs b/toolkit/components/places/bookmark_sync/src/driver.rs index 1dfb3f7db2e0..43bb1f1f792f 100644 --- a/toolkit/components/places/bookmark_sync/src/driver.rs +++ b/toolkit/components/places/bookmark_sync/src/driver.rs @@ -14,7 +14,7 @@ use moz_task::{Task, TaskRunnable, ThreadPtrHandle}; use nserror::nsresult; use nsstring::{nsACString, nsCString, nsString}; use storage_variant::HashPropertyBag; -use xpcom::interfaces::{mozIServicesLogger, mozISyncedBookmarksMirrorProgressListener}; +use xpcom::interfaces::{mozIServicesLogSink, mozISyncedBookmarksMirrorProgressListener}; extern "C" { fn NS_GeneratePlacesGUID(guid: *mut nsACString) -> nsresult; @@ -107,14 +107,14 @@ impl dogear::Driver for Driver { pub struct Logger { pub max_level: LevelFilter, - logger: Option>, + logger: Option>, } impl Logger { #[inline] pub fn new( max_level: LevelFilter, - logger: Option>, + logger: Option>, ) -> Logger { Logger { max_level, logger } } @@ -153,7 +153,7 @@ impl Log for Logger { /// Logs a message to the mirror logger. This task is created on the async /// thread, and dispatched to the main thread. struct LogTask { - logger: ThreadPtrHandle, + logger: ThreadPtrHandle, level: Level, message: nsString, } diff --git a/toolkit/components/places/bookmark_sync/src/merger.rs b/toolkit/components/places/bookmark_sync/src/merger.rs index 3070ce9d5285..eb628966810a 100644 --- a/toolkit/components/places/bookmark_sync/src/merger.rs +++ b/toolkit/components/places/bookmark_sync/src/merger.rs @@ -14,7 +14,7 @@ use storage::Conn; use thin_vec::ThinVec; use xpcom::{ interfaces::{ - mozIPlacesPendingOperation, mozIServicesLogger, mozIStorageConnection, + mozIPlacesPendingOperation, mozIServicesLogSink, mozIStorageConnection, mozISyncedBookmarksMirrorCallback, mozISyncedBookmarksMirrorProgressListener, }, RefPtr, XpCom, @@ -29,7 +29,7 @@ use crate::store; #[refcnt = "nonatomic"] pub struct InitSyncedBookmarksMerger { db: RefCell>, - logger: RefCell>>, + logger: RefCell>>, } impl SyncedBookmarksMerger { @@ -56,16 +56,16 @@ impl SyncedBookmarksMerger { Ok(()) } - xpcom_method!(get_logger => GetLogger() -> *const mozIServicesLogger); - fn get_logger(&self) -> Result, nsresult> { + xpcom_method!(get_logger => GetLogger() -> *const mozIServicesLogSink); + fn get_logger(&self) -> Result, nsresult> { match *self.logger.borrow() { Some(ref logger) => Ok(logger.clone()), None => Err(NS_OK), } } - xpcom_method!(set_logger => SetLogger(logger: *const mozIServicesLogger)); - fn set_logger(&self, logger: Option<&mozIServicesLogger>) -> Result<(), nsresult> { + xpcom_method!(set_logger => SetLogger(logger: *const mozIServicesLogSink)); + fn set_logger(&self, logger: Option<&mozIServicesLogSink>) -> Result<(), nsresult> { self.logger.replace(logger.map(RefPtr::new)); Ok(()) } @@ -125,7 +125,7 @@ struct MergeTask { db: Conn, controller: Arc, max_log_level: LevelFilter, - logger: Option>, + logger: Option>, local_time_millis: i64, remote_time_millis: i64, weak_uploads: Vec, @@ -138,7 +138,7 @@ impl MergeTask { fn new( db: &Conn, controller: Arc, - logger: Option>, + logger: Option>, local_time_seconds: i64, remote_time_seconds: i64, weak_uploads: Vec, @@ -152,15 +152,15 @@ impl MergeTask { Some(level) }) .map(|level| match level as i64 { - mozIServicesLogger::LEVEL_ERROR => LevelFilter::Error, - mozIServicesLogger::LEVEL_WARN => LevelFilter::Warn, - mozIServicesLogger::LEVEL_DEBUG => LevelFilter::Debug, - mozIServicesLogger::LEVEL_TRACE => LevelFilter::Trace, + mozIServicesLogSink::LEVEL_ERROR => LevelFilter::Error, + mozIServicesLogSink::LEVEL_WARN => LevelFilter::Warn, + mozIServicesLogSink::LEVEL_DEBUG => LevelFilter::Debug, + mozIServicesLogSink::LEVEL_TRACE => LevelFilter::Trace, _ => LevelFilter::Off, }) .unwrap_or(LevelFilter::Off); let logger = match logger { - Some(logger) => Some(ThreadPtrHolder::new(cstr!("mozIServicesLogger"), logger)?), + Some(logger) => Some(ThreadPtrHolder::new(cstr!("mozIServicesLogSink"), logger)?), None => None, }; let progress = callback diff --git a/toolkit/components/places/mozISyncedBookmarksMirror.idl b/toolkit/components/places/mozISyncedBookmarksMirror.idl index 8a44f137ba6d..3b0fe524bf25 100644 --- a/toolkit/components/places/mozISyncedBookmarksMirror.idl +++ b/toolkit/components/places/mozISyncedBookmarksMirror.idl @@ -2,7 +2,7 @@ * 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/. */ -#include "mozIServicesLogger.idl" +#include "mozIServicesLogSink.idl" #include "nsISupports.idl" interface mozIPlacesPendingOperation; @@ -79,7 +79,7 @@ interface mozISyncedBookmarksMerger : nsISupports { attribute mozIStorageConnection db; // Optional; used for logging. - attribute mozIServicesLogger logger; + attribute mozIServicesLogSink logger; // Merges the local and remote bookmark trees, applies the merged tree to // Places, and stages locally changed and reconciled items for upload. When diff --git a/toolkit/library/rust/shared/Cargo.toml b/toolkit/library/rust/shared/Cargo.toml index bbd0e9d09948..782a383b2195 100644 --- a/toolkit/library/rust/shared/Cargo.toml +++ b/toolkit/library/rust/shared/Cargo.toml @@ -49,6 +49,7 @@ wgpu_bindings = { path = "../../../../gfx/wgpu_bindings", optional = true } mapped_hyph = { git = "https://github.com/jfkthame/mapped_hyph.git", tag = "v0.3.0" } remote = { path = "../../../../remote", optional = true } fog = { path = "../../../components/glean", optional = true } +app_services_logger = { path = "../../../../services/common/app_services_logger" } unic-langid = { version = "0.8", features = ["likelysubtags"] } unic-langid-ffi = { path = "../../../../intl/locale/rust/unic-langid-ffi" } diff --git a/toolkit/library/rust/shared/lib.rs b/toolkit/library/rust/shared/lib.rs index 3285383a114f..eaaa6a61862d 100644 --- a/toolkit/library/rust/shared/lib.rs +++ b/toolkit/library/rust/shared/lib.rs @@ -6,6 +6,7 @@ extern crate geckoservo; +extern crate app_services_logger; #[cfg(feature = "cubeb-remoting")] extern crate audioipc_client; #[cfg(feature = "cubeb-remoting")] @@ -82,8 +83,10 @@ extern crate remote; extern crate gecko_logger; -use std::ffi::CStr; -use std::os::raw::c_char; +extern crate log; +use log::info; + +use std::{ffi::CStr, os::raw::c_char}; use gecko_logger::GeckoLogger; @@ -102,6 +105,15 @@ pub extern "C" fn intentional_panic(message: *const c_char) { panic!("{}", unsafe { CStr::from_ptr(message) }.to_string_lossy()); } +/// Used to implement `nsIDebug2::rustLog` for testing purposes. +#[no_mangle] +pub extern "C" fn debug_log(target: *const c_char, message: *const c_char) { + unsafe { + // NOTE: The `info!` log macro is used here because we have the `release_max_level_info` feature set. + info!(target: CStr::from_ptr(target).to_str().unwrap(), "{}", CStr::from_ptr(message).to_str().unwrap()); + } +} + #[cfg(feature = "oom_with_hook")] mod oom_hook { use std::alloc::{set_alloc_error_hook, Layout}; diff --git a/toolkit/profile/xpcshell/test_register_app_services_logger.js b/toolkit/profile/xpcshell/test_register_app_services_logger.js new file mode 100644 index 000000000000..cf777c4c6c9b --- /dev/null +++ b/toolkit/profile/xpcshell/test_register_app_services_logger.js @@ -0,0 +1,26 @@ +let waitForDebugLog = target => + new Promise(resolve => { + Cc["@mozilla.org/appservices/logger;1"] + .getService(Ci.mozIAppServicesLogger) + .register(target, { + maxLevel: Ci.mozIServicesLogSink.LEVEL_INFO, + info: resolve, + }); + }); + +let rustLog = (target, message) => { + Cc["@mozilla.org/xpcom/debug;1"] + .getService(Ci.nsIDebug2) + .rustLog(target, message); +}; + +add_task(async () => { + let target = "app-services:webext_storage:sync"; + let expectedMessage = "info error: uh oh"; + let promiseMessage = waitForDebugLog(target); + + rustLog(target, expectedMessage); + + let actualMessage = await promiseMessage; + Assert.ok(actualMessage.includes(expectedMessage)); +}); diff --git a/toolkit/profile/xpcshell/xpcshell.ini b/toolkit/profile/xpcshell/xpcshell.ini index 9a88c0620d56..fb94f0f884d4 100644 --- a/toolkit/profile/xpcshell/xpcshell.ini +++ b/toolkit/profile/xpcshell/xpcshell.ini @@ -44,3 +44,4 @@ skip-if = devedition [test_ignore_legacy_directory.js] [test_select_profile_argument.js] [test_select_profile_argument_new.js] +[test_register_app_services_logger.js] diff --git a/xpcom/base/nsDebugImpl.cpp b/xpcom/base/nsDebugImpl.cpp index 06916117d63a..ddd793c2b312 100644 --- a/xpcom/base/nsDebugImpl.cpp +++ b/xpcom/base/nsDebugImpl.cpp @@ -150,6 +150,15 @@ nsDebugImpl::RustPanic(const char* aMessage) { return NS_OK; } +// From toolkit/library/rust/lib.rs +extern "C" void debug_log(const char* target, const char* message); + +NS_IMETHODIMP +nsDebugImpl::RustLog(const char* aTarget, const char* aMessage) { + debug_log(aTarget, aMessage); + return NS_OK; +} + NS_IMETHODIMP nsDebugImpl::GetIsDebugBuild(bool* aResult) { #ifdef DEBUG diff --git a/xpcom/base/nsIDebug2.idl b/xpcom/base/nsIDebug2.idl index 8a161a91befb..0c4f9e24fd5a 100644 --- a/xpcom/base/nsIDebug2.idl +++ b/xpcom/base/nsIDebug2.idl @@ -87,6 +87,15 @@ interface nsIDebug2 : nsISupports */ void rustPanic(in string aMessage); + /** + * Request the process to log a message for a target and level from Rust code. + * + * @param aTarget the string representing the log target. + * @param aMessage the string representing the log message. + */ + void rustLog(in string aTarget, + in string aMessage); + /** * Cause an Out of Memory Crash. */ diff --git a/xpcom/rust/gecko_logger/Cargo.toml b/xpcom/rust/gecko_logger/Cargo.toml index e281f078c56f..d1c37507e010 100644 --- a/xpcom/rust/gecko_logger/Cargo.toml +++ b/xpcom/rust/gecko_logger/Cargo.toml @@ -9,3 +9,4 @@ license = "MPL-2.0" lazy_static = "1" log = {version = "0.4", features = ["release_max_level_info"]} env_logger = {version = "0.6", default-features = false} # disable `regex` to reduce code size +app_services_logger = { path = "../../../services/common/app_services_logger" } diff --git a/xpcom/rust/gecko_logger/src/lib.rs b/xpcom/rust/gecko_logger/src/lib.rs index 891a56508cff..da85ff92e51c 100644 --- a/xpcom/rust/gecko_logger/src/lib.rs +++ b/xpcom/rust/gecko_logger/src/lib.rs @@ -7,7 +7,7 @@ #[macro_use] extern crate lazy_static; -#[cfg(not(target_os = "android"))] +use app_services_logger::{AppServicesLogger, LOGGERS_BY_TARGET}; use log::Log; use log::{Level, LevelFilter}; use std::boxed::Box; @@ -185,6 +185,18 @@ impl GeckoLogger { log::set_boxed_logger(Box::new(gecko_logger)) } + fn should_log_to_app_services(target: &str) -> bool { + return AppServicesLogger::is_app_services_logger_registered(target.into()); + } + + fn maybe_log_to_app_services(&self, record: &log::Record) { + if Self::should_log_to_app_services(record.target()) { + if let Some(l) = LOGGERS_BY_TARGET.read().unwrap().get(record.target()) { + l.log(record); + } + } + } + fn should_log_to_gfx_critical_note(record: &log::Record) -> bool { record.level() == log::Level::Error && record.target().contains("webrender") } @@ -232,12 +244,13 @@ impl GeckoLogger { impl log::Log for GeckoLogger { fn enabled(&self, metadata: &log::Metadata) -> bool { - self.logger.enabled(metadata) + self.logger.enabled(metadata) || GeckoLogger::should_log_to_app_services(metadata.target()) } fn log(&self, record: &log::Record) { // Forward log to gfxCriticalNote, if the log should be in gfx crash log. self.maybe_log_to_gfx_critical_note(record); + self.maybe_log_to_app_services(record); self.log_out(record); } diff --git a/xpcom/rust/moz_task/src/lib.rs b/xpcom/rust/moz_task/src/lib.rs index b7f599640238..ed3c37393726 100644 --- a/xpcom/rust/moz_task/src/lib.rs +++ b/xpcom/rust/moz_task/src/lib.rs @@ -216,7 +216,6 @@ impl TaskRunnable { .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire) { Ok(_) => { - assert!(!is_current_thread(&self.original_thread)); self.task.run(); Self::dispatch(RefPtr::new(self), &self.original_thread) }