Bug 228809 - Deduplicate logging channel algorithms
Summary: Deduplicate logging channel algorithms
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on: 228897
Blocks: 228768 228899
  Show dependency tree / graph
 
Reported: 2021-08-04 19:29 PDT by Myles C. Maxfield
Modified: 2021-09-12 12:45 PDT (History)
25 users (show)

See Also:

Attachments
Patch (63.86 KB, patch)
2021-08-05 19:57 PDT, Myles C. Maxfield 		Hironori.Fujii: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for committing (64.38 KB, patch)
2021-08-06 20:05 PDT, Myles C. Maxfield 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch for committing (64.98 KB, patch)
2021-08-06 23:40 PDT, Myles C. Maxfield 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2021-08-04 19:29:08 PDT 
Deduplicate logging channel algorithms
Comment 1 Myles C. Maxfield 2021-08-05 19:57:12 PDT 
Created attachment 435051 [details]
Patch
Comment 2 Fujii Hironori 2021-08-06 14:40:34 PDT 
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 3 Fujii Hironori 2021-08-06 14:44:47 PDT 
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?
Comment 5 Fujii Hironori 2021-08-06 14:52:45 PDT 
Comment on attachment 435051 [details]
Patch

Nice refactoring. LGTM. But, cq- for the GTK build problem.
Comment 6 Myles C. Maxfield 2021-08-06 20:05:34 PDT 
Created attachment 435110 [details]
Patch for committing
Comment 7 Myles C. Maxfield 2021-08-06 23:40:08 PDT 
Created attachment 435119 [details]
Patch for committing
Comment 8 Myles C. Maxfield 2021-08-07 00:55:03 PDT 
Committed r280756 (240341@main): <https://commits.webkit.org/240341@main>
Comment 9 Radar WebKit Bug Importer 2021-08-07 00:56:17 PDT 
<rdar://problem/81647472>
Comment 10 Aakash Jain 2021-08-07 09:26:09 PDT 
(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
Comment 11 WebKit Commit Bot 2021-08-07 09:27:49 PDT 
Re-opened since this is blocked by bug 228897
Comment 12 Myles C. Maxfield 2021-08-07 11:15:58 PDT 
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.
Comment 13 EWS 2021-08-07 11:50:23 PDT 
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].
Comment 14 Myles C. Maxfield 2021-08-07 11:55:35 PDT 
Watching the GTK bot now...
Comment 15 David Kilzer (:ddkilzer) 2021-09-12 12:45:14 PDT 
(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>