RESOLVED FIXED 171523
Allow PAL to log messages
https://bugs.webkit.org/show_bug.cgi?id=171523
Summary Allow PAL to log messages
Myles C. Maxfield
Reported 2017-05-01 16:35:50 PDT
[PAL] Migrate WebCore/platform/Logging files to PAL
Attachments
Patch (247.09 KB, patch)
2017-05-01 16:43 PDT, Myles C. Maxfield
no flags
Patch (33.32 KB, patch)
2017-10-10 16:45 PDT, Myles C. Maxfield
no flags
CMake changes (744 bytes, patch)
2017-10-11 11:10 PDT, Don Olmstead
no flags
Patch (32.88 KB, patch)
2017-10-11 12:06 PDT, Myles C. Maxfield
no flags
Myles C. Maxfield
Comment 1 2017-05-01 16:43:06 PDT
Simon Fraser (smfr)
Comment 2 2017-05-01 16:47:52 PDT
Comment on attachment 308785 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308785&action=review > Source/WebCore/PAL/pal/Logging.h:93 > +#define PAL_LOG_CHANNELS(M) \ > + M(Animations) \ > + M(Archives) \ > + M(Compositing) \ > + M(ContentFiltering) \ > + M(DisplayLists) \ > + M(DOMTimers) \ > + M(Editing) \ > + M(Events) \ > + M(FileAPI) \ > + M(Frames) \ > + M(FTP) \ > + M(Fullscreen) \ > + M(Gamepad) \ > + M(History) \ > + M(IconDatabase) \ > + M(Images) \ > + M(IndexedDB) \ > + M(IndexedDBOperations) \ > + M(Layers) \ > + M(Layout) \ > + M(Loading) \ > + M(Media) \ > + M(MediaSource) \ > + M(MediaSourceSamples) \ > + M(MediaCaptureSamples) \ > + M(MemoryPressure) \ > + M(Network) \ > + M(NotYetImplemented) \ > + M(PageCache) \ > + M(PerformanceLogging) \ > + M(PlatformLeaks) \ > + M(Plugins) \ > + M(PopupBlocking) \ > + M(Progress) \ > + M(RemoteInspector) \ > + M(ResourceLoading) \ > + M(ResourceLoadObserver) \ > + M(Scrolling) \ > + M(Services) \ > + M(SpellingAndGrammar) \ > + M(SQLDatabase) \ > + M(StorageAPI) \ > + M(SVG) \ > + M(TextAutosizing) \ > + M(Tiling) \ > + M(Threading) \ > + M(URLParser) \ > + M(WebAudio) \ > + M(WebGL) \ > + M(WebGPU) \ > + M(WebRTC) \ > + M(WebReplay) \ > + M(WheelEventTestTriggers) \ Nope, these are WebCore-specifc and should not be in PAL.
Yoshiaki Jitsukawa
Comment 3 2017-05-01 16:50:49 PDT
*** Bug 171519 has been marked as a duplicate of this bug. ***
Sam Weinig
Comment 4 2017-05-01 17:00:37 PDT
All of the #include lists need to be resorted.
Myles C. Maxfield
Comment 5 2017-05-01 17:04:48 PDT
Comment on attachment 308785 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308785&action=review >> Source/WebCore/PAL/pal/Logging.h:93 >> + M(WheelEventTestTriggers) \ > > Nope, these are WebCore-specifc and should not be in PAL. Seems like we should put these in a "utils" folder in WebCore, since they don't belong in PAL nor WebCore/Platform.
Myles C. Maxfield
Comment 6 2017-05-01 17:07:26 PDT
Comment on attachment 308785 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308785&action=review > Source/WebCore/PAL/pal/Logging.h:101 > +String logLevelString(); This is platform-specific.
Don Olmstead
Comment 7 2017-08-09 19:09:15 PDT
With TextStream moved over this should be unblocked now.
Don Olmstead
Comment 8 2017-08-16 10:58:57 PDT
Simon do you have any thoughts on what should happen with Logging now that TextStream is in WTF and this is unblocked? I don't think we want to duplicate the .cpp files in WebCore and PAL. I'm also wondering if some of this should end up in WTF. Any thoughts on what you would like Logging to look like going forward?
Myles C. Maxfield
Comment 9 2017-10-10 14:47:30 PDT
Comment on attachment 308785 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=308785&action=review >>> Source/WebCore/PAL/pal/Logging.h:93 >>> + M(WheelEventTestTriggers) \ >> >> Nope, these are WebCore-specifc and should not be in PAL. > > Seems like we should put these in a "utils" folder in WebCore, since they don't belong in PAL nor WebCore/Platform. PAL itself will use the following log channels (because they are currently used in WebCore/platform/): Animations ContentFiltering DisplayLists Events Frames Fullscreen Gamepad IOSurface Images Media MediaCaptureSamples MediaSource MediaSourceSamples MediaStream MemoryPressure Network PerformanceLogging ResourceLoadStatistics SQLDatabase WebAudio WebGL WebGPU WebRTC So we need a way to declare some log channels in PAL but some in WebCore.
Myles C. Maxfield
Comment 10 2017-10-10 16:45:24 PDT
Myles C. Maxfield
Comment 11 2017-10-10 16:45:48 PDT
Don, can you help me with the CMake pieces of this?
Don Olmstead
Comment 12 2017-10-10 17:53:31 PDT
(In reply to Myles C. Maxfield from comment #11) > Don, can you help me with the CMake pieces of this? The problem for gtk and wpe appear to be due to the script to create forwarding headers as there's a conflict in names for it. That script should probably not run over stuff in PAL. Forwarding headers are a bit of an open question right now. There is a CMake way to do things that the CMake Mac port is using and then a script that the other ports are using.
Myles C. Maxfield
Comment 13 2017-10-10 19:51:56 PDT
(In reply to Don Olmstead from comment #12) > (In reply to Myles C. Maxfield from comment #11) > > Don, can you help me with the CMake pieces of this? > > The problem for gtk and wpe appear to be due to the script to create > forwarding headers as there's a conflict in names for it. > > That script should probably not run over stuff in PAL. > > Forwarding headers are a bit of an open question right now. There is a CMake > way to do things that the CMake Mac port is using and then a script that the > other ports are using. What do you think we should do to fix it? Should I just rename the file?
Don Olmstead
Comment 14 2017-10-11 11:10:23 PDT
Created attachment 323431 [details] CMake changes This should get cmake going for you
Myles C. Maxfield
Comment 15 2017-10-11 12:06:26 PDT
WebKit Commit Bot
Comment 16 2017-10-11 13:30:20 PDT
Comment on attachment 323438 [details] Patch Clearing flags on attachment: 323438 Committed r223206: <https://trac.webkit.org/changeset/223206>
WebKit Commit Bot
Comment 17 2017-10-11 13:30:22 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 18 2017-10-11 13:30:41 PDT
Ryan Haddad
Comment 19 2017-10-11 17:23:05 PDT
This change broke the Windows debug build: PAL.lib(Logging.obj) : error LNK2019: unresolved external symbol "class WTF::String __cdecl PAL::logLevelString(void)" (?logLevelString@PAL@@YA?AVString@WTF@@XZ) referenced in function "void __cdecl PAL::initializeLogChannelsIfNecessary(class std::optional<class WTF::String>)" (?initializeLogChannelsIfNecessary@PAL@@YAXV?$optional@VString@WTF@@@std@@@Z) [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\WebKitLegacy\WebKitLegacy.vcxproj] https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/4691
Myles C. Maxfield
Comment 20 2017-10-12 11:21:18 PDT
Note You need to log in before you can comment on or make changes to this bug.