Closed Bug 1454878 Opened 2 years ago Closed 2 years ago
External Log Handler as to use env _logger
In gecko, wr::WrLogLevelFilter::Error is always used for WrLogLevelFilter in WebRenderAPI::InitExternalLogHandler(). It seems better to set GfxLogLevel to WrLogLevelFilter. https://dxr.mozilla.org/mozilla-central/source/gfx/webrender_bindings/WebRenderAPI.cpp#257
Hmm, WrExternalLogHandler seems to work only when gpu process is enabled.
WrExternalLogHandler was set by calling log::set_logger(). But it works only when a logger is not set to rust log yet. https://dxr.mozilla.org/mozilla-central/source/gfx/webrender_bindings/src/bindings.rs#2367 When WebRender was crated on parent process, a logger was already set to rust log. It was done under InitializeServo(). Its stack was like the following. xul.dll!log::set_logger_inner<closure>(closure make_logger) 行 1054 xul.dll!env_logger::Builder::try_init() 行 610 xul.dll!env_logger::Builder::init() 行 622 xul.dll!geckoservo::glue::Servo_Initialize(a) 行 18 xul.dll!InitializeServo() 行 2444 C++ xul.dll!nsLayoutStatics::Initialize() 行 286 C++ xul.dll!Initialize() 行 274 C++ xul.dll!nsComponentManagerImpl::KnownModule::Load() 行 727 C++ xul.dll!nsFactoryEntry::GetFactory() 行 1748 C++ xul.dll!nsComponentManagerImpl::CreateInstanceByContractID()
Like the following link, log::set_logger_inner() store a logger only when it is not initialized yet. https://dxr.mozilla.org/mozilla-central/source/third_party/rust/log/src/lib.rs#1049
The rust log is a global system. It seems not make sense to set graphics only logger WrExternalLogHandler to the rust log.
MDN already has a document for logging in rust. https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Gecko_Logging#Rust
(In reply to Sotaro Ikeda [:sotaro] from comment #4) > The rust log is a global system. It seems not make sense to set graphics > only logger WrExternalLogHandler to the rust log. Then it seems better to remove WrExternalLogHandler and use env_logger instead to make the logging consistent to gecko.
I changed my mind, it seems better to keep WrExternalLogHandler for now. Even if WrExternalLogHandler works only with GPU process. It helps to analyze WebRender's bug on Windows. And WrExternalLogHandler could coexist with env_logger.
Summary: Connect GfxLogLevel to WrLogLevel → Update WrExternalLogHandler as to use env_logger
The patch also limit WrExternalLogHandler usage only in gpu process. On non-gpu process, InitializeServo() handles rust logger initialization and WrExternalLogHandler does not work already.
Attachment #8969180 - Flags: review?(jmuizelaar)
2 years ago
Priority: -- → P1
Attachment #8969180 - Flags: review?(jmuizelaar) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/be541e099174 Update WrExternalLogHandler as to use env_logger r=jrmuizel
You need to log in before you can comment on or make changes to this bug.