Closed Bug 1454878 Opened 2 years ago Closed 2 years ago

Update WrExternalLogHandler as to use env_logger


(Core :: Graphics: WebRender, enhancement, P1)




Tracking Status
firefox61 --- fixed


(Reporter: sotaro, Assigned: sotaro)


(Blocks 1 open bug)



(1 file, 1 obsolete file)

In gecko, wr::WrLogLevelFilter::Error is always used for WrLogLevelFilter in WebRenderAPI::InitExternalLogHandler(). It seems better to set GfxLogLevel to WrLogLevelFilter.
Assignee: nobody → sotaro.ikeda.g
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.

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++
Like the following link, log::set_logger_inner() store a logger only when it is not initialized yet.
The rust log is a global system. It seems not make sense to set graphics only logger WrExternalLogHandler to the rust log.
(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)
Blocks: 1456350
Attachment #8969180 - Flags: review?(jmuizelaar) → review+
Attachment #8969180 - Attachment is obsolete: true
Attachment #8970771 - Flags: review+
Pushed by
Update WrExternalLogHandler as to use env_logger r=jrmuizel
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.