| Summary: | Deduplicate logging channel algorithms | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||||||
| Component: | New Bugs | Assignee: | Myles C. Maxfield <mmaxfield> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | aakash_jain, aboxhall, andresg_22, annulen, apinheiro, benjamin, cdumez, cfleizach, cmarcelo, commit-queue, ddkilzer, dmazzoni, ews-watchlist, gyuyoung.kim, hi, Hironori.Fujii, jcraig, jdiggs, joepeck, ryuan.choi, samuel_white, sergio, simon.fraser, webkit-bot-watchers-bugzilla, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Bug Depends on: | 228897 | ||||||||||
| Bug Blocks: | 228768, 228899 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Myles C. Maxfield
2021-08-04 19:29:08 PDT
Created attachment 435051 [details]
Patch
Comment on attachment 435051 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435051&action=review > Source/WTF/wtf/LogChannels.cpp:37 > + WTFLogChannel* channel = WTFLogChannelByName(m_logChannels.data(), m_logChannels.size(), name.utf8().data()); You can use getLogChannel(name) here. > Source/WTF/wtf/LogChannels.cpp:45 > + WTFLogChannel* channel = WTFLogChannelByName(m_logChannels.data(), m_logChannels.size(), name.utf8().data()); Ditto. Comment on attachment 435051 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435051&action=review > Source/WebCore/platform/LogInitialization.cpp:32 > +#include <wtf/text/CString.h> Do you really use <wtf/text/CString.h> in this file? > Source/WebCore/platform/LogInitialization.cpp:36 > +#if PLATFORM(COCOA) Do you really need Cocoa specific headers in this file? They have some #undef for the problem. https://github.com/WebKit/WebKit/blob/4c96573f0f4dde291b8ce4a23dfd54d4b9fd7d5e/Source/WebCore/platform/graphics/GLContext.h#L51 https://github.com/WebKit/WebKit/blob/4c96573f0f4dde291b8ce4a23dfd54d4b9fd7d5e/Source/WebCore/platform/graphics/GraphicsContext.h#L73 Comment on attachment 435051 [details]
Patch
Nice refactoring. LGTM. But, cq- for the GTK build problem.
Created attachment 435110 [details]
Patch for committing
Created attachment 435119 [details]
Patch for committing
Committed r280756 (240341@main): <https://commits.webkit.org/240341@main> (In reply to Myles C. Maxfield from comment #8) > Committed r280756 (240341@main): <https://commits.webkit.org/240341@main> This seems to have broken GTK build. e.g.: https://build.webkit.org/#/builders/41/builds/4806 passed with 240340@main https://build.webkit.org/#/builders/41/builds/4807 failed with 240341@main Re-opened since this is blocked by bug 228897 Oh, somehow the patch I committed didn't include the fix for GTK! The Patch for committing above includes a change to Threading.h but r280756 doesn't. Let's try this again, with the fully updated patch this time. Committed r280758 (240343@main): <https://commits.webkit.org/240343@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 435119 [details]. Watching the GTK bot now... (In reply to EWS from comment #13) > Committed r280758 (240343@main): <https://commits.webkit.org/240343@main> > > All reviewed patches have been landed. Closing bug and clearing flags on > attachment 435119 [details]. Follow-up fix to the WebKit Xcode project: REGRESSION (r280758): WebKit project won't open in Xcode 11.4 <https://trac.webkit.org/changeset/282317/webkit> |