Support WTF logging channels
Created attachment 434895 [details] Patch
Created attachment 434896 [details] Patch
Comment on attachment 434896 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=434896&action=review r- to remove code duplicated between WTF, WebCore and WebKit. > Source/WTF/wtf/Logging.h:57 > +WTF_EXPORT_PRIVATE String logLevelString(); > +bool isLogChannelEnabled(const String& name); > +WTF_EXPORT_PRIVATE void setLogChannelToAccumulate(const String& name); > +WTF_EXPORT_PRIVATE void clearAllLogChannelsToAccumulate(); > +WTF_EXPORT_PRIVATE void initializeLogChannelsIfNecessary(std::optional<String> = std::nullopt); I think you should follow WebCore and put these in LogInitialization.h (there was a reason I split things into two headers, but I forget what it was). I think you can also remove the WebCore and WebKit implementations of these functions, sharing all the code from WTF.
Created attachment 435121 [details] Patch
Created attachment 435122 [details] Patch
Created attachment 435123 [details] Patch
You might want to update the documentation. https://github.com/WebKit/WebKit/blob/main/Introduction.md#logging-in-webkit
(In reply to Fujii Hironori from comment #7) > You might want to update the documentation. > https://github.com/WebKit/WebKit/blob/main/Introduction.md#logging-in-webkit Yeah, good idea. It looks like nothing that's currently written there is inaccurate with these latest patches, but I can add some more details which will make it more clear how to do things like enable/disable log channels.
Also, I can add information about how to add a new log channel, which macro to use where, etc.
https://bugs.webkit.org/show_bug.cgi?id=228899
Comment on attachment 435123 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435123&action=review > Source/WTF/wtf/MemoryPressureHandler.cpp:56 > +#endif Do you still need to explicitly call WTFInitializeLogChannelStatesFromString for MemoryPressure log channel? I think WTF::logChannels().initializeLogChannelsIfNecessary() does the initialization even for MemoryPressure log channel. > Source/WTF/wtf/unix/LoggingUnix.cpp:38 > + return makeString("-all"_s); This seems unexpected behavior change. This disable all WTF log channels as default if WEBKIT_DEBUG is not set. > Source/WTF/wtf/unix/LoggingUnix.cpp:46 > + return makeString("NotYetImplemented,"_s, logEnv); This seems useless. WTF module doesn't have "NotYetImplemented" log channel.
Comment on attachment 435123 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435123&action=review >> Source/WTF/wtf/MemoryPressureHandler.cpp:56 >> +#endif > > Do you still need to explicitly call WTFInitializeLogChannelStatesFromString for MemoryPressure log channel? > I think WTF::logChannels().initializeLogChannelsIfNecessary() does the initialization even for MemoryPressure log channel. Right! Good catch. >> Source/WTF/wtf/unix/LoggingUnix.cpp:38 >> + return makeString("-all"_s); > > This seems unexpected behavior change. This disable all WTF log channels as default if WEBKIT_DEBUG is not set. Right, yes. I copied this from WebCore, but you're right that I shouldn't be changing this behavior. >> Source/WTF/wtf/unix/LoggingUnix.cpp:46 >> + return makeString("NotYetImplemented,"_s, logEnv); > > This seems useless. WTF module doesn't have "NotYetImplemented" log channel. Right.
Created attachment 435163 [details] Patch
Comment on attachment 435163 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435163&action=review LGTM > Source/WTF/wtf/unix/LoggingUnix.cpp:41 > + // To disable logging notImplemented set the DISABLE_NI_WARNING environment variable to 1. Remove this comment.
Committed r280795 (240373@main): <https://commits.webkit.org/240373@main>
<rdar://problem/81713600>
(In reply to Myles C. Maxfield from comment #15) > Committed r280795 (240373@main): <https://commits.webkit.org/240373@main> This change caused build failures for Debug builds of Win, GTK and WPE ports. Bug 228938 – [GTK][WPE] REGRESSION(r280795): MemoryPressureHandlerUnix.cpp:45:28: error: ‘LogMemoryPressure’ was not declared in this scope Bug 228937 – [WTF][Win] REGRESSION(r280795) error LNK2019: unresolved external symbol "class WTF::String __cdecl WTF::logLevelString(void)"
(In reply to Fujii Hironori from comment #17) > (In reply to Myles C. Maxfield from comment #15) > > Committed r280795 (240373@main): <https://commits.webkit.org/240373@main> > > This change caused build failures for Debug builds of Win, GTK and WPE ports. > > Bug 228938 – [GTK][WPE] REGRESSION(r280795): > MemoryPressureHandlerUnix.cpp:45:28: error: ‘LogMemoryPressure’ was not > declared in this scope > Bug 228937 – [WTF][Win] REGRESSION(r280795) error LNK2019: unresolved > external symbol "class WTF::String __cdecl WTF::logLevelString(void)" Oh, no :( EWS was green though :( Thanks for fixing it!!!